search

web3 / web3.js

Collection of comprehensive TypeScript libraries for Interaction with the Ethereum JSON RPC API and utility functions.
https://web3js.org/
Other
19.39k stars 4.97k forks source link

Node 12 support #2913

Closed ellis2323 closed 5 years ago

ellis2323 commented 5 years ago

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.

https://github.com/barrysteyn/node-scrypt/issues/193

nivida commented 5 years ago

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.

SebastianGiro commented 5 years ago

https://github.com/ethereum/web3.js/pull/2903

@ellis2323 @nivida

levino commented 5 years ago

Why not remove the package altogether and use something else?

nivida commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

levino commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

joshstevens19 commented 5 years ago

:+1:

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

I believe this has been effectively closed by #2938.

alcuadrado commented 5 years ago

The new version, 1.2.0, can be installed in node 12. I didn't test if web3-eth-accounts works correctly.

michaelsbradleyjr commented 5 years ago

Actually, 1.2.0 will still spew a lot of errors when installed w/ node 12 because web3-eth-accounts still depends on scrypt.js. It's a newer version of scrypt.js that has scrypt as an optional dependency. However, even as an optional dependency there will be a large output of compilation errors and then the install of web3 itself will succeed (because scrypt was optional).

With my PR #2938 that landed today, the dependency on scrypt.js has been removed, but those changes aren't in a released version of web3 yet; maybe they'll be part of 1.2.1, not sure what @nivida's plans are re: timetable for the next patch/feature release of web3 1.x.

alcuadrado commented 5 years ago

I just installed it with node 12:

pato@pmbp:node12% node --version
v12.4.0
pato@pmbp:node12% npm --version
6.9.0
pato@pmbp:node12% npm i web3

> web3-providers-ws@1.2.0 preinstall /private/tmp/node12/node_modules/web3-providers-ws
> rm -rf node_modules/websocket/.git

> scrypt@6.0.3 preinstall /private/tmp/node12/node_modules/scrypt
> node node-scrypt-preinstall.js

> scrypt@6.0.3 install /private/tmp/node12/node_modules/scrypt
> node-gyp rebuild

  SOLINK_MODULE(target) Release/copied_files.node
  CC(target) Release/obj.target/scrypt_wrapper/src/util/memlimit.o
  CC(target) Release/obj.target/scrypt_wrapper/src/scryptwrapper/keyderivation.o
  CC(target) Release/obj.target/scrypt_wrapper/src/scryptwrapper/pickparams.o
  CC(target) Release/obj.target/scrypt_wrapper/src/scryptwrapper/hash.o
  LIBTOOL-STATIC Release/scrypt_wrapper.a
  CC(target) Release/obj.target/scrypt_lib/scrypt/scrypt-1.2.0/lib/crypto/crypto_scrypt.o
  CC(target) Release/obj.target/scrypt_lib/scrypt/scrypt-1.2.0/lib/crypto/crypto_scrypt_smix.o
  CC(target) Release/obj.target/scrypt_lib/scrypt/scrypt-1.2.0/libcperciva/util/warnp.o
  CC(target) Release/obj.target/scrypt_lib/scrypt/scrypt-1.2.0/libcperciva/alg/sha256.o
  CC(target) Release/obj.target/scrypt_lib/scrypt/scrypt-1.2.0/libcperciva/util/insecure_memzero.o
  CC(target) Release/obj.target/scrypt_lib/scrypt/scrypt-1.2.0/lib/scryptenc/scryptenc_cpuperf.o
  LIBTOOL-STATIC Release/scrypt_lib.a
  CXX(target) Release/obj.target/scrypt/src/node-boilerplate/scrypt_common.o
  CXX(target) Release/obj.target/scrypt/src/node-boilerplate/scrypt_params_async.o
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
In file included from ../src/node-boilerplate/inc/scrypt_async.h:28:
../src/node-boilerplate/inc/scrypt_common.h:39:14: warning: 'Get' is deprecated: Use maybe version [-Wdeprecated-declarations]
      N(obj->Get(Nan::New("N").ToLocalChecked())->Uint32Value()),
             ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3412:3: note: 'Get' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version", Local<Value> Get(Local<Value> key));
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
In file included from ../src/node-boilerplate/inc/scrypt_async.h:28:
../src/node-boilerplate/inc/scrypt_common.h:39:63: error: too few arguments to function call, single argument 'context' was not specified
      N(obj->Get(Nan::New("N").ToLocalChecked())->Uint32Value()),
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2567:3: note: 'Uint32Value' declared here
  V8_WARN_UNUSED_RESULT Maybe<uint32_t> Uint32Value(
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
In file included from ../src/node-boilerplate/inc/scrypt_async.h:28:
../src/node-boilerplate/inc/scrypt_common.h:40:14: warning: 'Get' is deprecated: Use maybe version [-Wdeprecated-declarations]
      r(obj->Get(Nan::New("r").ToLocalChecked())->Uint32Value()),
             ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3412:3: note: 'Get' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version", Local<Value> Get(Local<Value> key));
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
In file included from ../src/node-boilerplate/inc/scrypt_async.h:28:
../src/node-boilerplate/inc/scrypt_common.h:40:63: error: too few arguments to function call, single argument 'context' was not specified
      r(obj->Get(Nan::New("r").ToLocalChecked())->Uint32Value()),
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2567:3: note: 'Uint32Value' declared here
  V8_WARN_UNUSED_RESULT Maybe<uint32_t> Uint32Value(
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
In file included from ../src/node-boilerplate/inc/scrypt_async.h:28:
../src/node-boilerplate/inc/scrypt_common.h:41:14: warning: 'Get' is deprecated: Use maybe version [-Wdeprecated-declarations]
      p(obj->Get(Nan::New("p").ToLocalChecked())->Uint32Value()) {}
             ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3412:3: note: 'Get' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version", Local<Value> Get(Local<Value> key));
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
In file included from ../src/node-boilerplate/inc/scrypt_async.h:28:
../src/node-boilerplate/inc/scrypt_common.h:41:63: error: too few arguments to function call, single argument 'context' was not specified
      p(obj->Get(Nan::New("p").ToLocalChecked())->Uint32Value()) {}
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2567:3: note: 'Uint32Value' declared here
  V8_WARN_UNUSED_RESULT Maybe<uint32_t> Uint32Value(
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
In file included from ../src/node-boilerplate/inc/scrypt_params_async.h:28:
../src/node-boilerplate/inc/scrypt_async.h:53:17: warning: 'Call' is deprecated [-Wdeprecated-declarations]
      callback->Call(1, argv);
                ^
../../nan/nan.h:1673:3: note: 'Call' has been explicitly marked deprecated here
  NAN_DEPRECATED inline v8::Local<v8::Value>
  ^
../../nan/nan.h:103:40: note: expanded from macro 'NAN_DEPRECATED'
# define NAN_DEPRECATED __attribute__((deprecated))
                                       ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
../src/node-boilerplate/inc/scrypt_params_async.h:35:36: error: too few arguments to function call, single argument 'context' was not specified
      maxtime(info[0]->NumberValue()),
              ~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2564:3: note: 'NumberValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<double> NumberValue(Local<Context> context) const;
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
../src/node-boilerplate/inc/scrypt_params_async.h:36:39: error: too few arguments to function call, single argument 'context' was not specified
      maxmemfrac(info[1]->NumberValue()),
                 ~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2564:3: note: 'NumberValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<double> NumberValue(Local<Context> context) const;
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
../src/node-boilerplate/inc/scrypt_params_async.h:37:36: error: too few arguments to function call, single argument 'context' was not specified
      maxmem(info[2]->IntegerValue()),
             ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
In file included from ../src/node-boilerplate/scrypt_params_async.cc:4:
../src/node-boilerplate/inc/scrypt_params_async.h:38:39: error: too few arguments to function call, single argument 'context' was not specified
      osfreemem(info[3]->IntegerValue())
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/node-boilerplate/scrypt_params_async.cc:23:8: warning: 'Set' is deprecated: Use maybe version [-Wdeprecated-declarations]
  obj->Set(Nan::New("N").ToLocalChecked(), Nan::New<Integer>(logN));
       ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3358:3: note: 'Set' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version",
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
../src/node-boilerplate/scrypt_params_async.cc:24:8: warning: 'Set' is deprecated: Use maybe version [-Wdeprecated-declarations]
  obj->Set(Nan::New("r").ToLocalChecked(), Nan::New<Integer>(r));
       ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3358:3: note: 'Set' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version",
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
../src/node-boilerplate/scrypt_params_async.cc:25:8: warning: 'Set' is deprecated: Use maybe version [-Wdeprecated-declarations]
  obj->Set(Nan::New("p").ToLocalChecked(), Nan::New<Integer>(p));
       ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3358:3: note: 'Set' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version",
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
../src/node-boilerplate/scrypt_params_async.cc:32:13: warning: 'Call' is deprecated [-Wdeprecated-declarations]
  callback->Call(2, argv);
            ^
../../nan/nan.h:1673:3: note: 'Call' has been explicitly marked deprecated here
  NAN_DEPRECATED inline v8::Local<v8::Value>
  ^
../../nan/nan.h:103:40: note: expanded from macro 'NAN_DEPRECATED'
# define NAN_DEPRECATED __attribute__((deprecated))
                                       ^
8 warnings and 7 errors generated.
make: *** [Release/obj.target/scrypt/src/node-boilerplate/scrypt_params_async.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/pato/.nvm/versions/node/v12.4.0/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:200:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 18.6.0
gyp ERR! command "/Users/pato/.nvm/versions/node/v12.4.0/bin/node" "/Users/pato/.nvm/versions/node/v12.4.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /private/tmp/node12/node_modules/scrypt
gyp ERR! node -v v12.4.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok

> sha3@1.2.3 install /private/tmp/node12/node_modules/sha3
> node-gyp rebuild

  CXX(target) Release/obj.target/sha3/src/addon.o
In file included from ../src/addon.cpp:9:
In file included from ../src/KeccakNISTInterface.h:17:
../src/KeccakSponge.h:23:9: warning: 'ALIGN' macro redefined [-Wmacro-redefined]
#define ALIGN __attribute__ ((aligned(32)))
        ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/i386/param.h:83:9: note: previous definition is here
#define ALIGN(p)        __DARWIN_ALIGN(p)
        ^
../src/addon.cpp:83:11: warning: 'Set' is deprecated: Use maybe version [-Wdeprecated-declarations]
                target->Set(className, f);
                        ^
/Users/pato/.node-gyp/12.4.0/include/node/v8.h:3358:3: note: 'Set' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version",
  ^
/Users/pato/.node-gyp/12.4.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
2 warnings generated.
  CXX(target) Release/obj.target/sha3/src/displayIntermediateValues.o
  CXX(target) Release/obj.target/sha3/src/KeccakF-1600-reference.o
  CXX(target) Release/obj.target/sha3/src/KeccakNISTInterface.o
  CXX(target) Release/obj.target/sha3/src/KeccakSponge.o
  SOLINK_MODULE(target) Release/sha3.node

> websocket@1.0.26 install /private/tmp/node12/node_modules/websocket
> (node-gyp rebuild 2> builderror.log) || (exit 0)

  CXX(target) Release/obj.target/bufferutil/src/bufferutil.o
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN node12@1.0.0 No description
npm WARN node12@1.0.0 No repository field.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: scrypt@6.0.3 (node_modules/scrypt):
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: scrypt@6.0.3 install: `node-gyp rebuild`
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: Exit status 1

+ web3@1.2.0
added 331 packages from 218 contributors and audited 27041 packages in 38.49s
found 1 low severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details
pato@pmbp:node12% cat package.json | jq .dependencies
{
  "web3": "^1.2.0"
}

This works because 1.2.0 uses scrypt.js 0.3.0, which includes scrypt as an optional dependencies. Optional dependencies are dependencies that npm tries to install and ignores them if their installation fails.

michaelsbradleyjr commented 5 years ago

Yes, we said the same thing. In any case, users won't see that anymore after web 1.2.1 is released, or whatever the next 1.x version is, because scrypt.js dependency will be gone.

alcuadrado commented 5 years ago

Sorry about that @michaelsbradleyjr, I think I read a previous comment on gmail. Thanks for working on this.

adrianmcli commented 5 years ago

I'm able to install properly on Node v12 when I use npm, but not yarn. Anyone else encounter this issue?

Make sure you are under the latest Node 12 and Yarn:

To test, run the following in a new directory:

npm init -y
yarn add web3

This is the output that I got:

yarn add v1.17.3
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
error web3-bzz@1.2.0: The engine "node" is incompatible with this module. Expected version ">=8.0.0 <=11.15.0". Got "12.7.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.
michaelsbradleyjr commented 5 years ago

It's because yarn enforces "engines" at any level in a project's dependency tree while npm only enforces at the top-level of the project (the project/repo you're working on), i.e. not with respect to its dependencies.

Better support for Node 12.x will be in the next release of web3 1.x.

See:

2938

https://github.com/ethereum/web3.js/blob/1.x/packages/web3/package.json#L7-L9

adrianmcli commented 5 years ago

@michaelsbradleyjr thanks, so we're just waiting for the patched release then?

michaelsbradleyjr commented 5 years ago

@adrianmcli I believe so, yes.

mkrufky commented 5 years ago

Hmm... Looks like I'm late to the game here. I Saw the build error in node 12 and fixed it myself before looking on github. I think you made the right choice here, but if anybody has reason for still using node-scrypt with node 12, I fixed it in my PR: https://github.com/barrysteyn/node-scrypt/pull/197

nickserv commented 5 years ago

This was fixed in #2938 (version 1.2.0).