-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tooling: Support volta #28967
Tooling: Support volta #28967
Conversation
This is a draft, I accidentally created a "ready to review" PR which pinged a lot of folks. Sorry for that, feel free to unsubscribe. |
8b714a2
to
0975411
Compare
Size Change: 0 B Total Size: 1.39 MB ℹ️ View Unchanged
|
Ok, nevermind, I read the code which answers the question, it just uses the top level versions wanted. I suppose it can't use the engines field? |
Right. Volta only looks for the nearest package.json and stops. The recommendation for monorepos (Volta "workspaces") is to explicitly extend the pinned versions. Once the extends have been introduced, there's a single place to update the pinned version.
It can't use the engines field as far as I know. Pinned Volta versions must resolve to a specific version. The meaning of a pinned version cannot change between today and tomorrow. This is different from engines and .nvmrc which use "version-ish" versioning that can mean different things at different times. There's some interesting details and discussion around this here: volta-cli/volta#282 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should move forward with this! From what I can tell, this does not conflict with nvm
. So for most folks, this will continue working as expected. But for those of us who wish to check out volta, this would be very nice. I don't see any downside to merging this, other than maintenence:
- How do we enforce having to add volta to new package.jsons?
- how do we make sure that the root volta node/npm versions stay up to date when we update node for the project?
Aside from not having to ever run nvm use
, the big benefit for me is IDE GUIs. For example, when I make a commit via Visual Studio Code GUI, it will run precommit hooks. That script will use the nvm
defualt version rather than the version for the project. As you can imagine, when the "nvm default" and "nvm project version" are different, this casuse the precommit hook to fail for no reason! But with volta, it automatically uses the correct version without any intervention, which means these 'behind-the-scenes' tasks are using the correct node version by default. Huge fan of that.
0975411
to
a9bbe1c
Compare
Rebased and updated to pin npm@7. I believe Gutenberg is trying to move to npm@7 but I see the package-lock.json is still on lockversion 1. This is ready for broader discussion, we could add documentation suggesting it as a supported tool for managing versions in the same places nvm is mentioned. It may be worth bringing up at one of the weekly chats. |
Good questions @noahtallen, thanks.
It sounds like something @wordpress/npm-package-json-lint-config could handle if it doesn't already. At any rate, the root volta pin is almost always used, the only time packages are important are when the current directory is under another package. Volta only ascends to the nearest package.json. I think we're running volta commands from the repo root the vast majority of the time.
I believe tools like renovate already support volta, but I don't think we're using those tools. Volta pins a specific version so we can't get the same "set and forget" behavior as nvm where you give a "versionish" and nvm will stay up to date. The pinned volta versions will need to be updated when node lts releases happen. |
npm is being pinned to v6, see #28824, #29000, #29204
It doesn't have a rule for custom entries right now, could contribute a new rue by coppying the existing rule, maybe this one https://npmpackagejsonlint.org/docs/en/rules/type/engines-type With the above helpful things out of the way, I'm not sure what problem adding Volta solves here, is it just an alternative to nvm, like pnpm or yarn is to npm? We've avoided adding Yarn for what I guess is similar reasons of over complicating project tooling by adding the latest shiny tool. Aside: I use Fish Shell, and nvm is horrible in Fish, so personally I like the sound of Volta, but the above should still stand so that we don't end up adding and having to support numerous tool chains just for the sake of it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea :)
5c2259c
to
8e69199
Compare
I've just taken a quick look at any previous conversation in Slack
I think this stands still has yet to be answered either here or in Slack Right now even from the latest commit And to note, I'm not against adding support for additional tooling, whilst the original Prettier proposal was closed and not implemented in early 2018 (primarily because of the amount of code churn) Prettier support was subtly added and supported for developers who were using in their personal local setups #4628 (comment) If Volta can be used in a similar fashion locally until it gets more visibility and use from development teams then switching from nvm to Volta could be a possibility, but right now there's not that usage and this PR is simply adding additional maintenance burdens on the project IMHO |
I agree we shouldn't add whatever tool anyone wants to. This should be brought up and discussed at a meeting, I haven't had the time or energy to champion it personally. My team has been using volta and we're finding it to be a compelling improvement over nvm. There is a very concrete case volta fixes right now. Gutenberg is stuck between There's another interesting case, the I don't really expected WordPress to drop nvm and adopt volta in one go. But if folks try it (and have the opportunity to experience it!) I believe they'll find it compelling. I'm quite confident it would better serve us as the recommended tool over nvm. Regarding additional maintenance burden, it's true there is some. You need to run |
I'll close this. I still think volta is compelling and would benefit Gutenberg but it needs someone to champion the effort. |
Description
Volta is a JavaScript toolchain management tool similar to nvm.
This PR adds first-class Volta support by adding a
volta
property to package.json files. This is known as "pinning" in volta and ensures that Volta users will use the versions of Node.js, npm, and script dependencies (stuff innode_modules/.bin
) as defined by the project.This should have no negative impact on folks continuing to use nvm.
How has this been tested?
From the project root or anywhere in the project, run
volta list
and you should see the expected output:Screenshots
Types of changes
Checklist: