Make WordPress Core

Opened 9 months ago

Closed 5 months ago

Last modified 4 months ago

#59663 closed task (blessed) (fixed)

Bump the minimum required version of Node.js to 20.x

Reported by: desrosj's profile desrosj Owned by:
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: needs-patch add-to-field-guide
Focuses: Cc:

Description (last modified by desrosj)

Follow up to #56658.

Node.js 18.x is now available on the .org build server, so Core and Gutenberg can now look to start utilizing 18.x.

Version 16 has reached end of life on September 11, 2023, 7 months ahead of schedule.

Version 18 is the current active LTS version until October 18th, 2023. It will then enter maintenance LTS status until April 30, 2025.

Let's update the minimum version required to at least 18.x where we can safely remain until there's a need to update further.

Also see the related dev note.

Change History (39)

#1 @desrosj
9 months ago

  • Description modified (diff)
  • Keywords has-patch added

#3 @desrosj
9 months ago

At first glance, it seems Core itself can update fairly easily with no problems. The default themes, however, will require a bit of investigation and work.

#4 @benharri
9 months ago

Just tested your patch and first glance seems good.

npm install and build for the three bundled themes that have a package.json all succeeded.

There's no node/npm version specified in the themes' package.json.

Last edited 9 months ago by benharri (previous) (diff)

#5 @dhrupo
9 months ago

The patch is working fine.

I just tested for 4 themes and 3 plugins package.json, and all succeeded.

This ticket was mentioned in PR #5672 on WordPress/wordpress-develop by @desrosj.


8 months ago
#6

This updates all Twenty Twenty-One dependencies to their latest versions and run npm audit fix.

The one exception to this is stylelint, which has only been updated to 14.16.1, the latest version of that major branch. The stylelint related dependencies maintained upstream in WordPress/gutenberg do not yet seem to support 15.x.

This update is primarily to get TT1 to a state where there are no errors when running install and build tasks on Node.js 18.x. Running npm install with 18.x currently produces the following output:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@es-joy/jsdoccomment@0.10.8',
npm WARN EBADENGINE   required: { node: '^12 || ^14 || ^16' },
npm WARN EBADENGINE   current: { node: 'v18.18.2', npm: '9.8.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'eslint-plugin-jsdoc@36.1.1',
npm WARN EBADENGINE   required: { node: '^12 || ^14 || ^16' },
npm WARN EBADENGINE   current: { node: 'v18.18.2', npm: '9.8.1' }
npm WARN EBADENGINE }

These updates fix this issue.

Trac tickets:

@desrosj commented on PR #5672:


8 months ago
#7

@carolinan Do you mind giving this a once over? Mainly looking to get validation that these consolidated selectors will not be problematic.

The following errors are happening now when running build. This is likely due to that consolidated.

assets/css/ie-editor.css
 1237:43  ✖  Unexpected empty block  block-no-empty

assets/css/ie.css
 2665:43  ✖  Unexpected empty block  block-no-empty
 4135:43  ✖  Unexpected empty block  block-no-empty

@jorbin commented on PR #5672:


8 months ago
#8

One option to fix this is to use postcss-discard-empty, here is a patch that I tested which does so.

diff --git a/src/wp-content/themes/twentytwentyone/package.json b/src/wp-content/themes/twentytwentyone/package.json
index 6681a5defe..83b02a0bae 100644
--- a/src/wp-content/themes/twentytwentyone/package.json
+++ b/src/wp-content/themes/twentytwentyone/package.json
@@ -27,6 +27,7 @@
 		"postcss-css-variables": "^0.19.0",
 		"postcss-custom-media": "^10.0.2",
 		"postcss-discard-duplicates": "^6.0.0",
+		"postcss-discard-empty": "^6.0.0",
 		"postcss-focus-within": "^8.0.0",
 		"postcss-merge-rules": "^6.0.1",
 		"postcss-nested": "^6.0.1",
diff --git a/src/wp-content/themes/twentytwentyone/postcss.config.js b/src/wp-content/themes/twentytwentyone/postcss.config.js
index d889f314ba..3f674aee71 100644
--- a/src/wp-content/themes/twentytwentyone/postcss.config.js
+++ b/src/wp-content/themes/twentytwentyone/postcss.config.js
@@ -9,6 +9,7 @@ module.exports = {
 			precision: 0
 		}),
 		require('postcss-discard-duplicates'),
-		require('postcss-merge-rules')
+		require('postcss-merge-rules'),
+		require('postcss-discard-empty')
 	]
 };

@poena commented on PR #5672:


8 months ago
#9

To get the correct line numbers for the errors, one can use npm run build:stylelint
The empty block in all 3 reported errors, is

@media only screen and (max-width: 481px) {
}

I have confirmed that the styles are indeed still present in the files, consolidated, moved to a different media query. I would expect the empty block to be removed, but this does not seem to happen for the media queries.
If it can't be removed with the help of the tools, I support Aarons suggestion to discard the empty blocks.

@desrosj commented on PR #5672:


8 months ago
#10

Thanks all! I went with @aaronjorbin's patch above.

#11 @desrosj
8 months ago

In 57122:

Twenty Twenty-One: Update all dependencies.

In preparation for updating Core to use Node.js version 18.x, this updates the dependancies for the Twenty Twenty-One theme to the latest versions. This addresses an unsupported engine warning where packages within the dependency tree did not support Node.js > 16.x.

All changes to built files are included in this commit. These changes are a result of the following:

  • Identical sets of properties for multiple selectors are now consolidated into one.
  • The removal of a duplicate --branding--description--font-family definition.
  • The addition of the postcss-discard-empty dependency, which removes empty CSS rules within IE stylesheets after the previous consolidation is performed.
  • stylelint has only been updated to 14.x (15.x is the latest). This is because @wordpres/stylelint-config currently has a version constraint of ^14.2 and does not properly support 15.x.

The last change of note is the new configuration for the value-keyword-case rule in .stylelint-css.json. This was added as a way to prevent the currentColor from being changed to all lowercase.

Props jorbin, poena.
See #59663, #59658.

@desrosj commented on PR #5672:


8 months ago
#12

Merged in core.trac.wordpress.org/changeset/57122.

This ticket was mentioned in PR #5707 on WordPress/wordpress-develop by @desrosj.


8 months ago
#13

This PR works to determine what changes would be required in the 6.4 branch to update the version of Node.js from 16.x to 20.x.

Trac ticket: https://core.trac.wordpress.org/ticket/59663.

@desrosj commented on PR #5515:


8 months ago
#14

I've made a request to the systems team to install 20.x on the build server. It's possible we can skip 18.x all together, though I'd advise we proceed as though we'll only have 18.x available for this current cycle (WP 6.5). We can always bump a second time should something change.

@desrosj commented on PR #5515:


8 months ago
#16

@swissspidy It seems that this change will cause the comparison between the previous branch and this one to fail because of the differing versions of Node.js/npm between them.

I do plan to backport this update to the 6.4 branch because that is currently the only other one that uses 16.x (all previous ones use 14.x).

Will that then cause the workflow to fail in the 6.4 branch when comparing the 6.3 branch? I'd expect it to be failing currently since the 6.3 branch uses 14.x and not 16.x. 🤔

@joemcgill commented on PR #5515:


8 months ago
#17

@desrosj I think the old ones aren't failing because this will only happen when the workflow is run when comparing the current commit to the previous commit when the previous commit uses a different node version. Perhaps adding an nvm use or similar command before running npm ci when comparing against the previous commit will suffice? At any rate, the I would expect the very next commit to resolve the issue.

@swissspidy commented on PR #5515:


8 months ago
#18

What Joe says :) nvm install after "Check out target commit" and then again after "Reset to original commit" should do the trick. Should be safe to add, even if it theoretically only happens once when doing the version bump. For future commits it's a no-op because the Node version doesn't change between commits. So a reasonable addition 👍

#19 @desrosj
8 months ago

  • Summary changed from Bump the minimum required version of Node.js to 18.x to Bump the minimum required version of Node.js to 20.x

20.x has been installed on the .org build server. Changing the ticket summary to reflect the new targeted version in this ticket.

#20 @desrosj
7 months ago

In 57212:

Build/Test Tools: Raise minimum required version of Node.js/npm.

This bumps the minimum required version of Node.js/npm from 16.19.1 and 8.19.3 to 20.10.0 and 10.2.3.

Since 20.10.0 is the latest 20.x version of Node.js, the check-latest option has been enabled for actions/setup-node in GitHub Actions workflows. This performs an additional external call to the Node.js API confirming the latest version is installed on the runner for use. In testing, it seems that 20.10.0 was not consistently deployed to all runner machines in use. This should be removed in the near future when the version of Node.js is reliably above the new minimum requirement.

The Gutenberg repository has also been updated to use the same values for engines.

Props jorbin, joemcgill, swissspidy, benharri, dhrupo, flootr, gziolo, noahtallen.
See #59663.

#21 @desrosj
7 months ago

In 57213:

Build/Test Tools: Add engines for default themes.

This adds the engines field to the package.json file for the three default themes with build processes in order to encourage consistent tooling for contributors.

Some minor dependency updates for these themes are also included in this change.

Props jorbin, joemcgill, swissspidy.
See #59663.

#23 @desrosj
7 months ago

In 57214:

Build/Test Tools: Raise minimum required version of Node.js/npm.

This bumps the minimum required version of Node.js/npm from 16.19.1 and 8.19.3 to 20.10.0 and 10.2.3.

Since 20.10.0 is the latest 20.x version of Node.js, the check-latest option has been enabled for actions/setup-node in GitHub Actions workflows. This performs an additional external call to the Node.js API confirming the latest version is installed on the runner for use. In testing, it seems that 20.10.0 was not consistently deployed to all runner machines in use. This should be removed in the near future when the version of Node.js is reliably above the new minimum requirement.

The Gutenberg repository has also been updated to use the same values for engines.

Merges [57212] to the 6.4 branch.

Props jorbin, joemcgill, swissspidy, benharri, dhrupo, flootr, gziolo, noahtallen.
See #59663.

#25 @desrosj
7 months ago

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

Announcement note published: https://make.wordpress.org/core/2023/12/20/updating-wordpress-to-use-more-modern-versions-of-node-js-npm-2/

I think this can be closed out. Thanks all! If there's something that was missed, we can reopen and address.

#26 @jorbin
7 months ago

In 57217:

Build/Test: Update Grunt Patch Wordpress for Node 20+.

Props jorbin, desrosj.
See #59658, #59663.

#27 @kraftbj
7 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

What would y'all think about updating .nvmrc to 20.10 instead of simply 20? I had a 20 version less than 20.10 on my device as the alias default and nvm use was happy with it. But, when installing, it failed due to lack of proper version. Setting it to 20.10 will install the minimum required.

Last edited 7 months ago by kraftbj (previous) (diff)

This ticket was mentioned in PR #5820 on WordPress/wordpress-develop by @kraftbj.


7 months ago
#28

With the recent update to Node 20.x, we're actually requiring 20.10.x, so suggesting .nvmrc to 20.10 to ensure that nvm use works correctly.

#29 @kraftbj
7 months ago

Noting that if this is accepted, we should have a similar PR for Gutenberg, but I'll wait to open that until hearing if this is something acceptable.

#30 @desrosj
7 months ago

@kraftbj hmm I’d have to do some testing.

Ideally, when 20.11.2, 20.12.0, etc. is released, nvm will naturally install or update a contributors machine appropriately. I think if we change the file to 20.10, only 20.10.z releases will be installed and we’d have to commit an update with every release.

My typical workflow is to run nvm install, so I’ve never your edge case. I recently also set up my machine to always check for an .nvmrc file when changing directories, executing nvm install if one is present to ensure the right version is being used.

I’m not opposed to considering temporarily defining 20.10 to ease the transition. After a few more releases of 20.x, the likelihood of this goes down. I think it’s mostly a problem because 20.10 is the latest release. But maybe the better path is better documentation and education to use nvm install instead of nvm use since that would cover future scenarios like this as well.

#31 @desrosj
7 months ago

One other thought I just had: we did need to include check-latest when using the setup-node action in the GitHub Actions workflows because 20.10 was not reliably available on non-Ubuntu runner images. We could change the version back to 20 when that gets removed to save an extra external request in a few months.

#32 @kraftbj
7 months ago

I also have an automatic function to nvm in a new directory. Looking it up, it is the ohmyzsh-included nvm plugin which runs nvm install only if the version of node in .nvmrc is not installed and nvm use instead: https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/nvm/nvm.plugin.zsh

IMO, if the version in nvmrc could be met by versions not meeting package.json, one or the other should be adjusted (e.g. why 20.10 vs 20.x as the minimum? Is there a functional reason why 20.10 is the minimum of the LTS?)

#33 @desrosj
7 months ago

#59730 was marked as a duplicate.

#34 @johnbillion
6 months ago

I just experienced the same issue as Brandon. Running nvm use switched to 20.9, but npm install failed as it requires >=20.10.

Log:

> nvm use
Found '/Users/john/sites/wp/.nvmrc' with version <20>
Now using node v20.9.0 (npm v10.1.0)

> npm i
npm ERR! code EBADENGINE
npm ERR! engine Unsupported engine
npm ERR! engine Not compatible with your version of node/npm: WordPress@6.5.0
npm ERR! notsup Not compatible with your version of node/npm: WordPress@6.5.0
npm ERR! notsup Required: {"node":">=20.10.0","npm":">=10.2.3"}
npm ERR! notsup Actual:   {"npm":"10.1.0","node":"v20.9.0"}

#35 @johnbillion
6 months ago

  • Keywords needs-patch added; has-patch removed

#36 @stevenlinx
5 months ago

  • Keywords add-to-field-guide added

#37 @swissspidy
5 months ago

I feel like so much time has passed since these reported issues with nvm use that I don't think it's worth anymore to change .nvmrc to state 20.10.. Running nvm install is a quick way to fix this and I'm sure contributors have all updated in the meantime. So suggesting to close this again.

#38 @swissspidy
5 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.