Skip to content

Use tsup for bundling - #144

Merged
Mrtenz merged 8 commits into
mainfrom
mrtenz/tsup
Oct 16, 2023
Merged

Use tsup for bundling#144
Mrtenz merged 8 commits into
mainfrom
mrtenz/tsup

Conversation

@Mrtenz

@MrtenzMrtenz commented Oct 9, 2023

Copy link
Copy Markdown
Member

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

socket-securityBot commented Oct 9, 2023

Copy link
Copy Markdown

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

PackagesVersionNew capabilitiesTransitivesSizePublisher
tsup7.2.0eval, network, filesystem, shell, environment+46213 MBegoist

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

@socket-security

socket-securityBot commented Oct 9, 2023

Copy link
Copy Markdown

👍 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

    Copy link
    Copy Markdown
    MemberAuthor

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

    Nothing out of the ordinary here.

    @Mrtenz
    Mrtenz marked this pull request as ready for review October 9, 2023 14:23
    @Mrtenz
    Mrtenz requested a review from a team as a code ownerOctober 9, 2023 14:24
    @Mrtenz

    Copy link
    Copy Markdown
    MemberAuthor

    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
    

    @mcmiremcmire left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    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.

    Comment threadpackage.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",

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

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

    Comment threadtsconfig.build.json
    @mcmire

    Copy link
    Copy Markdown
    Contributor

    @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

    Copy link
    Copy Markdown
    Contributor

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

    @Mrtenz

    Copy link
    Copy Markdown
    MemberAuthor

    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.

    Comment threadpackage.json
    "url": "https://github.com/MetaMask/utils.git"
    },
    "license": "ISC",
    "sideEffects": false,

    Copy link
    Copy Markdown
    MemberAuthor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

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

    Comment threadtsconfig.build.json
    "./src/**/__tests__/**/*",
    "./src/**/__snapshots__/**/*",
    "./src/**/*.test.ts",
    "./src/**/*.test-d.ts",

    Copy link
    Copy Markdown
    MemberAuthor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

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

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Oops, good catch.

    @Mrtenz

    Copy link
    Copy Markdown
    MemberAuthor

    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?

    @mcmiremcmire left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

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

    Mrtenz added a commit to MetaMask/snaps that referenced this pull request Feb 22, 2024
    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.
    Mrtenz added a commit to MetaMask/snaps that referenced this pull request Feb 22, 2024
    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.
    Mrtenz added a commit to MetaMask/snaps that referenced this pull request Feb 23, 2024
    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.
    Mrtenz added a commit to MetaMask/snaps that referenced this pull request Feb 26, 2024
    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.
    Mrtenz added a commit to MetaMask/core that referenced this pull request Mar 7, 2024
    ## 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
    <!--
    If you're making any consumer-facing changes, list those changes here as
    if you were updating a changelog, using the template below as a guide.
    (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
    FIXED. For security-related issues, follow the Security Advisory
    process.)
    Please take care to name the exact pieces of the API you've added or
    changed (e.g. types, interfaces, functions, or methods).
    If there are any breaking changes, make sure to offer a solution for
    consumers to follow once they upgrade to the changes.
    Finally, if you're only making changes to development scripts or tests,
    you may replace the template below with "None".
    -->
    ### `@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
    naugtur pushed a commit to MetaMask/snaps that referenced this pull request Mar 28, 2024
    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.
    MajorLift added a commit to MetaMask/snaps that referenced this pull request Jul 17, 2024
    #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.
    ```
    MajorLift added a commit to MetaMask/core that referenced this pull request Jul 22, 2024
    …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>
    Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @Mrtenz@mcmire