Make WordPress Core

Opened 7 years ago

Last modified 6 years ago

#44246 reopened enhancement

Add `check-node-version` to check required Node.js and npm versions are installed

Reported by: netweb's profile netweb Owned by: azaozz's profile azaozz
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch needs-refresh
Focuses: Cc:

Description

This change adds a npm script to the preinstall task of npm install to check that the required npm and Node.js versions are installed and being used in the current terminal shell instance. If the Node.js version is not 8.9.3 or npm version is not greater than 6.1.0 a warning will be displayed notifying the user to update the respective build tooling.

The attached patch depends on #44245 being committed first

Related: #44245, #43731

Attachments (3)

44246.diff (224.6 KB) - added by netweb 7 years ago.
44246.2.diff (352 bytes) - added by netweb 7 years ago.
44246.3.diff (1.3 KB) - added by azaozz 6 years ago.

Download all attachments as: .zip

Change History (15)

@netweb
7 years ago

#1 @azaozz
7 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 43321:

Build tools: add check-node-version to check required Node.js and npm versions are installed.

Props netweb.
Fixes #44246.

#2 follow-up: @ocean90
7 years ago

  • Keywords needs-patch added; has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

You can't use the dev dependency check-node-version in preinstall unless it's installed globally.

#3 @azaozz
7 years ago

In 43322:

Build tools:

  • Remove check-node-version from package.json for now. Throws errors.
  • Minor fixes to package-lock.json, http => https.

See #44246.

This ticket was mentioned in Slack in #core-committers by azaozz. View the logs.


7 years ago

@netweb
7 years ago

#5 in reply to: ↑ 2 @netweb
7 years ago

Replying to ocean90:

You can't use the dev dependency check-node-version in preinstall unless it's installed globally.

Good catch, I've added 44246.2.diff that moves the task to postinstall, this way the dependancy is installed before the script is called.

It's not ideal as after updating node/npm removing node_modules and running npm install should probably occur, I'll investigate options to display a notice of some form in a follow up ticket

#6 follow-up: @netweb
7 years ago

Via Slack:

maybe we can run check-node-version from grunt and output just a "message"?

We could use https://www.npmjs.com/package/grunt-run to run check-node-version then use grunt.log.writeln to display anything extra not already displayed by check-node-version message

or actually can be a warning so it stops

Patch 44246.2.diff would achieve this, though no message would be displayed other than the node/npm warning message displayed by check-node-version:

Error: Wanted npm version >= 6.1.0 (>=6.1.0)
To install npm, run npm install -g npm@>= 6.1.0

@azaozz
6 years ago

#7 in reply to: ↑ 6 ; follow-up: @azaozz
6 years ago

Replying to netweb:

Patch 44246.2.diff would achieve this

Right. We can also run this from grunt and just show a "non-blocking" message. See 44246.3.diff: output a non-blocking warning grunt.log.writeln() when node or npm need updating.

#8 in reply to: ↑ 7 @netweb
6 years ago

  • Keywords commit has-patch added; needs-patch removed

Replying to azaozz:

Right. We can also run this from grunt and just show a "non-blocking" message. See 44246.3.diff output a non-blocking warning grunt.log.writeln() when node or npm need updating.

Nice 👌

#9 @jorbin
6 years ago

  • Milestone changed from 5.0 to 5.1

#10 @netweb
6 years ago

Also see https://github.com/WordPress/gutenberg/pull/12721

tl;dr check-engines is being added to @wordpress/scripts package

#11 @pento
6 years ago

  • Keywords needs-refresh added; commit removed

This needs a refresh to use check-engines.

#12 @pento
6 years ago

  • Milestone changed from 5.1 to Future Release

wp-scripts brings puppeteer, which is a hefty add-on just to check the node version. I don't want to bring in puppeteer until we're actually using it, so let's leave this until after e2e tests land. See #45165.

Note: See TracTickets for help on using tickets.