Update node version requirement

[iiroj]

PR adds the lowest required node version to package.json's engines.node to the bin entrypoint. The requirement currently comes from execa.

It also updates the CI environments to test with node versions 8, 10 and 12, because version 9 isn't really supported.

Replaces PR #525 as stale.

[codecov]

Codecov Report

Merging #646 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #646   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files          11       11           
  Lines         306      306           
  Branches       59       59           
=======================================
  Hits          302      302           
  Misses          4        4

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e24aaf2...47d5322. Read the comment docs.

[okonet]

We removed that some time ago to support users with Node 6 on the CI where the package would fail the installation. We do the node version validation with “please-upgrade-node”.

[DanielRuf]

But is NodeJS 6 even tested on CI currently? Normally this should live in package.json.

[iiroj]

@okonet Would something like this work then?

require('please-upgrade-node')(pkg, {
  exitCode: process.env.CI ? 0 : 1 // Do not fail with exit code 1 on CI
})

Although I'm not really sure why we wouldn't fail, given how that package doesn't really support Node v6.

[iiroj]

Ah, so if this package is a dev-dependency, running npm install might fail on the CI when dev-dependencies are also installed, when CI runs on Node v6.

With yarn, there's yarn --ignore-engines, but on npm there's nothing similar (See issue).

Maybe I'll move the engines field back to index.js.

[okonet]

if this package is a dev-dependency, running npm install might fail on the CI when dev-dependencies are also installed, when CI runs on Node v6.

Exactly.

[okonet]

Related issue #515

[iiroj]

I edited the commits to keep the current behaviour. The other changes are still valid, though, and required before updating devDependencies.

[okonet]

🎉 This PR is included in version 9.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[IgnusG]

Too bad, breaks legacy aws builds which require 8.10

[iiroj]

@IgnusG: if it’s possible for you to test this package manually and confirm it still works, we can downgrade the requirement. It’s just that execa specifies this version in their package.json, so even if lint-stage didn’t it might still not work.

[DanielRuf]

Should be ok that it requires a newer NodeJS version.

This PR is included in version 9.0.1

Not directly a major version but you should be fine with 8.x.x.

[IgnusG]

9.0.0 works fine for me

[DanielRuf]

9.0.0 works fine for me

Sure but then you should make sure that you pin it to this version in package.json as the default SemVer selectors ^ and ~ will pull a newer version when it is not properly pinned.

[IgnusG]

9.0.0 works fine for me

Sure but then you should make sure that you pin it to this version in package.json as the default SemVer selectors ^ and ~ will pull a newer version when it is not properly pinned.

Yap, already done ;)

[artem-zakharchenko]

Imho bumping a required version of Node is at least a breaking change, and should've been communicated as such (warnings in previous release, deprecation in the next one). As lint-staged begins to throw in applications operating with the < 8.12 version of Node, while being updated to the next patch version.

[iiroj]

@artem-zakharchenko I agree on principle, but figured this just syncing to the correct requirement based on dependencies.

If execa works on lower version, should we also open an issue there?