scrypt: move to nan ^2.14.0; fix node 12+ support #197
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mkrufky
changed the title
|
@barrysteyn can we merge this, scrypt is a direct dependency for a number of packages and this would stop breaking |
Open
|
Hi, I’ll merge it tomorrow
|
|
would be nice to merge this! |
|
Bump @barrysteyn ! |
mkrufky
changed the title
|
@barrysteyn any chance to get this merged soon? |
|
Hi all, sorry for taking so long. I was not aware that people are still using this project. Would you like me to publish a version with this merge? |
|
An npm release would be appreciated @barrysteyn |
|
Okay, will do it today then. A major version bump? |
|
Probably the best to avoid issues for people on the current release. |
|
I don't think it should be a major version bump, since the PR does not change the public API of node-script. I would just do a minor version bump. It does no harm for users of older versions to take this upgrade. |
|
okay, will update at the end of my day (I am on PST timezeone). |
|
Thanks!
…On Mon, May 11, 2020 at 4:22 PM Barry Steyn ***@***.***> wrote:
okay, will update at the end of my day (I am on PST timezeone).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#197 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIQBH5CY6ODTKDGW66DGQODRRBM7TANCNFSM4ILJOJYQ>
.
|
|
Sorry guys, I was having a bit of trouble updating things. I do intend to publish soon though. Quick question for anyone out there: I was under the impression that Node provides Scrypt encryption in it's own core libraries. If so, why are people still using this? |
|
There are various existing node.js modules that depend on node-script that have not been rewritten to use other solutions. node-script still works fine when used with node.js older than 12, but cannot build with later versions of node.js. Applying this fix is a fine solution, and there is no reason to rewrite all of those other modules if this patch fixes node-script. |
|
Yes, for instance drizzle uses this, and this version of scrypt the only breaking dependency for many projects on npm. |
|
Okay, cool. I wasn't really aware of it's popularity :) |
|
Hi, yes. I am trying to install drizzle and unfortunately it is not working because of the node-scrypt dependency :( Can you please fix this? |
|
On second thought, a major version bump is indeed a better idea - I didn't realize that the last release was ~4 years ago. nodejs/nan itself has dropped support for some older versions of node.js since then, so better to to do a major version bump. |
|
Okay... I am struggling with gettting my npm key from old pc (which is not
working). I am about to take out the harddrive to get at it. Sorry this is
taking so long everyone.
…On Tue, May 19, 2020 at 9:59 AM Michael Ira Krufky ***@***.***> wrote:
On second thought, a major version bump is indeed a better idea - I didn't
realize that the last release was ~4 years ago. nodejs/nan itself has
dropped support for some older versions of node.js since then, so better to
to do a major version bump.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#197 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALXKRDN3X4VNG2SRPBDNNLRSK3ILANCNFSM4ILJOJYQ>
.
|
|
It's great to see this fix merged. Would it be possible to push the new version to npm? Cheers! |
|
for now, this fixed the not-building bug, but it'd be good to see the updates in the npm repo: $ npm install https://github.com/barrysteyn/node-scrypt/ cheers for all the good work, @barrysteyn |
|
Any chance of getting a new version of this out? There are several blockchain-related projects that still have this as a dependency and it would be really nice to not have to use the head as the version. Thanks! |
3 tasks
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
Mar 11, 2021
Updated all packages with their engines locked to 10 to be >= 10. Updated dependency on old phone-number-privacy module that still had its engine locked to 10. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version) seemed like the most concise way of avoiding a dependency on scrypt.
This was referenced Mar 12, 2021
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
Mar 15, 2021
Updated all packages with their engines locked to 10 to be >= 10. Updated dependency on old phone-number-privacy module that still had its engine locked to 10. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version) seemed like the most concise way of avoiding a dependency on scrypt.
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
Apr 1, 2021
Updated all packages with their engines locked to 10 to be >= 10. Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. The new phone-number-privacy-common module updated its sdk components to version 1.0.2 and so to maintain compatability the remaining phone-number-privacy modules needed to update their sdk components to the same version. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version) seemed like the most concise way of avoiding a dependency on scrypt.
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
Apr 8, 2021
Updated all packages with their engines locked to 10 to be >= 10. Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. The new phone-number-privacy-common module updated its sdk components to version 1.0.2 and so to maintain compatability the remaining phone-number-privacy modules needed to update their sdk components to the same version. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version) seemed like the most concise way of avoiding a dependency on scrypt.
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
Apr 14, 2021
Updated all packages with their engines locked to 10 to be >= 10. Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. The new phone-number-privacy-common module updated its sdk components to version 1.0.2 and so to maintain compatability the remaining phone-number-privacy modules needed to update their sdk components to the same version. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version) seemed like the most concise way of avoiding a dependency on scrypt.
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
Apr 22, 2021
Updated all packages with their engines locked to 10 to be >= 10. Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. The new phone-number-privacy-common module updated its sdk components to version 1.0.2 and so to maintain compatability the remaining phone-number-privacy modules needed to update their sdk components to the same version. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version) seemed like the most concise way of avoiding a dependency on scrypt.
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
May 6, 2021
Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version 0.6.3 -> 0.6.4) seemed like the most concise way of avoiding a dependency on scrypt. Unfortunately the resolution didn't work because the code of ganache-core dynamically checked to see that the loaded ethereumjs-wallet version matched the version defined in its package.json (0.6.3) and if not attempted to use a bundled version of ganache-core, which didn't exist and would cause the build to fail. So instead I introduced a new version of ganach-core that depends on ethereumjs-wallet 0.6.4 and used a resolution to ensure references to ganache-core use the updated version.
piersy
added a commit
to celo-org/celo-monorepo
that referenced
this pull request
May 13, 2021
* Update packages to be compatible with node 12 Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version 0.6.3 -> 0.6.4) seemed like the most concise way of avoiding a dependency on scrypt. Unfortunately the resolution didn't work because the code of ganache-core dynamically checked to see that the loaded ethereumjs-wallet version matched the version defined in its package.json (0.6.3) and if not attempted to use a bundled version of ganache-core, which didn't exist and would cause the build to fail. So instead I introduced a new version of ganach-core that depends on ethereumjs-wallet 0.6.4 and used a resolution to ensure references to ganache-core use the updated version. * Update CI config to use node 12 Added dockerfile for node-12 CI build image and updated the circleci config to use the new docker image for builds. Parameterised and bumped node modules cache version in order to clean the cache for the CI build. * Update docs Make Node.js v12.x the only supported version, as opposed to suggesting we support lower versions as well in other places. Standardise to use the string "Node.js v12.x" in all places which should make updating Node.js versions simpler in the future.
jklmli
added a commit
to 0xProject/website
that referenced
this pull request
May 25, 2021
A [PR was merged in](barrysteyn/node-scrypt#197) last year to add support for Node 12+. The maintainer never published a new version on NPM though, so this fix isn't doing much. This commit instead pins the version of scrypt to the latest git revision, which includes the fix. This might mean that we miss out on future updates, but the module is [already deprecated](https://github.com/barrysteyn/node-scrypt#scrypt-for-node) so the risk of that is low.
jklmli
added a commit
to 0xProject/website
that referenced
this pull request
May 25, 2021
A [PR was merged in](barrysteyn/node-scrypt#197) last year to add support for Node 12+. The maintainer never published a new version on NPM though, so this fix isn't doing much. This commit instead pins the version of scrypt to the latest git revision, which includes the fix. This might mean that we miss out on future updates, but the module is [already deprecated](https://github.com/barrysteyn/node-scrypt#scrypt-for-node) so the risk of that is low.
medhakothari
pushed a commit
to celo-org/celo-monorepo
that referenced
this pull request
Jun 3, 2021
* Update packages to be compatible with node 12 Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version 0.6.3 -> 0.6.4) seemed like the most concise way of avoiding a dependency on scrypt. Unfortunately the resolution didn't work because the code of ganache-core dynamically checked to see that the loaded ethereumjs-wallet version matched the version defined in its package.json (0.6.3) and if not attempted to use a bundled version of ganache-core, which didn't exist and would cause the build to fail. So instead I introduced a new version of ganach-core that depends on ethereumjs-wallet 0.6.4 and used a resolution to ensure references to ganache-core use the updated version. * Update CI config to use node 12 Added dockerfile for node-12 CI build image and updated the circleci config to use the new docker image for builds. Parameterised and bumped node modules cache version in order to clean the cache for the CI build. * Update docs Make Node.js v12.x the only supported version, as opposed to suggesting we support lower versions as well in other places. Standardise to use the string "Node.js v12.x" in all places which should make updating Node.js versions simpler in the future.
medhakothari
pushed a commit
to celo-org/celo-monorepo
that referenced
this pull request
Jun 3, 2021
* Update packages to be compatible with node 12 Updated dependencies on old phone-number-privacy-common module that still had its engine locked to 10. Resolved problem with scrypt not building on node > 10. The dependency on scrypt was as follows: ethereumjs-wallet -> scrypt.js -> scrypt Although a pr was raised to fix scrypt (barrysteyn/node-scrypt#197) the module owner has lost his npm key and can't publish an updated version. scrypt.js still depends on scrypt in the latest version, so a resolution to override the version of ethereumjs-wallet (with a resolution for a more recent version 0.6.3 -> 0.6.4) seemed like the most concise way of avoiding a dependency on scrypt. Unfortunately the resolution didn't work because the code of ganache-core dynamically checked to see that the loaded ethereumjs-wallet version matched the version defined in its package.json (0.6.3) and if not attempted to use a bundled version of ganache-core, which didn't exist and would cause the build to fail. So instead I introduced a new version of ganach-core that depends on ethereumjs-wallet 0.6.4 and used a resolution to ensure references to ganache-core use the updated version. * Update CI config to use node 12 Added dockerfile for node-12 CI build image and updated the circleci config to use the new docker image for builds. Parameterised and bumped node modules cache version in order to clean the cache for the CI build. * Update docs Make Node.js v12.x the only supported version, as opposed to suggesting we support lower versions as well in other places. Standardise to use the string "Node.js v12.x" in all places which should make updating Node.js versions simpler in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Node.js 12 bundles a version of v8 with lots of breaking changes, with more coming.
As such, things were moved around in NAN.
This updates node-scrypt to the latest NAN and fixes the build breakage under Node 12.
Also, fix all remaining build warnings to future-proof upcoming v8 changes.
closes #193