node version requirement downgradable?

[yinso]

• Lock version: 10.13.0

Looks like lock requires node version >=6.9.1. With npm it's a warning:

npm WARN engine [email protected]: wanted: {"node":">=6.9.1"} (current: {"node":"5.5.0","npm":"3.3.12"})

With yarn it's an error:

error [email protected]: The engine "node" is incompatible with this module. Expected version ">=6.9.1".

As this is a client-side module I wonder if we can get away with a lower node version requirement. Upgrading local dev node version is easy with node but not as easily with prod, so is it possible to downgrade the requirement? Thanks.

[luisrudge]

This restriction is in place because we need to support people forking this repo and running the tests. Currently, the minimum version that this works is 6.9.1 (nodejs LTS).

[iamjochem]

maybe the tests should embed a check for the node version? having the "engine" property in package.json unnecessarily breaks things for users who are consuming the published module [
via yarn at least].

I don't think consumers of the published module should be confronted with bogus restrictions/errors that are designed to "support" people who are hacking on the source.

As it stands I cannot use [email protected], a purely client-side package, because yarn refuses to install it due to engine restriction ... I've checked the yarn issues list, their take on this situation is "fix the module, we will not change yarn" ... yarn has a --ignore-engines flag but this is not persisted so it is a non-starter (every invocation of yarn will need to include --ignore-engines in order to mitigate the problem with auth0-lock) ... funnily enough some of the modules we install are actually meant to be run with/in node and in those instances we want the engine check!!

[iamjochem]

BTW: I think this can be tackled by changing package.json as follows (requires a new devDependency of "check-node-version"):

BEFORE:

{
  // <snip>
  "engines": {
    "node": ">=6.9.1"
  },
  "main": "lib/index.js",
  "scripts": {
    "start": "grunt dev",
    "build": "grunt build",
    "design": "grunt design",
    "dev": "grunt dev",
    "dist": "grunt dist",
    "prepublish": "cross-env BABEL_ENV=npm grunt dist",
    "precommit": "lint-staged",
    "lint": "eslint --ext .jsx,.js src/",
    "test": "cross-env BABEL_ENV=test zuul -- test/**/*.test.js",
    "test:browser": "cross-env BABEL_ENV=test zuul --local 8080 --disable-tunnel -- test/**/*.test.js",
    "test:cli": "cross-env BABEL_ENV=test mochify --extension=.jsx --transform=babelify test/**/*.test.js",
    "test:watch": "cross-env BABEL_ENV=test mochify --watch --extension=.jsx --transform=babelify test/**/*.test.js",
    "test:jest": "jest --coverage",
    "test:jest:watch": "jest --watch --coverage",
    "publish:cdn": "ccu",
    "release": "scripts/release.sh",
    "i18n:translate": "grunt dist; node scripts/complete-translations.js"
  },
  // <snip>
}

AFTER:

{
  // <snip>
  "main": "lib/index.js",
  "scripts": {
    "start": "grunt dev",
    "build": "grunt build",
    "design": "grunt design",
    "dev": "grunt dev",
    "dist": "grunt dist",
    "prepublish": "cross-env BABEL_ENV=npm grunt dist",
    "precommit": "lint-staged",
    "lint": "eslint --ext .jsx,.js src/",
    "test:check-node-ver": "check-node-version --node >=6.9.1",
    "test": "npm run test:check-node-ver && cross-env BABEL_ENV=test zuul -- test/**/*.test.js",
    "test:browser": "npm run test:check-node-ver && cross-env BABEL_ENV=test zuul --local 8080 --disable-tunnel -- test/**/*.test.js",
    "test:cli": "npm run test:check-node-ver && cross-env BABEL_ENV=test mochify --extension=.jsx --transform=babelify test/**/*.test.js",
    "test:watch": "npm run test:check-node-ver && cross-env BABEL_ENV=test mochify --watch --extension=.jsx --transform=babelify test/**/*.test.js",
    "test:jest": "npm run test:check-node-ver && jest --coverage",
    "test:jest:watch": "npm run test:check-node-ver && jest --watch --coverage",
    "publish:cdn": "ccu",
    "release": "scripts/release.sh",
    "i18n:translate": "grunt dist; node scripts/complete-translations.js"
  },
  // <snip>
}

[iamjochem]

An alternative approach could be to use 'pre' & 'post' publish hooks, e.g.:

{
  // <snip>
  "scripts": {
    // <snip>   
    "prepublish": "cross-env BABEL_ENV=npm grunt dist && cp -f package.json .package.json.orig && jq 'delpaths([["engines", "node"]])' .package.json.orig > package.json",
    "postpublish": "[[ -f .package.json.orig ]]] && mv -f .package.json.orig package.json",
  }
  // <snip> 
}
    
 

[iamjochem]

@luisrudge - )I am not sure you get notifications on closed issues, hopefully this mention does trigger a notification), please read my comments above and consider re-opening this issue.

[luisrudge]

we're removing the restrictions in this PR: #1043

[iamjochem]

@luisrudge ... super :-) thanks for taking the time and for acting quickly! much appreciated.

PS. I was surprised you decided not to implement the alternative ("check-node-ver" script), is it not workable or was it check the 'node version check' in place no longer worth it?

[luisrudge]

Not worth it I guess.. Too much trouble 😝