typicode / please-upgrade-node Goto Github PK

If you were to use the semver package, you could easily have full semver range syntax support.

Example scenario:

• user on Node.js v10.24.1
• package foo: engines.node=>10

The field can be any valid semver range.

Expected behavior:

error:

foo requires at least version 11.0.0 of Node, please upgrade

Actual behavior

Version test succeeds

Failing test-case: 99b6ebd

I would think that require('please-upgrade-node')('./your-package-root') would be more appropriate, than a nested require, this way the test can run, then the please-upgrade can simply require(...) on its' own.

If you do this, there's no need to restrict syntax availability for the nested package.

I'd like to suggest that instead of calling process.exit(), we throw an Error.

As described in node's documentation for process.exit(), using this method is problematic because it disrupts all asynchronous work in progress. The documentation recommends using an error instead:

If it is necessary to terminate the Node.js process due to an error condition, throwing an uncaught error and allowing the process to terminate accordingly is safer than calling process.exit().

One concern with making this change would be preserving the functionality of opts.exitCode. I believe we can set process.exitCode right before throwing the error, and it should preserve the intended behavior.

If the maintainers are happy with this idea, I can contribute the change in a PR.

First of all, great little module. I would very much like to use this.

So, I stopped using the engines field on my projects quite a while ago, in favor of an .nvmrc file with the required Node version in it. A simple nvm install or nvm use picks that up and understands what version is needed, which I find very useful. Since I don't want to maintain the right Node version number in 2 places (.nvmrc and package.json), I stopped mentioning it in package.json.

It seems like your module could benefit from supporting .nvmrc on top of the engines field in package.json. What do you think?

This line:

var requiredVersion = pkg.engines.node.replace(">=", "");
var currentVersion = process.version.replace("v", "");
if (currentVersion < requiredVersion) { // oops!
  ...
}

…is an error.

'6' < '10.0.0' === false

You cannot use string comparison here, which sorts lexicographically. You'll need a bit more logic to compare version number strings.

If I try the try the suggested usage require('please-upgrade-node')(), it fails with the following error:

E:\Projects\repos\cosmiconfig\node_modules\please-upgrade-node\index.js:4
  var requiredVersion = pkg.engines.node.replace(">=", "");
                                   ^

TypeError: Cannot read property 'node' of undefined
    at module.exports (E:\Projects\repos\cosmiconfig\node_modules\please-upgrade-node\index.js:4:36)
    at Object.<anonymous> (E:\Projects\repos\cosmiconfig\index.js:2:31)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (E:\Projects\repos\cosmiconfig\test\caches.test.js:6:19)

I added a couple of console.log statements to check what was going wrong. It turns out, please-upgrade-node loads it's own package.json and errors out because it does not have the engines field.

i use NVM
nvm use 10.0.0 （my default is 8.0.0）
process.verison is return 8.0.0

i can just set deffrent default version in deffrent project
it is too complicated