Add check-node-version #5520
Add check-node-version #5520
Conversation
| const NODE_VERSION = process.version.match(/^v(\d+)/)[1] | ||
|
|
||
| if (NODE_VERSION < 7) { | ||
| printAndExit(`> You're using Node.js with version less than 7 which is not supported. Please upgrade to Node.js >= 7.`) |
There was a problem hiding this comment.
I think there should be defined version 8 as minimal. Automated tests with TravisCI have also version 8 as minimum.
| @@ -1,4 +1,8 @@ | |||
| // @flow | |||
|
|
|||
| // Check node version before including other dependencies | |||
| import '../server/lib/check-node-version' | |||
There was a problem hiding this comment.
🤔 This should probably be used in next-server/server/next-server.js instead.
|
Agree with @HaNdTriX to not polluting code with version checking. Just define it in package.json |
|
Even if someone force installs it / npm only shows a warning we should show a clear message instead of erroring out with a random stacktrace |
|
@timneutkens i agree, but package.json is first place where people will search dependencies. even engine dependencies. like in Gemfile in Ruby |
|
I'm not saying we shouldn't add engines, we definitely should 👍 |
|
@timneutkens should we check just node or also package managers? minimum version for node is 7.5 an up, right? |
Based on this PR #5520 there should be `engines` definition in package.json as first warn. Why i choose Node 8 as minimum? @timneutkens said (https://github.com/zeit/next.js/pull/5520/files#r228330327) that next.js should work on version 7.5 but automated tests in TravisCI are with versions 8 and 10. Version 7 was development branch. I think only production ready should be recommended.
|
Closing this in favor of #5724 |
Fixes #5508