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 missing babel-helper-evaluate-path dependency #963

Closed
wants to merge 1 commit into from
Closed

Add missing babel-helper-evaluate-path dependency #963

wants to merge 1 commit into from

Conversation

feross
Copy link

@feross feross commented Aug 15, 2019

At the moment, the babel-plugin-minify-guarded-expressions package is calling require('babel-helper-evaluate-path') without actually depending on this package. It only works right now because of package hoisting.

An exception is thrown when npm does not flatten the dependency tree. You can trigger this to verify it by forcing npm to install packages without flattening (the npm@2 nested node_modules approach) by running: npm install --legacy-bundling

At the moment, the `babel-plugin-minify-guarded-expressions` package is calling `require('babel-helper-evaluate-path')` without actually depending on this package. It causes an exception to be thrown when the npm does not flatten the dependency tree. You can trigger it by forcing npm to install packages without flattening (like it did in npm@2) by running: npm install --legacy-bundling
@feross feross requested a review from boopathi as a code owner August 15, 2019 07:44
@feross
Copy link
Author

feross commented Aug 15, 2019

Note: There might be other instances of this. This is the first one I found that crashed babel-minify.

@hzoo
Copy link
Member

hzoo commented Aug 15, 2019

Thanks for the ping online @feross. Ah looks like there was a previous PR for this, so goin to merge that one! #957

@hzoo hzoo closed this in d525b6c Aug 15, 2019
@feross feross deleted the patch-1 branch August 15, 2019 21:41
@feross
Copy link
Author

feross commented Aug 15, 2019

Great, thanks!

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