Skip to content
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

Node 12 support #2913

Closed
ellis2323 opened this issue Jun 26, 2019 · 27 comments
Closed

Node 12 support #2913

ellis2323 opened this issue Jun 26, 2019 · 27 comments
Labels
1.x 1.0 related issues 2.x 2.0 related issues

Comments

@ellis2323
Copy link

web3.js uses web3-eth-accounts which uses scrypt.js which uses scrypt. Scrypt is not maintained anymore and is incompatible with node 12. I've contacted scrypt.js but i'm not sure this package is well maintained. a pull request is available on scrypt.js to fix the problem.

barrysteyn/node-scrypt#193

@nivida
Copy link
Contributor

nivida commented Jun 26, 2019

Thanks for opening this issue! We have already discussed this issue and @alcuadrado opened the PR's for the scrypt.js package. The goal would be to have this fix included in the next release.

@nivida nivida added 1.x 1.0 related issues 2.x 2.0 related issues dependencies labels Jun 26, 2019
@SebastianGiro
Copy link

#2903

@ellis2323 @nivida

@nivida nivida mentioned this issue Jul 10, 2019
11 tasks
@levino
Copy link
Contributor

levino commented Jul 10, 2019

Why not remove the package altogether and use something else?

@nivida
Copy link
Contributor

nivida commented Jul 10, 2019

There are several ways to solve it:

  1. Moving of the logic from the scrypt.js package to Web3.js to fix and improve it.
  2. Alex (axic) the maintainer and part of the Ethereum Foundation can handover us the ownership and we will update the package. (Which would be good because it's used in many other ethereum JS related packages and projects which are depending on these modules)
  3. Usage of the scrypt.js logic and publishing of a new package with a future proof fix
  4. Checking if there are alternative packages with the same outcome.

The fix for this issue will be released with 1.0.1 and 2.0-alpha.1

@levino
Copy link
Contributor

levino commented Jul 10, 2019

The fix for this issue will be released with 1.0.1 and 2.0-alpha.1

Really? Why? This is the most important thing for web3? I think it is okay not to support node 12 for the time being...

@SebastianGiro
Copy link

@levino angular 8 depends on node 12. And is one of the most used frameworks out there.

@levino
Copy link
Contributor

levino commented Jul 11, 2019

Angular apps run in the browser, so not in node. Maybe the angular build tools depend on node 12 but then just use the minified browser version of web3 for your project and you should be fine...

@joshstevens19
Copy link
Contributor

joshstevens19 commented Jul 11, 2019

Just to correct that statement here, current web3 can run on angular 8 fine using the standard NPM install and not the minified browser version we have it all working with no problems.

Angular for all versions including 8 only recommends the latest stable node release it does work and supports node 10 and will even run on node 8.

If angular forced something like that it would be so much disruption for companies that would then need to upgrade all of their build servers with Node 12 before thinking about creating an Angular 8 project.

@SebastianGiro

@levino
Copy link
Contributor

levino commented Jul 11, 2019

For the record: I agree that this is an issue and that support for node 12 must be established rather sooner than later. I was merely saying that there are workarounds and imo nobody needs to be blocked by this atm.

Angular for all versions including 8 only recommends the latest stable node release it does work and supports node 10 and will even run on node 8.

That's correct.

If angular forced something like that it would be so much disruption for companies that would then need to upgrade all of their build servers with Node 12 before thinking about creating an Angular 8 project.

Please be reasonable. "Angular is wrong to require node 12" is surely not an argument.

@joshstevens19
Copy link
Contributor

joshstevens19 commented Jul 11, 2019

@levino angular does not require node 12! it recommends the latest version of node and runs perfectly fine on older versions, we should stop using the word require which can confuse a lot of people here. My statement was a fact in what angular actually pride themselves on not forcing people to have to change their build servers and easy upgrades. Moral of the story angular 8 does not need or require node 12 to work, so it should not be a point in this issue and i was merely just correcting the statements above to avoid huge confusion.

@levino
Copy link
Contributor

levino commented Jul 11, 2019

I agree with you. How about you carefully read my statements first.

@joshstevens19
Copy link
Contributor

joshstevens19 commented Jul 11, 2019

👍

Please be reasonable. "Angular is wrong to require node 12" is surely not an argument.

i did not know if you were telling me to be reasonable so was just replying to that just in case we got our wires crossed. Sorry if i have read your statement wrong reading again it still comes across like your discussing with me 😂 😄 anyway glad we just discussed that!

@SebastianGiro
Copy link

SebastianGiro commented Jul 11, 2019

@joshstevens19 yeah you are right, it doesn't depend on node 12, but they endorse it. So normally if you fresh install, you will just use what angular suggest, (also updating). Thats what happened to me. There is a workaround? Yes. After finding this errors with scrypt and node you can go and do all the workarounds you want.

Also regarding that it will even work with node 8:
Angular requires Node.js version 10.9.0 or later. directly from https://angular.io/guide/setup-local
(I'm not saying that it will not work with node 8)

What I'm trying to say is, that it would be nice to be able to install web3 and angular8 with the default environment angular suggest without any workarounds/investigation/errors in the console.

For now we are just using a preinstall script who changes to node 8, do npm i web3 and then switch back to node 12, then installing dependencies normally.

@hayesgm
Copy link

hayesgm commented Jul 11, 2019

I think we've veered off topic here. With Node 12 replacing Node 11, my team has felt in a dilemma: either we use a version of node without long term support, or we face Web3 compilation issues. I think supporting the current release versions of node is, in fact, the best reason to prioritize this issue.

@michaelsbradleyjr
Copy link
Contributor

michaelsbradleyjr commented Jul 11, 2019

Node 12 will become the Active LTS in about 3 months (!!) while Node 8 will hit EOL at the end of this year:

https://nodejs.org/en/about/releases/

scrypt.js is just a thin wrapper around scrypt (native code) with a fallback to scryptsy (pure JS). One unfortunate thing is that part of the fallback mechanism involves scrypt having failed to install as an optional dependency of scrypt.js ("optionalDependencies": { "scrypt": "^6.0.2"}) , either because the needed dev tools aren't available or because of compilation errors (e.g. when Node 12 is in use). In the latter case, there will be lots of nasty errors that are printed to the terminal even though scrypt.js installation will actually succeed since scrypt is an optional dependency.

Proposal

Add scryptsy as an explicit dependency of web3 v1.0 and v2.0 (it was already a transitive dependency) and remove scrypt.js as a dependency.

Add logic to web3 v1.0 and v2.0 that checks whether the runtime is Node and if so checks with semver whether process.version is >= 10.5.0, which is the version when scrypt became a Node built-in (see: crypto.scrypt; click History).

If that's the case then use the built-in.

If the runtime is Node but process.version is too old, then check whether require('scrypt') succeeds. If so, use it. If not, print a warning that says the user should install the deprecated scrypt package as a dependency of their own project if they want native performance, and then proceed to use scryptsy.

If the runtime is not Node then use scryptsy. Alternatively a "browser" field can be included in web3's package.json so that the module that contains the above checks is substituted for one that simply uses scryptsy. That field will be detected by webpack and other front-end build tools.

michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 11, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 11, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 11, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 12, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 16, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 16, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 16, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 16, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/web3.js that referenced this issue Jul 16, 2019
This was referenced Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues 2.x 2.0 related issues
Projects
None yet
Development

No branches or pull requests