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

fsevents crashes build on NPM 3.10.9 #155

Open
elledienne opened this issue Nov 23, 2016 · 1 comment
Open

fsevents crashes build on NPM 3.10.9 #155

elledienne opened this issue Nov 23, 2016 · 1 comment

Comments

@elledienne
Copy link

There's a bug in NPM versions 3.10.8 and 3.10.9 that might crash the build process if npm-shrinkwrap is used.

One of the suggested solutions is to add fsevents as optionalDependencies

"optionalDependencies": {
    "fsevents": "*"
  }

And then install the modules with the flag --no-optional

npm install --no-optional

I was wondering if it could make sense to have to possibility to run the buildpack with that flag somehow (maybe env variable?)

@yourcelf
Copy link
Collaborator

Looks like meteor is likely to be stuck on npm 3.10.8/3.10.9 for quite some time -- Nodejs 4, 6, and 7 all use those versions.

I'm considering what the cleanest design here would be. The build process involves two separate invocations of npm -- one to set up the environment that does the bundling (which includes dev dependencies), and one to install dependencies on the built slug (which uses --production to skip dev dependencies). It seems to me it could be confusing to add a single environment variable that controls flags in both places; but it could also be cumbersome to introduce two separate variables. This abstraction has a high danger of leaks. :)

I'd also be hesitant to add --no-optional as a default for everyone that doesn't opt-in to it, as this might expose bugs or disable features that people have in their npm packages with such dependencies.

I'm curious how widespread the impact for this is -- I know momentjs had an error with this (meteor/meteor#7958 ), but it appears to have been resolved upstream through changes in momentjs.

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

No branches or pull requests

2 participants