Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#51603 closed task (blessed) (fixed)

Define specific versions of Node in the .nvmrc file for the 5.0-5.5 branches

Reported by: desrosj's profile desrosj Owned by: desrosj's profile desrosj
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

The NodeJS version aliased to lts changes. Defining specific versions in the .nvmrc file will ensure npm install and building WordPress continue to work without issue going forward.

After some testing, the versions can be defined as such:

  • 5.0: 10.22.1 (npm 6.14.6)
  • 5.1: 11.15.0 (npm 6.7.0)
  • 5.2: 11.15.0 (npm 6.7.0)
  • 5.3: 12.19.0 (npm 6.14.8)

Previously: #48140.

Attachments (1)

51603.diff (5.3 KB) - added by desrosj 4 years ago.

Download all attachments as: .zip

Change History (22)

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


4 years ago
#1

  • Keywords has-patch added

#5 @desrosj
4 years ago

  • Keywords has-patch removed

Actually the versions I listed above are incorrect. Because of how NodeJS versions, using 10.x is preferred over 11.x. The versions are as follows:

  • 5.0-5.2: 10.22.1 (npm 6.14.6)
  • 5.3: 12.19.0 (npm 6.14.8)

#6 @desrosj
4 years ago

In 49277:

Build/Test Tools: Explicitly specify a version number in the .nvmrc file for the 5.0 branch.

This restores the ability to run NodeJS related tasks when using nvm install or nvm use.

The alias lts/* currently resolves to NodeJS 12.x (and will continue to change as newer versions are released). The 10.x version of NodeJS is the highest version supported in the 5.0 branch.

This also removes the explicit version when running nvm install during automated testing. The command will now fall back to the version in the .nvmrc file.

See #51603.

#8 @desrosj
4 years ago

In 49278:

Build/Test Tools: Explicitly specify a version number in the .nvmrc file for the 5.1 branch.

This restores the ability to run NodeJS related tasks when using nvm install or nvm use.

The alias lts/* currently resolves to NodeJS 12.x (and will continue to change as newer versions are released). The 10.x version of NodeJS is the highest version supported in the 5.1 branch.

This also removes the explicit version when running nvm install during automated testing. The command will now fall back to the version in the .nvmrc file.

See #51603.

#10 @desrosj
4 years ago

In 49279:

Build/Test Tools: Explicitly specify a version number in the .nvmrc file for the 5.3 branch.

This ensures the ability to run NodeJS related tasks when using nvm install or nvm use will continue to be usable as new versions of NodeJS are moved into LTS.

The alias lts/* currently resolves to NodeJS 12.x (which is the highest version of NodeJS supported in the 5.3 branch). However, lts/* will point to newer versions in the near future.

This also removes the explicit version when running nvm install during automated testing. The command will now fall back to the version in the .nvmrc file.

See #51603.

@desrosj
4 years ago

#12 @desrosj
4 years ago

The 5.2 branch is the only one remaining. When following this flow locally, I am seeing weird changes to the package-lock.json file (see 51603.diff).

nvm install
rm -rf node_modules
npm install

The same version was used in the 5.2 branch, but the same issue did not occur. The commands above complete successfully and I am still able to run build related commands.

This ticket was mentioned in Slack in #core-editor by desrosj. View the logs.


4 years ago

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


4 years ago

#15 @netweb
4 years ago

Sorry for getting back to this late, thankfully timezones I think you'll still be sleeping right now @desrosj ;)

A thing to be carefully of is the "WordPress Build Server" aka build.trac.wordpress.org

The build server does NOT have nvm installed or available

https://make.wordpress.org/systems/2020/01/21/update-svn1-node-versions-for-gutenberg/

Each time a new version of Node.js is required to "build" WordPress a make/systems request is required to update the Node.js versions available


@dd32, @nacin, and I have discussed the "Build Tools" many times over the years, when Grunt was added back in the day there was never a plan to have to maintain branches on the particular version of Node.js that was around at the time, this remains true to this day.

@dd32 and I have discussed this a few times over the years, maybe also with @johnbillion also

We decided that all branches can use the same Node.js version, based on the current version available, Node.js v12.x would be what we would look at today, avoiding v14 as it's not available on the build server just yet, I suspect the Gutenberg team will look at that in the coming weeks.

Maintaining legacy branches with out of date Node.js packages is incredibly difficult, SASS in particular, as if they unpublished the binaries used then we'd have issues, likewise deprecated, removed packages all cause further issues if a package was to be removed from npm for example.

So, any branch, WP 3.7, 4.2, 5.2 etc can be updated to use Node.js v12 for the build tools, so whilst this hasn't historically taken place it really should, and sorry I didn't notice this ticket earlier.

One issue, that has been "blessed" by @dd32 was that if for example we update WP 4.0 to use Node.js v12 and the latest versions of the packages from package.json at the time:

https://core.trac.wordpress.org/browser/branches/4.0/package.json

A few of these packages using the latest versions of these tools, for example autoprefixer, cssmin, imagemin, cssjanus (RTLCSS), sass may result in a few changes to CSS files and images, these changes are fine to commit and ship with any updated WP 4.0 release, this is because for the most part the changes would only be improvements not regressions, improved vendor prefixes, optimised images, RTL CSS improvements. Each change would have to be confirmed and to make sure the changes wouldn't overly affect the size of the release package for WordPress Automatic Updates but otherwise these changes are blessed to be made.

tl;dr: We can use Node.js v12.16.3 and npm v6.14.4 for all maintained WordPress Branches 3.7->5.6

Feel free to hit me up in Slack or here with any further questions or details

#16 follow-up: @desrosj
4 years ago

  • Summary changed from Define specific versions of Node in the .nvmrc file for the 5.0-5.3 branches to Define specific versions of Node in the .nvmrc file for the 5.0-5.5 branches

@netweb Thanks for this! That sounds good to me. I plan to step back after 5.6 and take a wider inventory of NodeJS in branches 3.7+ and trunk and come up with a plan of action. NodeJS < 6 is also a blocker for backporting the local Docker environment back to 3.7-4.5 as the needed packages do not support those versions. I think we can do that in a separate ticket.

For the time being, I am thinking that the .nvmrc file should be updated when a new version is branched. So when 5.6 is branched, the 5.5 branch becomes "unmaintained" except for security releases, so the .nvmrc file should be changed to specify a specific version of NPM to ensure anyone working with that branch is able to build without having to perform archaeology.

#17 @peterwilsoncc
4 years ago

Sorry (kind of) for the subscribe message to get emails. Subscribe :D

#18 in reply to: ↑ 16 @netweb
4 years ago

Replying to desrosj:

@netweb Thanks for this! That sounds good to me. I plan to step back after 5.6 and take a wider inventory of NodeJS in branches 3.7+ and trunk and come up with a plan of action. NodeJS < 6 is also a blocker for backporting the local Docker environment back to 3.7-4.5 as the needed packages do not support those versions. I think we can do that in a separate ticket.

The latest LTS release of Node.js should be used for all supported branches that continue to be maintained which are from the 3.7 branch onward, at the time of writing the latest LTS release of Node.js is v14.x

The .nvmrc file in each branch from 3.7 onward should all use lts/* (or add an .nvmrc file if it doesn't exist currently), nvm aliases lts/* to the latest LTS Node.js release ( See here)

All supported branches should have the same .nvmrc file that is used in /trunk:

https://core.trac.wordpress.org/browser/trunk/.nvmrc

Replying to desrosj:

For the time being, I am thinking that the .nvmrc file should be updated when a new version is branched. So when 5.6 is branched, the 5.5 branch becomes "unmaintained" except for security releases, so the .nvmrc file should be changed to specify a specific version of NPM to ensure anyone working with that branch is able to build without having to perform archaeology.

No, not really, NPM do not support legacy releases of NPM, the only supported releases of NPM are the latest releases, this has recently switched to v7 of NPM, though v6 of NPM will be maintained for I expect a period of time until v7 is considered stable and reliable.

To be able to build an older WordPress branch, for example 3.8 release, here's the package.json tag https://core.trac.wordpress.org/browser/tags/3.8/package.json, in the latest commit to that file the commit message is:

"Temporarily switch to a pinned version of grunt-cssjanus. This in turn temporarily pins cssjanus to a patched fork."

The above is an example of that "archaeology" we want to begin to avoid

By continuing to use the versions set in this package.json file there are known bugs in CSS & JS minification that continue to be shipped with each release of the WordPress 3.8 branch, however if the 3.8 branch was using NPM v7.x & Node.js v14.x with the latest versions of the CSS & JS minification packages then these known bugs would be resolved and no longer shipped in the next release of the 3.8 branch which would resolve known bugs from ~7 years ago that continue to be shipped in each 3.8.x release at the moment.

Another example is WordPress 3.9 https://core.trac.wordpress.org/browser/tags/3.9/package.json, in this example it will install grunt-sass version 0.10.0, this package requires node-sass version 0.8.0, The Node-SASS package no longer builds the binaries for unsupported releases, the oldest binaries available are for v4.5.3 from 3 years ago, v0.8.0 was released 6 years ago, LibSass and Node Sass are deprecated and it is recommended to use Dart Sass instead.

The above are just two examples, changing the versions of Node.js and NPM that were used when the WordPress branch was released requires being able to dig through this type of "archaeology".

To not have to done any "archaeology" gear, we should update all the supported branches 3.7 thru 5.6 to use Node.js v14.x and NPM v7.x

To do this, for WP 3.8 grunt-cssjanus package would be replaced with the same RTL CSS package used in /trunk grunt-rtlcss, this brings the RTL CSS improvements over CSSJanus to the users still using WP 3.8. For WP 3.9, grunt-sass would be updated from 0.8.0 to 3.1.0 and the latest Dart Sass instead of LibSass/Node-Sass (see also #51763 Evaluate moving to Dart Sass)

Likewise for any other Node.js packages, if the old WordPress branch uses an old version of a package it can be upgraded to the latest, or changed to what is now used in /trunk where new and improved packages have been chosen (E.g. The switch to RTLCSS from CSSJanus)

Doing this we patch the known bugs in the the next releases of legacy branches, CSS & JS bugs in WP 3.8.36, SCSS CSS compiled by Dart Sass in WP 3.9.34, and so on through each of the supported branches.

Even the @wordpress/scripts package can be added to the WordPress 3.7 branches, going forward when /trunk updates packages in package.json, these changes are backported to the branches thru 3.7 where appropriate.

It will aid backporting the local docker environment in #48301, and backporting GitHub Actions in #50401, Use Webpack & NPM scripts to build all the things in #43731, Updating Gutenberg and Block-Editor in legacy branches, the benefits of this solution are far reaching for the WP Build Tools in simplifying developer, maintenance, and systems tools all at the same time.

Further, every supported branch of WordPress 3.7 thru 5.6 and beyond will have these features:

  • Every branch uses the latest LTS Node.js and NPM release
  • There is no need for any developer or maintainer to have to switch Node.js versions when checking out a branch
  • The build servers only need support the latest releases of Node.js and NPM, the make/systems team no longer have to maintain specific versions of Node.js and NPM on the w.org build for each branch released.
  • A common set of Grunt, Node script commands for each branch, e.g. npm run lint, `grunt lint, etc (Composer scripts too)

The above, is what I envision as to how to remove the build tool "archaeology" in all aspects, for good, it's been blessed by Nacin and Dion for years, we've just never gone ahead and actually done it.

Last edited 4 years ago by netweb (previous) (diff)

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#20 @hellofromTonya
4 years ago

  • Milestone changed from 5.6 to 5.7

Punting this ticket to 5.7 as today is 5.6 RC2.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#21 @desrosj
4 years ago

  • Milestone changed from 5.7 to 5.6
  • Resolution set to fixed
  • Status changed from assigned to closed

Going to close this out. The only unaddressed branch is the 5.2 branch, but this effort will be superseded by #52341, and the 5.2 branch will be updated there.

Note: See TracTickets for help on using tickets.