WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 10 months 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: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch needs-refresh
Focuses: Cc:
PR Number:

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 17 months ago.
44246.2.diff (352 bytes) - added by netweb 17 months ago.
44246.3.diff (1.3 KB) - added by azaozz 16 months ago.

Download all attachments as: .zip

Change History (15)

@netweb
17 months ago

#1 @azaozz
17 months 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
17 months 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
17 months 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.


17 months ago

@netweb
17 months ago

#5 in reply to: ↑ 2 @netweb
17 months 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
17 months 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
16 months ago

#7 in reply to: ↑ 6 ; follow-up: @azaozz
16 months 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
16 months 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
13 months ago

  • Milestone changed from 5.0 to 5.1

#10 @netweb
11 months ago

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

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

#11 @pento
10 months ago

  • Keywords needs-refresh added; commit removed

This needs a refresh to use check-engines.

#12 @pento
10 months 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.