core icon indicating copy to clipboard operation
core copied to clipboard

Bump min Node version to 18.18; use LTS for dev

Open mcmire opened this issue 1 year ago • 16 comments

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

mcmire avatar Dec 01 '23 22:12 mcmire

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 :)

mcmire avatar Dec 01 '23 22:12 mcmire

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

legobeat avatar Dec 02 '23 03:12 legobeat

Thank you! I will work through that list then.

mcmire avatar Dec 02 '23 03:12 mcmire

Setting this to draft in the meantime.

mcmire avatar Dec 05 '23 02:12 mcmire

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).

mcmire avatar Dec 05 '23 02:12 mcmire

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.

Gudahtt avatar Jan 10 '24 14:01 Gudahtt

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 avatar Jan 10 '24 16:01 Gudahtt

@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.

mcmire avatar Jan 10 '24 16:01 mcmire

I've updated this PR so that it is ready to go when we merge https://github.com/MetaMask/smart-transactions-controller/issues/249.

mcmire avatar Jan 25 '24 17:01 mcmire

I've merged in changes from main and fixed lint violations. Ready for review again.

mcmire avatar Feb 09 '24 17:02 mcmire

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 avatar Feb 09 '24 18:02 MajorLift

@MajorLift No, this was just an oversight 🤦🏻 I've fixed the versions to actually follow the module template.

mcmire avatar Feb 09 '24 18:02 mcmire

@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.

mcmire avatar Feb 09 '24 18:02 mcmire

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.

mcmire avatar Mar 15 '24 19:03 mcmire

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

socket-security[bot] avatar Mar 15 '24 19:03 socket-security[bot]

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.

mcmire avatar Apr 30 '24 15:04 mcmire