WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 4 days ago

#44246 reopened enhancement

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

Reported by: netweb Owned by: azaozz
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: commit has-patch
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 4 weeks ago.
44246.2.diff (352 bytes) - added by netweb 4 weeks ago.
44246.3.diff (1.3 KB) - added by azaozz 5 days ago.

Download all attachments as: .zip

Change History (11)

@netweb
4 weeks ago

#1 @azaozz
4 weeks 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
4 weeks 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
4 weeks 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.


4 weeks ago

@netweb
4 weeks ago

#5 in reply to: ↑ 2 @netweb
4 weeks 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
4 weeks 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
5 days ago

#7 in reply to: ↑ 6 ; follow-up: @azaozz
5 days 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
4 days 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 👌

Note: See TracTickets for help on using tickets.