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

feat: update minimum required node.js version to 4.8 for travis CI #82

Merged
merged 1 commit into from
Sep 30, 2017

Conversation

alexander-akait
Copy link
Member

Based on webpack-contrib/file-loader#183 (comment).
Issue around npm - npm/npm#18395.
From slack:

sokra [12:29 PM] 
we are no longer chained to 4.3. You can use that lastest 4.x (node.js version currently in maintenance mode.
4.3 was because of AWS, but they upgraded to node 6.x so this is no longer an issue.

.babelrc Outdated
@@ -5,7 +5,7 @@
{
"useBuiltIns": true,
"targets": {
"node": "4.3"
"node": "4.8"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>= 4 (if babel-preset-env (browserslist) supports it) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to keep in mind here. I still think the minimum needs to be a static value as opposed to >= 4 ( for both preset-env & all the engines objects in the pacakge.json ).

Thinking being you will have an ever increasing minimum node version which would have a less than optimal UX.

It's also probably worth considering finding the lowest viable version number to set as the minimum. i.e. 4.6 as opposed to the latest NodeJS minor version. I'm sure there are plenty of people that aren't on 4.8 yet ( for reasons I will never comprehend )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the comment on that thread by the npm staff, the earliest supported npm node version is 4.7.

Would webpack packages using this change technically require a major version bump if we're no longer testing / supporting older node 4.x versions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m afraid people who aren’t on 4.8 aren’t on 4.6 either ;-| But I don’t know how to prove either of these points.

Copy link
Member

@joshwiens joshwiens Sep 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that Node 8 is about to go LTS, I don't know if I feel terribly bad about pushing up the minimum 4.x version.

To @mattlewis92's point, following the strictest letter of the lay, yes it should be a major as it is breaking based on what is currently supported.

Give Webpack follows the LTS schedule of NodeJS & v8 goes LTS this October, the least disruptive path for the community may be to just drop 4.x when v8 officially takes over as the latest LTS.

In the mean time, the Travis config can be modified to accomodate the issue with 4.3 & npm@5

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d3viant0ne just change only travis configuration?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Buffer related backports landed in v4.5.0, no idea about the if and when V8 semver minor update(s) happend in the node v4+ line. Normally within a major range it is mandatory to update to the latest minor/patch whenever possible (security etc pp). If for any corporate reasons this isn't possible, imho at some point it's just their problem and getting webpack to run is one of the smaller ones they have then 😛

@alexander-akait
Copy link
Member Author

@sapegin @michael-ciniawsky @mattlewis92 @d3viant0ne let's update only travis configuration for avoid problem with npm@5

@alexander-akait alexander-akait changed the title feat: update minimum required node.js version to 4.8 feat: update minimum required node.js version to 4.8 for travis CI Sep 12, 2017
@brianhelba
Copy link

@evilebottnawi @mattlewis92 Can this be merged? I'd like to pull it downstream to fix failing tests in https://github.com/webpack-contrib/file-loader . See webpack-contrib/file-loader#204

@joshwiens joshwiens merged commit ff4faf7 into master Sep 30, 2017
@brianhelba
Copy link

@d3viant0ne Can you please publish a new release of this repo, so we can pull the changes downstream to other projects?

@joshwiens
Copy link
Member

Just did

@joshwiens
Copy link
Member

webpack-defaults@1.6.0

@brianhelba
Copy link

Awesome, thanks so much!

@michael-ciniawsky michael-ciniawsky deleted the update-nodejs-version branch September 30, 2017 00:52
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

6 participants