Skip to content

Change .nvmrc and documentation for Node.js version (LTS to 14.18.1)#36744

Merged
gziolo merged 5 commits intoWordPress:trunkfrom
alexstine:update/node-version-info
Nov 23, 2021
Merged

Change .nvmrc and documentation for Node.js version (LTS to 14.18.1)#36744
gziolo merged 5 commits intoWordPress:trunkfrom
alexstine:update/node-version-info

Conversation

@alexstine
Copy link
Copy Markdown
Contributor

@alexstine alexstine commented Nov 22, 2021

Description

Instead of using Node.js lts version in .nvmrc and other documentation, this PR switches items to use version 14.18.1. I've found that the lts version can vary from system to system and this has gotten me a couple times. It seems hardcoding the correct version is the appropriate option to fix any confusion going forward.

Why the Change?

This is what happens when running the nvm install command as mentioned in documentation.

`alexstine@asm gutenberg % nvm install --lts

Installing latest LTS version.

v16.13.0 is already installed.

Now using node v16.13.0 (npm v8.1.0)`

I'm not sure exactly why this started happening, but these versions do not work with Gutenberg.

`alexstine@asm gutenberg % node --version

v16.13.0

alexstine@asm gutenberg % npm --version

8.1.0`

While I am not in favor of using older versions, using what works is a must. After asking @tellthemachines on Slack, I decided that a PR with the correct version instead of a generic "lts/*" may be an improvement.

I kind of wonder if this is just some strange issue with my system and this doesn't effect others. In that case, this can be closed out.

How has this been tested?

Whenever I run nvm use, the correct version is loaded up and things just work. 👍

Screenshots

Types of changes

Bug fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Nov 22, 2021

There is another PR opened nearly 2 weeks ago which pins node to 14.x line rather than to the specific version which is going to change quickly. See #36384 from @walbo. I think the only blocker for using LTS is the fact that Node 16.x that was promoted to LTS a few weeks ago is using npm v8 rather than npm v6 that we still have to use.

I recently noticed that many projects use --legacy-peer-deps with npm 7/8 as a temporary meassure needed until the whole ecosystem is able to handle automatic peer dependencies installation. Maybe we should follow the same path and migrate quicker to Node 16 + npm 8. The only blocker I see is what @desrosj raised a few weeks back that we should keep the same versions as in WordPress Core to ensure it builds correctly with the infrastructre in place.

@alexstine
Copy link
Copy Markdown
Contributor Author

@gziolo If I change to v14, will it allow us to get this in sooner? Every time I face issues with this and I have to imagine others do as well. This also updates the all important docs to better reflect the versions.

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Nov 22, 2021

@gziolo If I change to v14, will it allow us to get this in sooner? Every time I face issues with this and I have to imagine others do as well. This also updates the all important docs to better reflect the versions.

Yes, it's also what @youknowriad did for wp/5.8 branch a few days ago:

https://github.com/WordPress/gutenberg/blob/wp/5.8/.nvmrc

and what we did in the past (version number used changes over time) for branches containing code for older major WordPress releases.

Using LTS sounds great in overall, but the change happens once per year, so maybe it's fine to apply the change explicitly with a PR like this one.


- Node.js
Gutenberg is a JavaScript project and requires [Node.js](https://nodejs.org/). The project is built using the latest active LTS release of node, and the latest version of NPM. See the [LTS release schedule](https://github.com/nodejs/Release#release-schedule) for details.
Gutenberg is a JavaScript project and requires [Node.js](https://nodejs.org/). The project is built using the version (14.18.1) release of node, and the latest version of NPM. See the [LTS release schedule](https://github.com/nodejs/Release#release-schedule) for details.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment we also use npm v6 rather than the latest one.


Quit and restart terminal
Install the long-term support (lts) version of node.
Install node version (14.18.1).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep this paragraph as is. See https://nodejs.org/en/about/releases/. Node 14 is considered Maintenance LTS release.

@alexstine
Copy link
Copy Markdown
Contributor Author

@gziolo What do you think of latest changes? I updated things so I am more generically using v14. I am not sure about keeping that one LTS line the same as I feel it may be a bit confusing. Thoughts?

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Nov 22, 2021

@mkaz and @ryanwelcher, would you mind helping polishing the documentation changes? 😄

The part where we use 14 as the Node.js version is solid 👍🏻

@alexstine
Copy link
Copy Markdown
Contributor Author

Thanks @gziolo .

Also, managed to get the checks to pass again. 👍

Copy link
Copy Markdown
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use consistently v6 and v14 for versions based on the Node.js releases page:

https://nodejs.org/en/about/releases/

Screenshot 2021-11-23 at 15 41 02

Copy link
Copy Markdown
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexstine, thank you for wrangling these changes, much appreciated 🙇🏻

@gziolo gziolo merged commit 1714d93 into WordPress:trunk Nov 23, 2021
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 23, 2021
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Nov 23, 2021

@noisysocks, you might want to cherry-pick this PR to the 5.9 release. It's going to be necessary at some point when we create wp/5.9 branch to ensure that the Node.js version is pinned to v14 forever 😄

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Nov 23, 2021

I have just tested with the latest trunk and it works like a charm 🎉

noisysocks pushed a commit that referenced this pull request Nov 29, 2021
…36744)

* Documentation updates and better NVM support.

* Be more generic about version 14.

* Merge trunk in to branch.

* Apply suggestions from code review

Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
@ellatrix ellatrix mentioned this pull request Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants