core
core copied to clipboard
Bump min Node version to 18.18; use LTS for dev
Explanation
Now that both extension and mobile are using Node 18, we can follow suit.
Changelog
For all packages
- BREAKING: Bump minimum Node version to 18.18
Checklist
- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Note that this would require major versions for everything all over again, so there's no immediate urgency to merge this, but I want to go ahead and start the conversation :)
Note: packages from these repos are still supporting node v16 and depending on at least one of these packages:
- [ ] https://github.com/MetaMask/metamask-desktop
- [x] https://github.com/MetaMask/swaps-controller
- [x] ~https://github.com/MetaMask/eth-snap-keyring~
- [x] ~https://github.com/MetaMask/keyring-api~
- [x] ~https://github.com/MetaMask/ppom-validator~
- [x] ~https://github.com/MetaMask/smart-transactions-controller~
EDIT: Updated with progress
Thank you! I will work through that list then.
Setting this to draft in the meantime.
I've created tickets for the above repos, aside from metamask-desktop (it's been put on ice, so I don't think we have to worry about that right now).
Shouldn't we bump the minimum version here first, before bumping the dependencies? We'd want to increase the minimum from the bottom-up, starting at the clients, then here, then to dependencies. Otherwise v16 support will effectively be dropped here prior to the supported range changing.
Oh I see, sorry I misunderstood. That list is for projects depending on this repository, similar to extension/mobile, so yes we'd want to update those first.
@Gudahtt (Oops I deleted my comment thinking I had misread @legobeat's comment) I think what @legobeat is saying is that the packages he linked to depend on some packages in the core monorepo, i.e., the core packages are the dependencies.
I've updated this PR so that it is ready to go when we merge https://github.com/MetaMask/smart-transactions-controller/issues/249.
I've merged in changes from main and fixed lint violations. Ready for review again.
In other packages we've been using "^18.18 || >=20" instead of ">=18.18 || >=20" in the engines field and yarn constraints. Are we purposefully going for a stronger constraint in core?
@MajorLift No, this was just an oversight 🤦🏻 I've fixed the versions to actually follow the module template.
@Gudahtt mentioned that we may want to merge https://github.com/MetaMask/core/pull/3908 before merging this PR. Adding DO-NOT-MERGE in the meantime.
This branch became stale, so I rebased it against the latest changes in main, and squashed all of the commits.
Merging this will kick off another round of breaking changes. Feel free to approve this, but we will see if we can pair this with some other breaking changes that we want to make anyway.
No dependency changes detected. Learn more about Socket for GitHub ↗︎
👍 No dependency changes detected in pull request
Update on this PR: This is something that we still want to do, but we are waiting until we finish some fixes we want to make to the keyring controller, and I am also heads down on work to support multichain for the next few weeks or so. After we get through those, though, I can start the process to merge this PR and cut new major releases for everything.