Tooling: Add volta versions #48734

sirreal · 2021-01-08T14:10:35Z

Changes proposed in this Pull Request

This prepares the environment for use with Volta, a potential replacement for nvm (but more).

It sets a volta property in the root package.json. It also points all the other package.json in the repo to extend that root config. This is under the assumption that all tooling should use the version in the root.

It was a little annoying to add a volta property to every package.json, but that should be a one-time thing and doesn't seem like it should have any negative impact. That seems to be the way to do things in Volta at the moment. See volta-cli/volta#906 and the Workspaces docs.

Testing instructions

You'll need to set your system up for Volta. I have no idea if that will play well with nvm. I've removed nvm from my system completely.

From an arbitrary location like ~, install some volta tools (that don't correspond to the versions in Calypso). For example:

volta install node@latest
volta install typescript@beta

Check the versions with volta list:

$ volta list
⚡️ Currently active tools:

    Node: v15.6.0 (default)
    npm: v6.14.11 (default)
    Yarn: v1.22.10 (default)
    Tool binaries available:
        wp-env (default)
        neovim-node-host (default)
        tsc, tsserver (default)
        yarn-deduplicate (default)

$ node --version
v15.6.0
$ tsc --version
Version 4.2.0-dev

move to Calypso and check the versions:

$ volta list
⚡️ Currently active tools:

    Node: v12.20.1 (current @ /Users/jonsurrell/a8c/calypso/package.json)
    Yarn: v1.22.10 (current @ /Users/jonsurrell/a8c/calypso/package.json)
    Tool binaries available:
        wp-env (default)
        neovim-node-host (default)
        tsc, tsserver (current @ /Users/jonsurrell/a8c/calypso/package.json)
        yarn-deduplicate (default)

# Nice, volta picked up different versions! Let's check…

$ node --version
# installs on demand if necessary!!
v12.20.1

# It detects the package's version and prefers it!
$ tsc --version
Version 4.1.3

Repeat from another directory with a package.json such as packages/*/ or apps/editing-toolkit or client.

matticbot · 2021-01-08T14:10:36Z

Test live: https://calypso.live/?branch=add/volta-pin

matticbot · 2021-01-08T14:11:34Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

package.json

matticbot · 2021-01-21T09:25:11Z

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D55743-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

sirreal · 2021-01-21T17:22:07Z

Looks like renovate should support: renovatebot/renovate#4262

noahtallen

I did a bit of testing with nvm + volta, and it looks like if NVM is configured in .the zshrc file, then it overrides volta. nvm use sets to 12.x, and that persists when changing to other directories. (e.g. the volta version is not used)

When nvm is not set in .zshrc, it works as expected:

It appears that nvm still works fine with volta config available. So in other words, this shouldn't break anything for existing users :)

I'm all for this, as I've been getting frustrated by NVM. For example, if I try to do a git commit or push via Visual Studio Code's GUI, the precommit hooks will utilize the "default" node version rather than the local nvm node version, which sucks when different projects use different node versions.

After trying out the precommit hooks with volta, it looks like that problem is finally solved! My editor is using the node version for the project without any intervention. (Tried with both wp-calypso and gutenberg using your branches.)

scinos · 2021-02-18T04:25:41Z

package.json

+	},
+	"volta": {
+		"node": "12.20.1",
+		"yarn": "1.22.10"


How does this interact with the version set in .yarnrc?

Part of the yarn executable's behavior is to get the correct version via .yarnrc, isn't it? I did a test by pinning [email protected] for volta. You can see that although volta will invoke it's [email protected] binary, yarn reports its version is 1.22.10. I think everything will work smoothly and ultimately .yarnrc determines the version like I'd expect.

# volta list ⚡️ Currently active tools: Node: v12.20.1 (current @ …/calypso/package.json) Yarn: v1.21.1 (current @ …/calypso/package.json) Tool binaries available: wp-env (default) neovim-node-host (default) stryker (default) tsc, tsserver (current @ …/calypso/package.json) yarn-deduplicate (default) See options for more detailed reports by running `volta list --help`. # yarn --version 1.22.10

Seems weird and confusing to have a yarn version defined in two places. Do we really need to tell volta about yarn version?

What's the dev experience if we drop the version in .yarnrc and we want to update yarn?

If we go all-in with volta, I believe we could safely drop either one.

What's the dev experience if we drop the version in .yarnrc and we want to update yarn?

I don't understand the question. Updating yarn via volta means running volta pin yarn@latest in the repo root.

Let's say we drop .yarnrc and we pin yarn using volta to a specific version (1.22.10).

How do update the version? I guess it is only a change in package.json, right?

What happens when a dev checks out a branch that points to a version they don't have locally? Does volta automatically downloads the new version?

How do update the version? I guess it is only a change in package.json, right?

Yep.

What happens when a dev checks out a branch that points to a version they don't have locally? Does volta automatically downloads the new version?

Yes.

scinos · 2021-02-18T04:28:02Z

It is great that we are looking for alternatives to NVM if NVM is causing problems. Could you expand on the problems we are trying to solve with volta?

In any case, I think that we should only have one tool to do one thing. In other words, if we go ahead with this change, it should be a migration from nvm to volta with the aim of removing nvm from our system. Otherwise we are increasing the maintenance burden and the risk of `nvm

griffbrad · 2021-02-18T20:38:16Z

It is great that we are looking for alternatives to NVM if NVM is causing problems. Could you expand on the problems we are trying to solve with volta?

I found @noahtallen’s comment on the related Gutenberg PR helpful in this regard. With volta, you no longer need to nvm i && nvm use. Instead, you should automatically use the correct node version for the project. That’s nice when on an interactive shell but perhaps more useful in the context of other tools that might otherwise end up using the incorrect version.

Dropping nvm in Calypso would be fine with me.

creativecoder · 2021-04-29T14:42:42Z

FYI, I'm proposing the same change in Jetpack: Automattic/jetpack#19705

noahtallen · 2021-04-30T21:43:48Z

I do like the ergonomics of Volta a lot, and have been using it exclusively recently, even without these changes. I wonder what our next steps should be? Do we need to commit to nvm or volta, or can we support both at the same time? (I don't see supporting volta as having a high cost, since it just involves bumping the version in the root package.json whenever we update node.)

Relatedly... I really, really wish they had not gone with this approach for monorepos. It seems to be the largest source of concern for project adoption since it increases the surface area by so much.

scinos · 2021-05-01T08:41:51Z

I wonder what our next steps should be? Do we need to commit to nvm or volta, or can we support both at the same time?

I still think we should draft a plan to do a full transition from NVM to Volta. IMO, at the very least we need (in no particular order):

Change our Docker images to use Volta instead of NVM
Write P2s explaining the difference, defining the transition plan and in general educating our devs about the new tool.
Land this PR.

Nice to have:

Clean up all those package.json. As @sirreal said, only a few of those will actually need volta (let's say only those that define 'scripts'?). Eventually get rid of the "fake" package.json files, this is not the first time they cause troubles.
Detect when someone is using node from nvm, and warn them (example: have a pre-install hook that warns if the node binary is from ~/.nvm/versions/node/v*/bin/node)

creativecoder · 2021-05-10T21:16:51Z

I still think we should draft a plan to do a full transition from NVM to Volta. IMO, at the very least we need (in no particular order): ...

That all seems good to me.

Note that I still have nvm installed and volta seems to take precedence when running shell commands for which version of node and any npm installed binaries are used.

sirreal · 2021-05-11T12:43:06Z

I can draft or help with a p2 post. I'd recommend upgrading to Node 16 first (p7H4VZ-31V-p2) so everyone gets native Node binaries with Volta.

github-actions · 2021-07-10T18:37:28Z

This PR has been marked as stale due to lack of activity within the last 30 days.

matticbot · 2021-10-04T08:14:31Z

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-16074
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-16074&env=jetpack

sirreal · 2023-04-11T13:25:48Z

This is very stale, I'll close it now. If there's interest and a desire to move forward with this I'd be happy to get this ready again.

noahtallen · 2023-04-11T18:54:41Z

Totally. I really wish they would just support reading from a higher-level .json file! I also looked into manual pinning -- like if I could achieve automatic version switching via a file I could add to .gitignore, but that also isn't supported :(

So I'm back to nvm for now, just because both Gutenberg and Calypso use it.

scinos · 2023-04-11T20:00:19Z

I really wish they would just support reading from a higher-level .json file

For the record, they kind of do, with extends. Docs.

noahtallen · 2023-04-11T20:05:17Z

Oh hey! Hope things are going great :D That's what this PR was exploring originally

creativecoder · 2023-04-11T20:47:25Z

I'm following this issue in the volta repo, but there hasn't been much movement: volta-cli/volta#282

sirreal added the Build label Jan 8, 2021

sirreal self-assigned this Jan 8, 2021

sirreal added the [Status] In Progress label Jan 8, 2021

sirreal commented Jan 14, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

sirreal force-pushed the add/volta-pin branch from 2f396d1 to a380a0b Compare January 21, 2021 09:16

sirreal force-pushed the add/volta-pin branch from a380a0b to b4180d3 Compare January 21, 2021 09:25

sirreal added [Type] Tooling Related to tools, scripts, automation, DevX, etc. Framework [Type] Question [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jan 21, 2021

sirreal force-pushed the add/volta-pin branch from b4180d3 to 0c1c093 Compare January 21, 2021 17:20

sirreal marked this pull request as ready for review January 21, 2021 17:22

sirreal requested review from a team as code owners January 21, 2021 17:22

sirreal requested review from withinboredom, noahtallen and a team and removed request for a team January 21, 2021 17:22

sirreal force-pushed the add/volta-pin branch from 0c1c093 to acff2fb Compare February 16, 2021 13:16

noahtallen approved these changes Feb 17, 2021

View reviewed changes

scinos reviewed Feb 18, 2021

View reviewed changes

sirreal force-pushed the add/volta-pin branch 4 times, most recently from a781074 to 40ba97a Compare April 29, 2021 11:41

creativecoder mentioned this pull request Apr 29, 2021

Add volta configuration for managing node versions Automattic/jetpack#19705

Closed

github-actions bot added the [Status] Stale label Jul 10, 2021

sirreal added 3 commits October 4, 2021 09:31

Add root volta config

36bfa15

Add volta extends to all package.json files

55bd5df

Pin current node/npm/yarn versions

05956e1

sirreal force-pushed the add/volta-pin branch from 40ba97a to 05956e1 Compare October 4, 2021 08:12

sirreal requested review from worldomonation, WunderBart and a team as code owners October 4, 2021 08:12

worldomonation removed request for a team and worldomonation June 6, 2022 17:20

sirreal closed this Apr 11, 2023

sirreal deleted the add/volta-pin branch April 11, 2023 13:25

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tooling: Add volta versions #48734

Tooling: Add volta versions #48734

sirreal commented Jan 8, 2021 •

edited

Loading

matticbot commented Jan 8, 2021

matticbot commented Jan 8, 2021 •

edited

Loading

matticbot commented Jan 21, 2021

sirreal commented Jan 21, 2021

noahtallen left a comment

scinos Feb 18, 2021

sirreal Feb 19, 2021

scinos Feb 20, 2021

sirreal Feb 22, 2021

scinos Feb 22, 2021

sirreal Feb 22, 2021

scinos commented Feb 18, 2021

griffbrad commented Feb 18, 2021

creativecoder commented Apr 29, 2021

noahtallen commented Apr 30, 2021

scinos commented May 1, 2021

creativecoder commented May 10, 2021

sirreal commented May 11, 2021

github-actions bot commented Jul 10, 2021

matticbot commented Oct 4, 2021 •

edited

Loading

sirreal commented Apr 11, 2023

noahtallen commented Apr 11, 2023

scinos commented Apr 11, 2023

noahtallen commented Apr 11, 2023

creativecoder commented Apr 11, 2023

Tooling: Add volta versions #48734

Tooling: Add volta versions #48734

Conversation

sirreal commented Jan 8, 2021 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Jan 8, 2021

matticbot commented Jan 8, 2021 • edited Loading

matticbot commented Jan 21, 2021

sirreal commented Jan 21, 2021

noahtallen left a comment

Choose a reason for hiding this comment

scinos Feb 18, 2021

Choose a reason for hiding this comment

sirreal Feb 19, 2021

Choose a reason for hiding this comment

scinos Feb 20, 2021

Choose a reason for hiding this comment

sirreal Feb 22, 2021

Choose a reason for hiding this comment

scinos Feb 22, 2021

Choose a reason for hiding this comment

sirreal Feb 22, 2021

Choose a reason for hiding this comment

scinos commented Feb 18, 2021

griffbrad commented Feb 18, 2021

creativecoder commented Apr 29, 2021

noahtallen commented Apr 30, 2021

scinos commented May 1, 2021

creativecoder commented May 10, 2021

sirreal commented May 11, 2021

github-actions bot commented Jul 10, 2021

matticbot commented Oct 4, 2021 • edited Loading

sirreal commented Apr 11, 2023

noahtallen commented Apr 11, 2023

scinos commented Apr 11, 2023

noahtallen commented Apr 11, 2023

creativecoder commented Apr 11, 2023

sirreal commented Jan 8, 2021 •

edited

Loading

matticbot commented Jan 8, 2021 •

edited

Loading

matticbot commented Oct 4, 2021 •

edited

Loading