-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The async library #55
Comments
@alcuadrado Sina already removed most (all?) usages of the This sounds highly obvious to me with a huge structural win both on the readability and debuggability side, so I think taking some effort here is very much worth it. |
The merkle tree library is also a heavy |
Not sure if there are still occurrences here, will keep this open. |
This another meta-issue on how to improve IMO the state of ethereumjs in general.
Most libraries use the npm package
async
. This was kind of standard practice in the node.js community many years ago, but that's no longer the case.Code written with this library is hard to read and maintain, and I believe it prevents new people from collaborating. Take a look at this function for a clear example of how scary it can get.
Personal preferences apart, this library prevents v8 from creating async stack traces. These were released (enabled by default) in node 12. For them to work, you have to stick to
async
/await
. If you don't, this feature removes some internal (e.g.process._tickcallback
calls) stack frames, leaving an almost empty (even emptier than before!) stack trace.I know this is a huge change, but I believe it's important.
I think a good way to start working on this can be migrating the tests from a library. For example, ethereumjs-blockchain's test.
The text was updated successfully, but these errors were encountered: