-
-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump TypeScript module output options target
and lib
to ES2022
#4507
Labels
Comments
MajorLift
changed the title
Bump TypeScript module output
Bump TypeScript module output options Jul 9, 2024
target
and lib
optionstarget
and lib
to ES2022
57 tasks
MajorLift
added a commit
that referenced
this issue
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/[email protected]](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/[email protected]` 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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Motivation
module{,Resolution}
option toNode16
#3645 sets themoduleResolution
option tonode16
, which implies a target ofES2022
.Features
ES2022
Top-level-> not available to CJS modulesawait
expressionsmoduleResolution
set tonode16
, we can only reference ESM-only modules using dynamic import syntax, which becomes much easier to write with top-levelawait
.ES2021
String.prototype.replaceAll()
Promise.any()
Regressions
ErrorOptions
type defined in@metamask/approval-controller
needs to be renamed, as it shadows a new global variable defined in theES2022.Error
library.The text was updated successfully, but these errors were encountered: