BREAKING: Bump to Node 16 by FrederikBolding · Pull Request #1262 · MetaMask/core

FrederikBolding · 2023-04-25T10:13:48Z

Description

Bumps the minimum Node version of the packages to 16.18.24.

Changes

BREAKING: Bumps the minimum Node version of all packages to 16.18.24

legobeat

lgtm!

FrederikBolding · 2023-04-25T10:32:09Z

ty for the updates! I swear I looked for docs that mentioned Node 14 but didn't find any 😅

legobeat · 2023-04-25T10:32:18Z

...However, should we make sure each package has a non-major release published before merging this @FrederikBolding ? Considering this repo does not have as straightforward a flow for backports, we might as well get any pending non-breaking changes published first.

WDYT?

FrederikBolding · 2023-04-25T10:33:55Z

...However, should we make sure each package has a non-major release published before merging this @FrederikBolding ? Considering this repo does not have as straightforward a flow for backports, we might as well get any pending non-breaking changes published first.
WDYT?

Both the extension and mobile are to my knowledge ready for Node 16. So not sure if there is a reason to wait? 🤔 As in, why would be doing backports anyway?

legobeat · 2023-04-25T10:36:30Z

I don't have a scenario in mind - I just view is as standard practice and you never know when The Unknown Unknown rears a head (and version rollbacks wouldn't be unheard of I think).

And in the off-chance we have external dependents of these packages with different runtime constraints, this may make a big change for such users.

FrederikBolding · 2023-04-25T10:39:31Z

I don't have a scenario in mind - I just view is as standard practice and you never know when The Unknown Unknown rears a head.
And in the off-chance we have external dependents of these packages with different runtime constraints, this may be a big change for such users.

Fair enough, I am merely trying to unblock #1184 and thought it was easier to do a release after all had been merged. But I'm also fine with us publishing a release first. Doesn't look like many packages have outstanding changes though

Gudahtt · 2023-04-25T20:51:38Z


-> **Note** 
-> `workspaceName` in these commands is the `name` field within a package's `package.json`, e.g., `@metamask/address-book-controller`, not the directory where it is located, e.g., `packages/address-book-controller`.
+> **Note** > `workspaceName` in these commands is the `name` field within a package's `package.json`, e.g., `@metamask/address-book-controller`, not the directory where it is located, e.g., `packages/address-book-controller`.


Nit: Seems that a newline was accidentally removed here
Suggested change
> **Note** > `workspaceName` in these commands is the `name` field within a package's `package.json`, e.g., `@metamask/address-book-controller`, not the directory where it is located, e.g., `packages/address-book-controller`.
> **Note**
> `workspaceName` in these commands is the `name` field within a package's `package.json`, e.g., `@metamask/address-book-controller`, not the directory where it is located, e.g., `packages/address-book-controller`.

lint is drunk

Yeah, not sure what is going on here. This is what Prettier wants

Gudahtt · 2023-04-25T20:52:57Z

I have applied the DO-NOT-MERGE label for now, until the next release is published. I'll ensure we get it published tomorrow.

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

FrederikBolding · 2023-04-26T12:15:38Z

This should be good to review now!

socket-security · 2023-04-26T12:16:08Z

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

👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

📊 Modified Dependency Overview:

🚮 Removed packages:@types/node@14.18.43

Gudahtt

LGTM!

legobeat

LGTM

Gudahtt · 2023-04-26T12:32:35Z

Looks like we need to run lint:fix

FrederikBolding · 2023-04-26T12:42:21Z

@Gudahtt That is what resulted in the contributing.md change that you requested reverted 😅

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

The `AbortController` polyfill has not been required since updating to a minimum Node.js version of v16 in #1262. This API has been built into Node.js since v15.4.0, and is included in all versions of v16. It is also supported by all browsers that our extension supports, and has been supported by React Native since v0.60.0. This helps unblock the TypeScript update to v4.8.4. The update resulted in some type errors from this polyfill package.

## Explanation The `AbortController` polyfill has not been required since updating to a minimum Node.js version of v16 in #1262. This API has been [built into Node.js since v15.4.0](https://nodejs.org/docs/latest-v16.x/api/globals.html#class-abortcontroller), and is included in all versions of v16. It is also supported by [all browsers that our extension supports](https://developer.mozilla.org/en-US/docs/Web/API/AbortController#browser_compatibility), and has been [supported by React Native since v0.60.0](https://github.com/react-native-community/releases/blob/master/CHANGELOG.md#v0600). This helps unblock the TypeScript update to v4.8.4. The update resulted in some type errors from this polyfill package. ## References None ## Changelog ### `@metamask/assets-controllers` ### Removed - **BREAKING:** Remove AbortController polyfill - This package now assumes that the AbortController global exists ## 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

Bump to Node 16
c225084

FrederikBolding requested a review from a team as a code owner April 25, 2023 10:13

legobeat added 2 commits April 25, 2023 19:28

devDeps: @types/node@14.14.31->16.18.24
c6cd14c

docs: update nodejs version in contributing.md
156fb83

legobeat previously approved these changes Apr 25, 2023
View reviewed changes

Fix lint
edda2d1

FrederikBolding dismissed legobeat’s stale review via edda2d1April 25, 2023 10:35

legobeat mentioned this pull request Apr 25, 2023
Release 49.0.0 #1263
Merged

Gudahtt reviewed Apr 25, 2023
View reviewed changes

Gudahtt added the DO-NOT-MERGE label Apr 25, 2023

Gudahtt removed the DO-NOT-MERGE label Apr 26, 2023

FrederikBoldingand others added 2 commits April 26, 2023 14:15

Update docs/contributing.md
0091b1d
Co-authored-by: Mark Stacey <markjstacey@gmail.com>

Merge branch 'main' into fb/bump-to-node16
a6e5c89

Gudahtt previously approved these changes Apr 26, 2023
View reviewed changes

legobeat previously approved these changes Apr 26, 2023
View reviewed changes

Gudahtt reviewed Apr 26, 2023
View reviewed changes

Comment threaddocs/contributing.md

Update docs/contributing.md
c059e66
Co-authored-by: Mark Stacey <markjstacey@gmail.com>

FrederikBolding dismissed stale reviews from legobeat and Gudahtt via c059e66April 26, 2023 13:16

Gudahtt approved these changes Apr 26, 2023
View reviewed changes

Merge branch 'main' into fb/bump-to-node16
a93e27a

FrederikBolding merged commit 5a68e16 into mainApr 26, 2023

FrederikBolding deleted the fb/bump-to-node16 branch April 26, 2023 13:30

Gudahtt mentioned this pull request Sep 26, 2023
chore: Remove AbortController polyfill #1726
Merged
3 tasks

	> Note > `workspaceName` in these commands is the `name` field within a package's `package.json`, e.g., `@metamask/address-book-controller`, not the directory where it is located, e.g., `packages/address-book-controller`.
	> Note
	> `workspaceName` in these commands is the `name` field within a package's `package.json`, e.g., `@metamask/address-book-controller`, not the directory where it is located, e.g., `packages/address-book-controller`.

Uh oh!

Uh oh!

Conversation

FrederikBolding commented Apr 25, 2023• edited by legobeat Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Uh oh!

legobeat left a comment

Choose a reason for hiding this comment

Uh oh!

FrederikBolding commented Apr 25, 2023

Uh oh!

legobeat commented Apr 25, 2023

Uh oh!

FrederikBolding commented Apr 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

legobeat commented Apr 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FrederikBolding commented Apr 25, 2023

Uh oh!

GudahttApr 25, 2023

Choose a reason for hiding this comment

Uh oh!

legobeatApr 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FrederikBoldingApr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Gudahtt commented Apr 25, 2023

Uh oh!

FrederikBolding commented Apr 26, 2023

Uh oh!

socket-securityBot commented Apr 26, 2023

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

legobeat left a comment

Choose a reason for hiding this comment

Uh oh!

Gudahtt commented Apr 26, 2023

Uh oh!

FrederikBolding commented Apr 26, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

FrederikBolding commented Apr 25, 2023•
edited by legobeat
Loading

FrederikBolding commented Apr 25, 2023•
edited
Loading

legobeat commented Apr 25, 2023•
edited
Loading

legobeatApr 26, 2023•
edited
Loading