-
Notifications
You must be signed in to change notification settings - Fork 57
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
Doesn't work on NodeJS 12 #38
Comments
Looks like v12 finally removed a bunch of functionality that we make heavy use of... For now, use a node version < v12. v10 LTS will be supported until April 2021. But it's nice to have this here for priorities. Thanks for the report. |
I reported NaN issues to the NaN repo, we will need a fix from them before we can work on this: Here's the list of problems we can fix: Basically just stop using
|
Also: #26 seems like a good idea... people will see the error logs, but using optionalDependencies we can at least pass your tests with the javascript implementation. |
In the meantime can we set the max version to < 12 here? https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/package.json#L8 |
Why there? Why not here? How would this help if the result is a failed install either way? |
Ooops, I'm sorry, I think was looking at a different repo when I copy and pasted that. Yeah I mean here. I think for me, it wasn't pleasant hunting down why my heroku deployment failed though I have no problem setting node version of my package anyways. Just thought it might help those in the meantime if/when tiny-secp256k1 supports the changes introduced by node12. |
Looks like secp256k1-node have a fix that could be ported here? |
I fixed it. |
Fixed in v1.1.1 thanks to @Xmader ! Thanks a lot! |
I specified NodeJS Stable on Travis CI, but yesterday the pipelines broke on
tiny-secp256k1
. More context on travis-ci.org/GetEpona/Epona-js.The text was updated successfully, but these errors were encountered: