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

Upgrade node to 8 and npm to 5.0.3 #2136

Closed
wants to merge 2 commits into from
Closed

Upgrade node to 8 and npm to 5.0.3 #2136

wants to merge 2 commits into from

Conversation

nLight
Copy link
Contributor

@nLight nLight commented Apr 24, 2017

New LTS version 8 is out \o/ why not using it?

I went ahead and updated all the places I could find.
I also had to fix one test (version dependent tests /o)
and a webpack config bug

Dependencies

#2148
#2153
🕐 d2iq-archive/reactjs-components#406

@d2iq-mergebot
Copy link

This repo has @mesosphere-mergebot integration. You can interact with the following commands.

@mesosphere-mergebot integrate  
@mesosphere-mergebot ship-it  
@mesosphere-mergebot test  

@nLight
Copy link
Contributor Author

nLight commented Apr 24, 2017

Looks like our Jenkins should be adjusted

@mlunoe
Copy link
Contributor

mlunoe commented Apr 26, 2017

There is a bug in node 6 that makes jest not run and proposed solution is to downgrade to node 5.11.1: webpack/webpack#2463 (comment)

Sad face 😢

@mlunoe
Copy link
Contributor

mlunoe commented Apr 27, 2017

I could not get it to work with node 6 :(

@mlunoe
Copy link
Contributor

mlunoe commented Apr 27, 2017

However, I did put up a PR for prettier in reactjs-components: d2iq-archive/reactjs-components#402

@nLight
Copy link
Contributor Author

nLight commented Apr 27, 2017

Does it work with node 7?
Since node is just a build tool, meaning we bundle the app and don't run anything on node.js in production, for us we could go with a non LTS release.

Anyways, this is not a priority, because of the reason I mentioned above, let's move forward with more important PRs rather then investing in this one right now. 🙏

@mlunoe
Copy link
Contributor

mlunoe commented Apr 27, 2017

Yup, I think node 7 is the way to go. Did not try it out though

@nLight
Copy link
Contributor Author

nLight commented May 2, 2017

Another proposed solution was to upgrade babel to 6.10.x which looks like the right thing to do choosing between downgrading to node 5 (non-LTS) and upgrading babel to a recent version :)

@nLight nLight mentioned this pull request May 2, 2017
@weblancaster
Copy link
Contributor

@nLight let's just use Node 7+, as you noted we are only using as a tool and not shipping to prod.

@nLight
Copy link
Contributor Author

nLight commented May 2, 2017

@weblancaster yep, still we should upgrade babel :)

@weblancaster
Copy link
Contributor

@nLight yes

@weblancaster
Copy link
Contributor

@nLight #2148 worked greatly with node v7.8.0 npm 4.2.0 and my build was noticeably faster.

@mlunoe
Copy link
Contributor

mlunoe commented May 2, 2017

Ok, related:
I put up a PR to update reactjs-components that makes the prettier linting rule happy: #2153

@nLight nLight changed the title Upgrade node to 6.10.2 and npm to 3.10.10 Upgrade node to 7.10.0 and npm to 4.5.0 May 4, 2017
@nLight
Copy link
Contributor Author

nLight commented May 4, 2017

I observe strange bugs with this setup:
when running npm run build it errors out performing
./node_modules/.bin/gulp checkDependencies && npm install compression-webpack-plugin@latest saying Cannot find module internal/fs

Then trying to run npm shrinkwrap --dev it enters into an infinite loop

😩

@nLight nLight changed the title Upgrade node to 7.10.0 and npm to 4.5.0 Upgrade node to 6.10.2 and npm to 4.5.0 May 5, 2017
@nLight
Copy link
Contributor Author

nLight commented May 5, 2017

Turns out the loop of shrinkwrap was caused by npm run shrinkwrap task that calls npm shrinkwrap --dev I suspect npm 4 now doesn't require run keyword for overrides this is why it used to work with npm 3 and stopped with npm 4.

Moreover I think we don't need fixShrinkwrap anymore as new setup doesn't depend on fsevent and there's no need in removing it from the npm-shrinkwrap.json

@orlandohohmeier orlandohohmeier added this to the 1.11 milestone May 19, 2017
@nLight nLight changed the title Upgrade node to 6.10.2 and npm to 4.5.0 Upgrade node to 8 and npm to 5.0.3 Jun 8, 2017
@nLight
Copy link
Contributor Author

nLight commented Jul 21, 2017

Closing this one for now

@nLight nLight closed this Jul 21, 2017
@nLight nLight deleted the drozhkov/node-6 branch July 21, 2017 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants