Use `tsup` for bundling #144

Mrtenz · 2023-10-09T09:40:46Z

It turns out doing hybrid ESM+CJS builds is quite complicated, since the requirements for ESM are a lot more strict than CommonJS, resulting in issues like MetaMask/key-tree#152, MetaMask/key-tree#144. This made the build more complicated than it should be:

We had to run SWC (or TSC) twice: Once for the ESM build and once for the CommonJS build.
We had to add a package.json to the dist/esm folder, since the file was .js and not .mjs.
We had to run TypeScript separately to get type checking.

And after all of that, the ESM build was still invalid. Some bundlers would accept it (like Webpack, with the fullySpecified rule disabled), but it wouldn't work in native ESM environments, e.g., when running Node.js with ESM. To solve this, we would have to change all imports throughout the code to use the .js extension:

- import { assert } from './assert';
- import { add0x, assertIsHexString } from './hex';
+ import { assert } from './assert.js';
+ import { add0x, assertIsHexString } from './hex.js';

Which is not very pretty, and would be a lot of work to change in all codebases, since the ESLint rule to enforce this is not automatically fixable.

Instead, I suggest we use tsup. tsup is built on top of esbuild, which has comparable performance to SWC. It's designed with hybrid builds in mind, meaning that we can significantly simplify our build process:

All files are bundled into a single entry (dist/index.js and dist/index.mjs), meaning that relative imports are no longer a concern. External imports (from libraries) are kept as-is.
The ESM build has the proper .mjs extension, meaning that we don't need to create a dummy package.json.
It supports generating declaration files with TSC, so we can remove that step from the build process.
It has special features to ensure support for both CJS and ESM, like replacing __dirname with import.meta.url in the ESM build.
All build scripts can be replaced with a single command.

I think this is the best way forward to support hybrid builds, while avoiding adding more complexity, though I'm open for other suggestions.

socket-security · 2023-10-09T09:50:03Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
tsup	7.2.0	eval, network, filesystem, shell, environment	`+46`	213 MB	egoist

🚮 Removed packages: @swc/cli@0.1.62, @swc/core@1.3.93

socket-security · 2023-10-09T09:50:06Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: source-map@0.8.0-beta.0, esbuild@0.18.20

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

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

Mrtenz · 2023-10-09T10:26:12Z

@SocketSecurity ignore source-map@0.8.0-beta.0
@SocketSecurity ignore esbuild@0.18.20

Nothing out of the ordinary here.

Mrtenz · 2023-10-09T14:33:36Z

I verified that this works in both CommonJS and ESM. You can check by creating a simple JS file (with .mjs extension), and importing @metamask/utils in it. On main, this results in the error below, while it works on this PR.

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '[...]/node_modules/@metamask/utils/dist/esm/assert' imported from [...]/node_modules/@metamask/utils/dist/esm/index.js

mcmire

Overall this looks like a fabulous solution, and I like that it simplifies the process and config. I just had a comment about declaration maps, which you can read below.

mcmire · 2023-10-10T20:24:33Z

package.json

  ],
  "scripts": {
-    "build": "yarn build:source && yarn build:types",
-    "build:cjs": "swc src --out-dir dist/cjs --config-file .swcrc.build.json --config module.type=commonjs",
-    "build:clean": "rimraf dist && yarn build",


Ah, I see that the clean: true option in tsup.config.ts handles this. Nice.

tsconfig.build.json

mcmire · 2023-10-12T18:15:52Z

@Mrtenz I checked the types/ directory and it seems to match what was in there before, so that's good.

However, we now have a single file for index.mjs. Is this right? I thought the whole goal of ESM was that the one-module-per-file pattern could be preserved. It seems like if we have all code in one file, we lose the benefits of ESM (I'm thinking primarily of tree shaking, although there may be other benefits).

mcmire · 2023-10-13T01:32:33Z

Ah, I see a similar question was left here: MetaMask/key-tree#152 (comment). Do you still feel the same way?

Mrtenz · 2023-10-13T11:20:13Z

Ah, I see a similar question was left here: MetaMask/key-tree#152 (comment). Do you still feel the same way?

I wasn't sure of it, so I did some testing with a Webpack project importing a function from @metamask/utils. It turns out it does make a difference. Fortunately, tsup supports multiple files as entry, so we can do something like entry: ['src/**/*.ts']. It will update the import extensions for each file to .mjs for the ESM bundle, so that should work fine. That results in a folder structure like this:

dist/
├─ types/
│  ├─ index.d.ts
│  ├─ index.d.ts.map
│  ├─ file-a.d.ts
│  ├─ file-a.d.ts.map
│  ├─ etc.
├─ index.js
├─ index.js.map
├─ index.mjs
├─ index.mjs.map
├─ foo.js
├─ foo.js.map
├─ foo.mjs
├─ foo.mjs.map
├─ chunk-ABCDEF.js (chunk for `foo.js`)
├─ chunk-ABCDEF.js.map
├─ chunk-ABCDEF.mjs (chunk for `foo.mjs`)
├─ chunk-ABCDEF.mjs.map

foo.(m)js in this case imports the functions from the chunk and just exports them, and the chunk has the actual code that was in foo.ts. I'm not sure why it uses the chunk files though. I'll play around with the config a bit more to see if we can do it without those.

Mrtenz · 2023-10-16T08:09:30Z

package.json

@@ -11,30 +11,24 @@
    "url": "https://github.com/MetaMask/utils.git"
  },
  "license": "ISC",
+  "sideEffects": false,


We use this option in the module template, but it wasn't enabled in utils yet.

Mrtenz · 2023-10-16T08:11:18Z

tsconfig.build.json

@@ -18,6 +18,7 @@
    "./src/**/__tests__/**/*",
    "./src/**/__snapshots__/**/*",
    "./src/**/*.test.ts",
+    "./src/**/*.test-d.ts",


test-d.ts files were included in the dist folder before. I've added it to the excludes here and in tsup.config.ts.

Oops, good catch.

Mrtenz · 2023-10-16T08:12:51Z

foo.(m)js in this case imports the functions from the chunk and just exports them, and the chunk has the actual code that was in foo.ts. I'm not sure why it uses the chunk files though. I'll play around with the config a bit more to see if we can do it without those.

I couldn't find a way to disable the chunk files, but I think it's fine as-is?

mcmire

I think the chunk files are fine too. Thanks for verifying that this approach works. I'm good with this!

This changes all packages to be built with `tsup`, instead of SWC. `tsup` uses `esbuild` under the hood, so performance should be comparable. More context here: MetaMask/utils#144.

## Explanation This changes the build system to build all packages with `tsup`, instead of `tsc`. `tsc` is still used for generating the declaration files and type checking, and `tsup` is used for building the `.js` and (new) `.mjs` files. The benefit of this is that we now have a ESM build as well as a CJS build. ## References See MetaMask/utils#144. ## Changelog  ### `@metamask/accounts-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/address-book-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/announcement-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/approval-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/assets-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/base-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/build-utils` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/composable-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/controller-utils` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/ens-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/eth-json-rpc-provider` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/gas-fee-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/json-rpc-engine` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/json-rpc-middleware-stream` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/keyring-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/logging-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/message-manager` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/name-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/network-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/notification-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/permission-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/permission-log-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/phishing-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/polling-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/preferences-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/queued-request-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/rate-limit-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/selected-network-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/signature-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/transaction-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ### `@metamask/user-operation-controller` - **BREAKING**: Add ESM build. - It's no longer possible to import files from `./dist` directly. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] 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

This changes all packages to be built with `tsup`, instead of SWC. `tsup` uses `esbuild` under the hood, so performance should be comparable. More context here: MetaMask/utils#144.

#2445) ## Explanation As part of the Wallet Framework Team's OKR ([Q2 2024 O3KR4](https://docs.google.com/document/d/1NYWepE--9HLG9NHAxmsHKQI7lfZL2nW0kv7UVd4q160/edit#bookmark=kix.h65bod9heydk), [Q3 2024 O2KR4](https://docs.google.com/document/d/1JLEzfUxHlT8lw8ntgMWG0vQb5BAATcrYZDj0wRB2ogI/edit#bookmark=kix.yxz96lxhvjk3)) for upgrading TypeScript to v5.0+ in the core monorepo, we are updating dependencies of the core repo so that they can be used with projects that use `Node16` or `NodeNext` as their `moduleResolution` tsconfig option. To achieve this, all dependencies that are ESM packages need to be updated so that they generate separate builds and type declarations that are explicitly designated for CJS and ESM. This requirement applies to nested dependencies as well, so we are also replacing `superstruct` with the ESM-compatible fork `@metamask/superstruct` in all dependencies of core that have `superstruct` as a dependency. ## Description - [x] Replace `superstruct` dependency with `@metamask/superstruct` `^3.0.0`. - [x] `^3.1.0` - [x] Replace all `superstruct` import statements with `@metamask/superstruct` - [x] Bump `@metamask/utils` to `^8.5.0`. - [x] `^9.1.0` - [x] remove yarn resolution to `@metamask/superstruct@npm:3.1.0` - [ ] ~If feasible without too much additional work:~ -> create separate PRs for these tasks - [ ] ~Bump `typescript` to `~5.0.4`~ - [ ] ~#2514 Further context on why the `superstruct` and `utils` changes are necessary: - MetaMask/utils#144 - MetaMask/superstruct#1 - MetaMask/superstruct#18 - MetaMask/utils#182 - MetaMask/metamask-module-template#247 ### `yarn resolutions` `@metamask/utils` is pinned to `^9.1.0` via yarn resolutions, as there are a large number of dependencies that are set to `^8.5.0` (see below), and some of them (especially the core packages) are blocked by the merge and release of this PR: - core monorepo: `approval-controller`, `base-controller`, `controller-utils`, `eth-json-rpc-provider`, `json-rpc-engine`, `json-rpc-middleware-stream`, `permission-controller` - standalone: `browser-passworder`, `create-release-branch`, `eth-block-tracker`, `eth-json-rpc-middleware`, `eth-sig-util`, `key-tree`, `post-message-stream`, `providers`, `snaps-registry` Mixed usage of `utils` v8 and v9 anywhere in the monorepo's dependency tree causes the following type errors: - https://github.com/MetaMask/snaps/actions/runs/9720788583/job/26900545288?pr=2445 ### Release order roadmap Due to interdependencies between the packages involved in this PR, we will need to update and release them in a specific order: - [ ] Merge this PR: #2445 - Request snaps team to hold off on releases until yarn resolution for utils can be removed - Bump `@metamask/utils` in dependencies of `snaps-{sdk,utils,rpc-methods}`: - [x] `rpc-errors`: - [x] MetaMask/rpc-errors#147 - [x] MetaMask/rpc-errors#148 - [x] `key-tree` - [x] MetaMask/key-tree#181 - [x] MetaMask/key-tree#182 - [x] `snaps-registry` - [x] MetaMask/snaps-registry#693 - [x] MetaMask/snaps-registry#694 - [x] `base-controller`, `permission-controller` - [x] MetaMask/core#4516 - [x] MetaMask/core#4517 - [ ] Release `snaps-sdk` - [x] Release `keyring-api`: MetaMask/keyring-api#328 - [ ] Release `snaps-utils` - [ ] Release `snaps-rpc-methods` - [ ] Merge core PR: MetaMask/core#3645 - Before merging, first remove yarn resolutions for `snaps-sdk`, `snaps-utils`, `keyring-api` - [ ] Release all core pkgs (especially deps of `snaps-controllers` and consumers of `utils`) - Exclude core pkgs that have `snaps-controllers` as dependency: `{accounts,chain,profile-sync}-controller` - [ ] Bump and release all remaining `@metamask/utils@8.x.x` usage in the `snaps-controllers` dependency tree - [x] `browser-passworder` - [x] MetaMask/browser-passworder#63 - [x] MetaMask/browser-passworder#64 - [x] `post-message-stream` - [x] MetaMask/post-message-stream#140 - ~Blocked by build error https://github.com/MetaMask/post-message-stream/actions/runs/9863225191/job/27235524010?pr=140~ - [x] MetaMask/post-message-stream#141 - [ ] Release `snaps-controllers` - [ ] Release `{accounts,chain,profile-sync}-controller`, `eth-snap-keyring` (MetaMask/eth-snap-keyring#311) - [ ] Bump and release all remaining `@metamask/utils@8.x.x` usage in the snaps monorepo dependency tree - [ ] `create-release-branch` - [x] MetaMask/create-release-branch#150 - [x] MetaMask/create-release-branch#149 - [ ] `eth-block-tracker` - ~Blocked by `eth-json-rpc-provider`~ - [ ] Blocked by MetaMask/eth-block-tracker#252 - [ ] `eth-json-rpc-middleware` - Blocked by `eth-block-tracker`, ~`eth-json-rpc-provider`,~ `eth-sig-util`, ~`json-rpc-engine`~ - [x] `abi-utils` - [x] MetaMask/abi-utils#80 - [x] MetaMask/abi-utils#81 - [ ] `eth-sig-util` - ~Blocked by `abi-utils`~ - [x] MetaMask/eth-sig-util#381 - [x] MetaMask/eth-sig-util#382 - [x] `providers` - ~Blocked by `json-rpc-engine`, `json-rpc-middleware-stream`, `rpc-errors`~ - [x] MetaMask/providers#345 - [x] MetaMask/providers#347 - [ ] Remove yarn resolution for `@metamask/utils` - [ ] Release all remaining snaps pkgs - New snaps monorepo releases are now safe to publish. ## References - Closes #2444 - Followed by #2514 - Blocked by type errors caused by simultaneous usage of `@metamask/utils` `9.0.0` and `8.5.0` in dependency tree. - All tests passing with `@metamask/utils` fixed to `9.0.0` in yarn resolutions: https://github.com/MetaMask/snaps/actions/runs/9745954683/job/26895208572?pr=2445 - [ ] `snaps-sdk`, `snaps-utils` and their dependencies that use `@metamask/utils@8.5.0` are blocking: - [ ] core release via MetaMask/core#3645, which is blocking: - [ ] `snaps-controllers` type errors - [ ] `snaps-rpc-methods` type errors - [ ] `{accounts,chain,profile-sync}-controller` as dependencies - `@metamask/providers` update to `^17.1.0` is being blocked by MetaMask/providers#340, ts-bridge/ts-bridge#22 - Causes CI failure: https://github.com/MetaMask/snaps/actions/runs/9783767567/job/27013136688?pr=2445 - [x] Fix: MetaMask/providers#347 ## Changelog ### `@metamask/snaps-cli` ```md ### Changed - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. ``` ### `@metamask/snaps-controllers` ```md ### Changed - Bump `@metamask/base-controller` from `^6.0.0` to `^6.0.1` ([#2445](#2445)) - Bump `@metamask/permission-controller` from `^10.0.0` to `^10.0.1` ([#2445](#2445)) - Bump `@metamask/rpc-errors` from `^6.2.1` to `^6.3.1` ([#2445](#2445)) - Bump `@metamask/snaps-registry` from `^3.1.0` to `^3.2.1` ([#2445](#2445)) - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ``` ### `@metamask/snaps-execution-environments` ```md ### Changed - Bump `@metamask/rpc-errors` from `^6.2.1` to `^6.3.1` ([#2445](#2445)) - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. - Set `@metamask/providers` from `^17.0.0` to `17.0.0` ([#2445](#2445)) - `17.1.0` and `17.1.1` introduce regressions. ``` ### `@metamask/snaps-jest` ```md ### Changed - Bump `@metamask/base-controller` from `^6.0.0` to `^6.0.1` ([#2445](#2445)) - Bump `@metamask/key-tree` from `^9.1.1` to `^9.1.2` ([#2445](#2445)) - Bump `@metamask/permission-controller` from `^10.0.0` to `^10.0.1` ([#2445](#2445)) - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. ``` ### `@metamask/snaps-rpc-methods` ```md ### Changed - Bump `@metamask/key-tree` from `^9.1.1` to `^9.1.2` ([#2445](#2445)) - Bump `@metamask/permission-controller` from `^10.0.0` to `^10.0.1` ([#2445](#2445)) - Bump `@metamask/rpc-errors` from `^6.2.1` to `^6.3.1` ([#2445](#2445)) - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. ``` ### `@metamask/snaps-sdk` ```md ### Changed - Bump `@metamask/key-tree` from `^9.1.1` to `^9.1.2` ([#2445](#2445)) - Bump `@metamask/rpc-errors` from `^6.2.1` to `^6.3.1` ([#2445](#2445)) - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. - Set `@metamask/providers` from `^17.0.0` to `17.0.0` ([#2445](#2445)) - `17.1.0` and `17.1.1` introduce regressions. ``` ### `@metamask/snaps-simulator` (major) ```md ### Changed - **BREAKING:** Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) - Due to the return type of `bigIntToHex` being narrowed from `string` to `Hex`, the return type of `hexlifyTransactionData` is narrowed from an object of type `Record<keyof transaction, string>` to an object of type `Record<keyof transaction, Hex>`, where `transaction` is of type `Omit<TransactionFormData, 'transactionOrigin' | 'chainId'>` - Bump `@metamask/base-controller` from `^6.0.0` to `^6.0.1` ([#2445](#2445)) - Bump `@metamask/key-tree` from `^9.1.1` to `^9.1.2` ([#2445](#2445)) - Bump `@metamask/permission-controller` from `^10.0.0` to `^10.0.1` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. ``` ### `@metamask/snaps-utils` ```md ### Changed - Bump `@metamask/base-controller` from `^6.0.0` to `^6.0.1` ([#2445](#2445)) - Bump `@metamask/key-tree` from `^9.1.1` to `^9.1.2` ([#2445](#2445)) - Bump `@metamask/permission-controller` from `^10.0.0` to `^10.0.1` ([#2445](#2445)) - Bump `@metamask/rpc-errors` from `^6.2.1` to `^6.3.1` ([#2445](#2445)) - Bump `@metamask/snaps-registry` from `^3.1.0` to `^3.2.1` ([#2445](#2445)) - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#2445](#2445)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. ``` ### `@metamask/snaps-webpack-plugin` ```md ### Changed - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ``` ### `@metamask/test-snaps` ```md ### Changed - Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#2445](#2445)) ### Fixed - Set `@metamask/providers` from `^17.0.0` to `17.0.0` ([#2445](#2445)) - `17.1.0` and `17.1.1` introduce regressions. ```

…ode16` (#3645) ## Explanation ### TypeScript v5.0 As part of the Wallet Framework Team's OKRs (Q2 2024 O3KR4, Q3 2024 O2KR4), we are upgrading TypeScript to v5.0+ for all packages in the core monorepo. These upgrades will give us access to new features, aid us in writing more type-safe and modern code, and also allow us to reach parity with Extension and other MetaMask projects. ### `Node16` In order to maximize the benefits of this upgrade, we are also enabling the `Node16` setting for the `module` and `moduleResolution` tsconfig options. ## Motivation ### Interop: CJS modules referencing ESM modules The core monorepo is a collection of CJS packages, which use CJS module resolution rules internally, and are treated as CJS modules by Node.js. This is true despite that fact that these packages are authored in TypeScript using ESM syntax (`import`/`export` statements) and are set up to output dual builds for both CJS and ESM. With `Node16` or `NodeNext` enabled, CJS modules are unable to reference ESM modules via static/synchronous `import` statements, as TypeScript assumes them to be compiled down to CJS-only `require` statements. There are three solutions for this issue, of which we are utilizing the first two: 1. Update the ESM-only dependency so that it outputs a CJS build and type declaration as well. 2. Replace the static `import` statements with dynamic import syntax (which, based on CJS emit rules, are not transformed to `require` statements). 3. Migrate our module to ESM (by setting `"type": "module"` in package.json and renaming all of our scripts to *.cjs) > See https://www.typescriptlang.org/docs/handbook/modules/reference.html#interoperability-rules With dependencies that we control or use extensively, we pursue the first option, as we're doing with `superstruct` and `@metamask/utils` (and all of the many core dependencies that are downstream of either or both). With dependencies that see more limited usage, we are opting for either the second option (e.g. `multiformats`) or, if available, using a CJS-compatible alternative (e.g. replacing `lodash-es` with `lodash`). The third solution of migrating to ESM is the most fundamental, long-term measure, but we are refraining from it at this stage until we can make a concerted effort to migrate our codebase as a whole. This is because any individual ESM migration can cause a cascading effect through the dependency tree where other packages are now required to migrate as well. ### Reasons for moving away from `node`/`node10` The following outlines the motivation for switching to `Node16`, backed by relevant entries from the official TypeScript documentation. - Starting with TypeScript v5.0, the previously used `moduleResolution` setting `node` is renamed to `node10`, and strongly discouraged from usage. - "It [reflects the CommonJS module resolution algorithm as it existed in Node.js versions earlier than v12](https://www.typescriptlang.org/docs/handbook/modules/reference.html#node10-formerly-known-as-node). **It should no longer be used.**" - "Because `node16` and `nodenext` are the only module options that reflect the complexities of Node.js’s dual module system, they are the [**only correct module options** for all apps and libraries that are intended to run in Node.js v12 or later](https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext), whether they use ES modules or not." - The `node10` setting is unable to guarantee correct module resolution for ESM-only dependencies. - "[Because Node.js v12 introduced different module resolution rules for ES modules,](https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution) though, it’s a very bad model of modern versions of Node.js. It should not be used for new projects." - "`node16` and `nodenext` describe the full range of behavior for Node.js’s dual-format module system, and **emit files in either CommonJS or ESM format**. This is different from every other `module` option, which are runtime-agnostic and force all output files into a single format, [leaving it to the user to ensure the output is valid for their runtime.](https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext)" - The `node10` setting does not support the package.json `"exports"` field, which is used in our libraries to expose dual builds and type declarations. - https://www.typescriptlang.org/docs/handbook/modules/reference.html#packagejson-exports - The `Node16` and `NodeNext` settings maximize downstream compatibility. - "When compiling a library, you don’t know where the output code will run, but you’d like it to run in as many places as possible. Using "module": "nodenext" (along with the implied "moduleResolution": "nodenext") is the best bet for maximizing the compatibility of the output JavaScript’s module specifiers, since [it will force you to comply with Node.js’s stricter rules for import module resolution.](https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution-for-libraries)" - "`"moduleResolution": "nodenext"` is only checking that the output works in Node.js, but in most cases, [module code that works in Node.js will work in other runtimes and in bundlers](https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution-for-libraries)" #### `Node16` vs. `NodeNext` The two settings are currently identical, and `Node16` is intended to work with all current node versions v16 or higher. If additional capabilities are added to `NodeNext` or `Node18`/`Node20` that we want to apply to our codebases, we will be able to introduce them after checking for disruptive regressions or breaking changes. #### Relationship with other tsconfig options - `--module` `nodenext` or `node16` implies and enforces the `moduleResolution` with the same name (and vice versa). - `--module` `node16` implies (up to) `--target` `es2022`. - `--module` `nodenext` implies (up to) `--target` `esnext`. > https://www.typescriptlang.org/docs/handbook/modules/reference.html#implied-and-enforced-options ## Description - [x] Replace `superstruct` dependency with `@metamask/superstruct` `^3.0.0`. - [x] `^3.1.0` - [x] Replace all `superstruct` import statements with `@metamask/superstruct` - [x] Bump `@metamask/utils` to `^8.5.0`. - [x] `^9.0.0` - [x] remove yarn resolution to `@metamask/superstruct@npm:3.1.0` - [x] Bump `typescript` to `~5.0.4` - [x] Set `module` and `moduleResolution` tsconfig options to `Node16` - [ ] ~#4507 Further context on why the `superstruct` and `utils` changes are necessary: - MetaMask/utils#144 - MetaMask/superstruct#1 - MetaMask/superstruct#18 - MetaMask/utils#182 - MetaMask/metamask-module-template#247 - https://www.typescriptlang.org/docs/handbook/modules/guides/choosing-compiler-options.html#considerations-for-bundling-libraries ## Release order roadmap Due to interdependencies between the packages involved in this PR, we will need to update and release them in a specific order: - [x] #4516 - [x] Release `{base,permission}-controller` - [x] (wait for releases: `snaps-sdk`, `snaps-utils`, `snaps-controllers`, `keyring-api`) - [x] MetaMask/keyring-api#356 - [x] MetaMask/snaps#2445 - [x] MetaMask/snaps#2584 - [x] MetaMask/snaps#2589 - [x] Remove yarn resolutions for `snaps-sdk`, `snaps-utils`, ~`keyring-api`~ - [ ] Merge this PR: #3645 - [x] Set yarn resolution for `@metamask/providers` via `@metamask/snaps-sdk` to `17.1.1` - [x] Leave messages in Changelog for affected packages (`{accounts,chain,profile-sync}-controller`) to hold off on new releases ```md ### Uncategorized - Please hold off on new releases of this package until the yarn resolution for `@metamask/providers` is removed. - This is blocked by a `@metamask/snaps-sdk` release with `@metamask/providers` bumped to `>=17.1.1`. - See: [Fix regressions introduced by @metamask/providers@17.1.1](MetaMask/snaps#2579) - Build error fixed by yarn resolution: [MetaMask/core/actions/runs/10011688901/job/27675682526?pr=3645](https://github.com/MetaMask/core/actions/runs/10011688901/job/27675682526?pr=3645) ``` - [ ] Release all core pkgs (especially deps of `snaps-controllers` and consumers of `utils`) - Exclude core pkgs that have `snaps-controllers` as dependency, and are affected by `@metamask/providers` yarn resolution - `{accounts,chain,profile-sync}-controller` - [ ] Release `snaps-controllers` - [ ] Release `{accounts,chain,profile-sync}-controller`, `eth-snap-keyring` (MetaMask/eth-snap-keyring#311) - [ ] (release remaining `snaps` packages while releasing `@metamask/utils@9.0.0` version bumps for all dependencies and nested dependencies) ## References - Contributes to #3651 - Blocked by: - `superstruct` - [x] MetaMask/superstruct#18 - [x] MetaMask/superstruct#24 - [x] MetaMask/superstruct#25 - [x] MetaMask/superstruct#28 - `utils` - [x] MetaMask/utils#185 - [x] MetaMask/utils#191 - [x] MetaMask/utils#194 - [x] MetaMask/utils#196 - Blocked by downstream consumers of `superstruct`, `utils`: - `abi-utils`: - [x] MetaMask/abi-utils#73 - [x] MetaMask/abi-utils#78 - [x] MetaMask/abi-utils#80 - [x] MetaMask/abi-utils#81 - `chain-api`: - [x] MetaMask/accounts-chain-api#5 - [x] https://github.com/MetaMask/accounts-chain-api/releases/tag/v0.1.0 - `eth-simple-keyring`: - [x] MetaMask/eth-simple-keyring#177 - [x] MetaMask/eth-simple-keyring#178 - `providers`: - [x] MetaMask/providers#336 - [x] MetaMask/providers#337 - Blocked by MetaMask/providers#340, ts-bridge/ts-bridge#22 - Causes CI failure: https://github.com/MetaMask/snaps/actions/runs/9783767567/job/27013136688?pr=2445 - [x] MetaMask/providers#345 - [x] MetaMask/providers#347 - `rpc-errors`: - [x] MetaMask/rpc-errors#147 - [x] MetaMask/rpc-errors#148 - `snaps-registry`: - [x] MetaMask/snaps-registry#613 - [x] MetaMask/snaps-registry#670 - [x] MetaMask/snaps-registry#693 - [x] MetaMask/snaps-registry#694 - Blocked by `snaps` monorepo releases - `snaps-sdk`, `snaps-utils` - `keyring-api` - [x] MetaMask/keyring-api#328 - `snaps-controllers` - `eth-snap-keyring` - [x] MetaMask/eth-snap-keyring#311 ## Changelog ### `@metamask/accounts-controller` ```md ### Changed - Bump `@metamask/keyring-api` from `^8.0.0` to `^8.0.1` ([#3645](#3645)) - Bump `@metamask/snaps-sdk` from `^4.2.0` to `^6.1.0` ([#3645](#3645)) - Bump `@metamask/snaps-utils` from `^7.4.0` to `^7.8.0` ([#3645](#3645)) - Bump peer dependency `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.0` ([#3645](#3645)) ``` ### `@metamask/assets-controllers` (major) ```md ### Changed - **BREAKING:** `getIpfsCIDv1AndPath`, `getFormattedIpfsUrl` are now async functions ([#3645](#3645)) - Add `immer` `^9.0.6` as a new dependenc. ([#3645](#3645)) - Bump `@metamask/abi-utils` from `^2.0.2` to `^2.0.3` ([#3645](#3645)) - Bump `multiformats` from `^9.5.2` to `^13.1.0` ([#3645](#3645)) ``` ### `@metamask/chain-controller` ```md ### Changed - Bump `@metamask/chain-api` from `^0.0.1` to `^0.1.0` ([#3645](#3645)) - Bump `@metamask/keyring-api` from `^8.0.0` to `^8.0.1` ([#3645](#3645)) - Bump `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.0` ([#3645](#3645)) - Bump `@metamask/snaps-sdk` from `^4.2.0` to `^6.1.0` ([#3645](#3645)) - Bump `@metamask/snaps-utils` from `^7.4.0` to `^7.8.0` ([#3645](#3645)) ``` ### `@metamask/keyring-controller` ```md ### Changed - Bump `@metamask/eth-simple-keyring` from `^6.0.1` to `^6.0.2` ([#3645](#3645)) - Bump `@metamask/keyring-api` from `^8.0.0` to `^8.0.1` ([#3645](#3645)) - Set yarn resolution for `@metamask/snaps-sdk/@metamask/providers` to `17.1.1` ([#3645](#3645)) - Remove once `@metamask/snaps-sdk` bumps its `@metamask/providers` version to `>=17.1.1`. ``` ### `@metamask/network-controller` (minor) ```md ### Added - Newly exports the following types: `AutoManagedNetworkClient`, `InfuraNetworkClientConfiguration`, `CustomNetworkClientConfiguration` ([#3645](#3645)) ``` ### `@metamask/profile-sync-controller` ```md ### Changed - Bump dependency and peer dependency `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.0` ([#3645](#3645)) - Bump `@metamask/snaps-sdk` from `^4.2.0` to `^6.1.0` ([#3645](#3645)) - Bump `@metamask/snaps-utils` from `^7.4.0` to `^7.8.0` ([#3645](#3645)) ``` ### `@metamask/transaction-controller` ```md ### Changed - Bump `@metamask/keyring-api` from `^8.0.0` to `^8.0.1` ([#3645](#3645)) ``` ### `@metamask/user-operation-controller` ```md ### Fixed - Replace `superstruct` with ESM-compatible `@metamask/superstruct` `^3.1.0` ([#3645](#3645)) - This fixes the issue of this package being unusable by any TypeScript project that uses `Node16` or `NodeNext` as its `moduleResolution` option. ``` ## 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 --------- Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

Mrtenz marked this pull request as ready for review October 9, 2023 14:23

Mrtenz requested a review from a team as a code owner October 9, 2023 14:24

Mrtenz mentioned this pull request Oct 9, 2023

bug: Invalid ESM build MetaMask/key-tree#152

Closed

Mrtenz force-pushed the mrtenz/tsup branch from 145d6fb to 3d99fd4 Compare October 10, 2023 07:58

kenhkan assigned Mrtenz Oct 10, 2023

mcmire reviewed Oct 10, 2023

View reviewed changes

Mrtenz commented Oct 16, 2023

View reviewed changes

Mrtenz added 8 commits October 16, 2023 10:11

Use tsup for bundling

73d2322

Simplify CI workflows

7507a19

Remove unused dependency

cf776a2

Remove unnecessary config option

01285e1

Use tsc for generating declarations

775f71d

Fix formatting of package.json

fe758df

Enable code splitting

befdf2d

Revert some changes

d6bdabd

Mrtenz force-pushed the mrtenz/tsup branch from 5fecc81 to d6bdabd Compare October 16, 2023 08:11

mcmire approved these changes Oct 16, 2023

View reviewed changes

Mrtenz merged commit 184d425 into main Oct 16, 2023
16 checks passed

Mrtenz deleted the mrtenz/tsup branch October 16, 2023 21:05

Mrtenz mentioned this pull request Nov 2, 2023

Use tsup for building MetaMask/metamask-module-template#229

Merged

mcmire mentioned this pull request Dec 12, 2023

BREAKING: Build packages as both CJS and ESM MetaMask/core#1480

Closed

3 tasks

Mrtenz mentioned this pull request Jan 22, 2024

Build packages with tsup MetaMask/snaps#2120

Merged

Mrtenz mentioned this pull request Feb 29, 2024

BREAKING: Build packages with tsup MetaMask/core#3998

Merged

3 tasks

This was referenced Jul 7, 2024

Upgrade to TypeScript v5.0 and set module{,Resolution} option to Node16 MetaMask/core#3645

Merged

Replace superstruct with ESM-compatible fork @metamask/superstruct MetaMask/snaps#2445

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `tsup` for bundling #144

Use `tsup` for bundling #144

Mrtenz commented Oct 9, 2023 •

edited

Loading

socket-security bot commented Oct 9, 2023 •

edited

Loading

socket-security bot commented Oct 9, 2023 •

edited

Loading

Mrtenz commented Oct 9, 2023

Mrtenz commented Oct 9, 2023

mcmire left a comment •

edited

Loading

mcmire Oct 10, 2023

mcmire commented Oct 12, 2023

mcmire commented Oct 13, 2023

Mrtenz commented Oct 13, 2023

Mrtenz Oct 16, 2023

Mrtenz Oct 16, 2023

mcmire Oct 16, 2023

Mrtenz commented Oct 16, 2023

mcmire left a comment

Use tsup for bundling #144

Use tsup for bundling #144

Conversation

Mrtenz commented Oct 9, 2023 • edited Loading

socket-security bot commented Oct 9, 2023 • edited Loading

socket-security bot commented Oct 9, 2023 • edited Loading

Next steps

Mrtenz commented Oct 9, 2023

Mrtenz commented Oct 9, 2023

mcmire left a comment • edited Loading

Choose a reason for hiding this comment

mcmire Oct 10, 2023

Choose a reason for hiding this comment

mcmire commented Oct 12, 2023

mcmire commented Oct 13, 2023

Mrtenz commented Oct 13, 2023

Mrtenz Oct 16, 2023

Choose a reason for hiding this comment

Mrtenz Oct 16, 2023

Choose a reason for hiding this comment

mcmire Oct 16, 2023

Choose a reason for hiding this comment

Mrtenz commented Oct 16, 2023

mcmire left a comment

Choose a reason for hiding this comment

Use `tsup` for bundling #144

Use `tsup` for bundling #144

Mrtenz commented Oct 9, 2023 •

edited

Loading

socket-security bot commented Oct 9, 2023 •

edited

Loading

socket-security bot commented Oct 9, 2023 •

edited

Loading

mcmire left a comment •

edited

Loading