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 core-js build when using npm 3 #51

Merged
merged 1 commit into from
Sep 28, 2015

Conversation

jthomaschewski
Copy link
Contributor

Don't rely on core-js in babel-core directory but add it as top level dependency.
Fixes build after npm install with npm 3 and later.

See #50

@AdamBrodzinski
Copy link
Collaborator

👍 Cool I never got around to using NPM 3 yet... any major hurdles in general?

@jthomaschewski
Copy link
Contributor Author

My linux distro (Arch) upgraded the official repositories to nodejs 4.1 and npm 3.
As I got deprecation warnings in the past with NPM 2, I decided to remove the "node_modules" directory and do a clean npm install - and only got some errors because core-js was missing.

After applying the changes of this pull request and removing the previously build lib/webpack/core-js* files, everything was fine again.

One thing: For one of my dependencies, react-sweetalert, I had to add the sweetalert package to my dependencies as it is declared as a peer dependency by the package author.
There will be some more packages out there with peerDependencies declared.

Fortunately NPM3 still warns about unmet peerDependencies in the install tree which is displayed when doing "npm install"

@AdamBrodzinski
Copy link
Collaborator

@JBBr cool NPM3 and 0.14 should help with double loading React!

@jedwards1211
Copy link
Owner

Hmmmmm...I want to figure out a solution that will work out of the box with npm 2 or npm 3, so that we don't have to complicate the requirements...I'll have to investigate if there are npm commands I can use in the script to figure out where core-js lives. Since I'm pretty busy this week, if you guys have any idea feel free to let me know.

@jedwards1211
Copy link
Owner

Wait, I'm thinking too hard. All I have to make the script do is query the npm version and pick the paths based upon whether it's 2 or 3. Duh :)

@jthomaschewski
Copy link
Contributor Author

The fix just adds core-js as a core dependency which ensures that it's available in node_modules.
This shouldn't break compatibility with npm 2

@jthomaschewski
Copy link
Contributor Author

I'll try it in a docker container or use nvm - don't see why it shouldn't work with npm2.
Only question: Does core-js version have to match the core-js version used for babel build or something like that? Or is it completely independent?
If that's somehow important, checking core-js version when updating babel in package.json will be important.

@jedwards1211
Copy link
Owner

@JBBr ohhhh gotcha. I don't know for sure if it has to match. It would depend on what babel-transpiled code invokes in the core-js polyfill. If anything in the polyfill changes then I suppose it could be a problem. Obviously we have to break support for anything that uses the Number constructor polyfill, but in this case if there's a mismatch in any part of the polyfill and the transpiler, there might be a problem.

@jthomaschewski
Copy link
Contributor Author

I verified NPM 2 compatibility by running my branch in a docker container with NPM 2.14.4 and node 4.1.1. I haven't had any problems.

@AdamBrodzinski
Copy link
Collaborator

Just used this to get around my accidental NPM install... lol. works well.

jedwards1211 added a commit that referenced this pull request Sep 28, 2015
Fix core-js build when using npm 3
@jedwards1211 jedwards1211 merged commit faf1341 into jedwards1211:master Sep 28, 2015
@jedwards1211
Copy link
Owner

Cool, thanks guys!

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.

None yet

3 participants