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

Add node-gyp to dependencies #19

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sammko
Copy link

@sammko sammko commented Oct 4, 2023

Recently npm has stopped bundling the node-gyp-bin directory (npm/cli#6554). But yarn classic uses this location to find npm's bundled node-gyp. Yarn detects that event-loop-stats uses node-gyp in its lifecycle script but does not specify it as a dependency, so it tries to ensure that node-gyp is installed. As it can't find the npm's bundled one, it instead tries to install it globally into its own store (equivalent to yarn global add node-gyp). But this triggers a yarn bug (yarnpkg/yarn#3728), where it doesn't then proceed with the rest of the installation correctly and doesn't generate a yarn.lock and in fact run lifecycle scripts on our own package (prepare, postinstall).

@sammko
Copy link
Author

sammko commented Oct 4, 2023

On second thought, I'm not sure this is the right approach, as node-gyp and its 27MiB of dependencies will be transitively present in users' node_modules. It does seem to be the recommended approach by yarn, though.

@bripkens
Copy link
Owner

bripkens commented Oct 5, 2023

Thank you for the PR @sammko. TBH I haven't followed the state of things node-gyp related and I am therefore unaware of changed best practices. Given my lack of additional insight, I would therefore keep this PR open until additional insight (in the form of comments/articles) emerges. I hope you understand.

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.

2 participants