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

fix: roll back node-abi until lgeiger/node-abi#90 is resolved #118

Merged
merged 1 commit into from
Nov 10, 2020

Conversation

emadum
Copy link
Contributor

@emadum emadum commented Nov 3, 2020

Description

electron/node-abi#90 currently breaks our prebuilds, this PR rolls it
back to a working version directly in the package-lock.json as a
temporary fix until the issue is resolved.

What changed?

@emadum emadum changed the title fix: temporarily roll back until lgeiger/node-abi#90 is resolved fix: roll back node-abi until lgeiger/node-abi#90 is resolved Nov 3, 2020
@emadum emadum requested review from mbroadst and nbbeeken November 3, 2020 20:34
"version": "2.19.1",
"resolved": "https://registry.npmjs.org/node-abi/-/node-abi-2.19.1.tgz",
"integrity": "sha512-HbtmIuByq44yhAzK7b9j/FelKlHYISKQn0mtvcBrU5QBkhoCMp5bu8Hv5AI34DcKfOAcJBcOEMwLlwO62FFu9A==",
"version": "2.18.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the change to package.json? We should pin the version there, not just the lock file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't depend on it directly, it's a dependency of prebuild.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it not possible to explicitly pin it in our package.json even if its a transient dependency? I'm concerned that the next time someone runs npm install it will "fix" this and revert your change

Copy link
Contributor Author

@emadum emadum Nov 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's certainly possible to add it to our package.json but I don't think it's necessarily the correct solution since it's not a direct dependency. Regarding your concern, subsequent npm installs should pull the version from package-lock.json, someone would have to delete the current package-lock.json before running an npm install, or explicitly run npm install node-abi@latest, in order to update the version in the lock file.

I'd also hope the underlying issue with node-abi will be resolved by then.

@emadum emadum requested a review from mbroadst November 5, 2020 15:08
@emadum emadum merged commit 880ae2e into master Nov 10, 2020
@mbroadst mbroadst deleted the rollback-node-abi branch November 11, 2020 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants