Fix ESM/CJS compatibility #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcmire
force-pushed
the
fix-esm-cjs-compatibility
branch
from
0179067
to
ee18632
Compare
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@babel/cli@7.23.9, npm/@babel/plugin-transform-modules-commonjs@7.23.3, npm/@babel/preset-env@7.23.9, npm/@babel/preset-typescript@7.23.3, npm/@babel/register@7.23.7, npm/babel-eslint@10.1.0 |
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again. Next stepsWhat is new author?A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package. Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights. What is shell access?This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code. Packages should avoid accessing the shell which can reduce portability, and make it easier for malicious shell access to be introduced. What is network access?This module accesses the network. Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use. What is an install script?Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts. Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead. Take a deeper look at the dependencyTake 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 packageIf 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 riskTo ignore an alert, reply with a comment starting with
|
mcmire
force-pushed
the
fix-esm-cjs-compatibility
branch
2 times, most recently
from
7783d21
to
f3412ac
Compare
Superstruct cannot be imported in a TypeScript project that uses a
`moduleResolution` of `node16` or `nodenext`. Any attempt to do so will result
in type errors such as the following:
```
node_modules/superstruct/dist/index.d.ts:1:15 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
1 export * from './error';
~~~~~~~~~
node_modules/superstruct/dist/index.d.ts:2:15 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
2 export * from './struct';
~~~~~~~~~~
node_modules/superstruct/dist/index.d.ts:3:15 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
3 export * from './structs/coercions';
~~~~~~~~~~~~~~~~~~~~~
...
```
This is happening because although Superstruct is defined as an ESM library —
the `module` field in `package.json` is set to `true` — its published TypeScript
declaration files aren't ESM-compatible. This is due to the fact that imports
are missing extensions, which is a requirement for ESM.
At the same time, although Rollup is supposedly configured to emit both ESM- and
CJS-compatible files, TypeScript does not know how to use the CJS variants. The
`exports` field is the way to instruct TypeScript on how to do this, but
`package.json` is missing this field. This makes it impossible to use
Superstruct in a non-ESM library.
This commit fixes both of these issues by making the following changes:
- All imports in TypeScript files now use an extension of `.js`. This may seem
like an odd choice, as there are no JavaScript files in the `src/` directory,
but TypeScript doesn't resolve a module by trying to find a file with the
given extension; it tries to find a declaration file that maps to the module
path, based on a set of rules. Therefore, the file extension is really just a
formality. In the end, Rollup is consolidating implementation files into one,
so all of the imports will go away.
- Type declaration files have been split into ESM and CJS variants. Since Rollup
generates ESM variants with an `.mjs` extension and CJS variants with a `.cjs`
extension, imports in declaration files also use either `.mjs` or `.cjs`
depending on the variant.
- `package.json` now has an `exports` field which instructs TypeScript how to
resolve modules from either an ESM or CJS perspective.
In addition, the tests have been updated to use Vitest instead of Babel so that
they can read directly from the TypeScript configuration instead of having to
use an explicit transform step.
In conjunction with this change, the minimum Node version has been bumped to
16.x and the GitHub workflows have been updated to stop testing on Node 14.x.
14.x has reached EOL, the version of Vitest we're using isn't compatible with
this version, and a `target` of `esnext` is being used in the TypeScript config,
which require a JavaScript runtime that supports newer ES features such as
`??=`, so the minimum Node version is effectively 16.x anyway.
This commit is derived from the following PR on the Superstruct repo:
<ianstormtaylor#1211>
For more information on how TypeScript resolves modules, the "Modules Reference"
section of the TypeScript handbook is quite helpful, and in particular these
sections:
- <https://www.typescriptlang.org/docs/handbook/modules/theory.html#the-role-of-declaration-files>
- <https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext>
mcmire
force-pushed
the
fix-esm-cjs-compatibility
branch
from
0ca03b1
to
4e10994
Compare
mcmire
commented
Feb 9, 2024
| "include": ["./**/*.ts"], | ||
| "compilerOptions": { | ||
| "lib": ["esnext"], | ||
| "skipLibCheck": true |
There was a problem hiding this comment.
It is unfortunate we have to do this. If we don't do this, then we get type errors from vitest. It seems that there are some requirements we may not be following here that vitest does, one of which is to use a moduleResolution of bundler (this can be observed in the vitest repo and in another project that uses vitest, such as this one). However, that would require using TypeScript 5. Upgrading that in this project causes all kinds of errors, and I didn't want to bother with that considering this is just a temporary solution.
|
I'm getting 84 errors on the final node_modules/superstruct/dist/index.d.ts:1:15 - error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.and a fair number of these: src/assert.ts:1:15 - error TS2305: Module '"superstruct"' has no exported member 'Struct'.
1 import type { Struct } from 'superstruct';
~~~~~~
src/assert.ts:1:29 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
1 import type { Struct } from 'superstruct';
~~~~~~~~~~~~~It looks like the branch checkout might not be registering for |
|
Still seeing the same errors. Here's a dump of the output I'm seeing. node_modules/superstruct/dist/index.d.ts(1,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/index.d.ts(2,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/index.d.ts(3,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/index.d.ts(4,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/index.d.ts(5,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/index.d.ts(6,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/utils.d.ts(1,58): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
node_modules/superstruct/dist/utils.d.ts(2,25): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
src/assert.ts(1,15): error TS2305: Module '"superstruct"' has no exported member 'Struct'.
src/assert.ts(1,29): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/assert.ts(2,10): error TS2305: Module '"superstruct"' has no exported member 'assert'.
src/assert.ts(2,45): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/base64.ts(1,15): error TS2305: Module '"superstruct"' has no exported member 'Struct'.
src/base64.ts(1,29): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/base64.ts(2,10): error TS2305: Module '"superstruct"' has no exported member 'pattern'.
src/base64.ts(2,25): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/caip-types.ts(1,15): error TS2305: Module '"superstruct"' has no exported member 'Infer'.
src/caip-types.ts(1,28): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/caip-types.ts(2,10): error TS2305: Module '"superstruct"' has no exported member 'is'.
src/caip-types.ts(2,14): error TS2305: Module '"superstruct"' has no exported member 'pattern'.
src/caip-types.ts(2,23): error TS2305: Module '"superstruct"' has no exported member 'string'.
src/caip-types.ts(2,37): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/checksum.ts(1,10): error TS2305: Module '"superstruct"' has no exported member 'size'.
src/checksum.ts(1,16): error TS2305: Module '"superstruct"' has no exported member 'string'.
src/checksum.ts(1,30): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/coercers.ts(1,15): error TS2305: Module '"superstruct"' has no exported member 'Infer'.
src/coercers.ts(1,28): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/coercers.ts(3,3): error TS2305: Module '"superstruct"' has no exported member 'bigint'.
src/coercers.ts(4,3): error TS2305: Module '"superstruct"' has no exported member 'coerce'.
src/coercers.ts(5,3): error TS2305: Module '"superstruct"' has no exported member 'create'.
src/coercers.ts(6,3): error TS2305: Module '"superstruct"' has no exported member 'instance'.
src/coercers.ts(7,3): error TS2305: Module '"superstruct"' has no exported member 'number'.
src/coercers.ts(8,3): error TS2305: Module '"superstruct"' has no exported member 'string'.
src/coercers.ts(9,3): error TS2305: Module '"superstruct"' has no exported member 'StructError'.
src/coercers.ts(10,3): error TS2305: Module '"superstruct"' has no exported member 'union'.
src/coercers.ts(11,8): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/coercers.ts(109,54): error TS2571: Object is of type 'unknown'.
src/coercers.ts(147,53): error TS2571: Object is of type 'unknown'.
src/coercers.ts(188,53): error TS2571: Object is of type 'unknown'.
src/hex.ts(2,15): error TS2305: Module '"superstruct"' has no exported member 'Struct'.
src/hex.ts(2,29): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/hex.ts(3,10): error TS2305: Module '"superstruct"' has no exported member 'is'.
src/hex.ts(3,14): error TS2305: Module '"superstruct"' has no exported member 'pattern'.
src/hex.ts(3,23): error TS2305: Module '"superstruct"' has no exported member 'string'.
src/hex.ts(3,37): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/hex.ts(99,23): error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.
src/json.ts(1,15): error TS2305: Module '"superstruct"' has no exported member 'Context'.
src/json.ts(1,24): error TS2305: Module '"superstruct"' has no exported member 'Infer'.
src/json.ts(1,37): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/json.ts(3,3): error TS2305: Module '"superstruct"' has no exported member 'any'.
src/json.ts(4,3): error TS2305: Module '"superstruct"' has no exported member 'array'.
src/json.ts(5,3): error TS2305: Module '"superstruct"' has no exported member 'boolean'.
src/json.ts(6,3): error TS2305: Module '"superstruct"' has no exported member 'coerce'.
src/json.ts(7,3): error TS2305: Module '"superstruct"' has no exported member 'create'.
src/json.ts(8,3): error TS2305: Module '"superstruct"' has no exported member 'define'.
src/json.ts(9,3): error TS2305: Module '"superstruct"' has no exported member 'integer'.
src/json.ts(10,3): error TS2305: Module '"superstruct"' has no exported member 'is'.
src/json.ts(11,3): error TS2305: Module '"superstruct"' has no exported member 'lazy'.
src/json.ts(12,3): error TS2305: Module '"superstruct"' has no exported member 'literal'.
src/json.ts(13,3): error TS2305: Module '"superstruct"' has no exported member 'nullable'.
src/json.ts(14,3): error TS2305: Module '"superstruct"' has no exported member 'number'.
src/json.ts(15,3): error TS2305: Module '"superstruct"' has no exported member 'object'.
src/json.ts(16,3): error TS2305: Module '"superstruct"' has no exported member 'optional'.
src/json.ts(17,3): error TS2305: Module '"superstruct"' has no exported member 'record'.
src/json.ts(18,3): error TS2305: Module '"superstruct"' has no exported member 'string'.
src/json.ts(19,3): error TS2305: Module '"superstruct"' has no exported member 'union'.
src/json.ts(20,3): error TS2305: Module '"superstruct"' has no exported member 'unknown'.
src/json.ts(21,3): error TS2305: Module '"superstruct"' has no exported member 'Struct'.
src/json.ts(22,8): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/json.ts(27,8): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct/dist/utils")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/json.ts(147,17): error TS7006: Parameter 'value' implicitly has an 'any' type.
src/json.ts(147,24): error TS7006: Parameter 'context' implicitly has an 'any' type.
src/json.ts(150,15): error TS7006: Parameter 'value' implicitly has an 'any' type.
src/json.ts(150,22): error TS7006: Parameter 'context' implicitly has an 'any' type.
src/json.ts(162,36): error TS7006: Parameter 'value' implicitly has an 'any' type.
src/json.ts(190,60): error TS7006: Parameter 'value' implicitly has an 'any' type.
src/versions.ts(8,15): error TS2305: Module '"superstruct"' has no exported member 'Struct'.
src/versions.ts(8,29): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/versions.ts(9,10): error TS2305: Module '"superstruct"' has no exported member 'is'.
src/versions.ts(9,14): error TS2305: Module '"superstruct"' has no exported member 'refine'.
src/versions.ts(9,22): error TS2305: Module '"superstruct"' has no exported member 'string'.
src/versions.ts(9,36): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("superstruct")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jongsun/Code/utils/package.json'.
src/versions.ts(64,4): error TS7006: Parameter 'value' implicitly has an 'any' type.
src/versions.ts(75,4): error TS7006: Parameter 'value' implicitly has an 'any' type. |
|
@MajorLift Hmm, strange. The branch checkout should register. I'm seeing references to |
|
I'm still seeing the errors. It looks like
|
|
@MajorLift I think the issue is that you're adding a |
|
Renaming the Removing the Moving the 'superstruct/dist/utils' imports to fall under 'superstruct' triggers 3 errors e.g. On the |
|
@MajorLift Ah, yes. Okay, perhaps saying that there shouldn't be any errors was inaccurate. There is another issue with Superstruct which I need to correct, although it's less to do with a bug and more to do with how we are using it. To be precise, it seems that So, in order to prevent any type errors with I wanted to submit this ESM/CJS PR upstream so it could be integrated into the official Superstruct repo. Since the subpath imports issue seems separate, I was planning on fixing that in another PR. Does that plan make sense to you? So I guess we could amend the instructions to say "You should see no errors, except for |
|
@mcmire Yes that sounds like a plan. Fixing the subpath imports issue in its own PR seems sensible. Getting this merged upstream would be exciting! |
|
@MajorLift Yeah for sure! If you're good with this would you mind giving it an approval? |
MajorLift
approved these changes
Feb 26, 2024
|
It doesn't seem like superstruct is well maintained. Maybe we should update it to match the module template and publish to NPM ourselves? |
|
@Mrtenz Yeah, that's the main plan here for sure. |
3 tasks
This was referenced Jul 7, 2024
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Superstruct cannot be imported in a TypeScript project that uses a
moduleResolutionofnode16ornodenext. Any attempt to do so will result in type errors such as the following:This is happening because although Superstruct is defined as an ESM library — the
modulefield inpackage.jsonis set totrue— its published TypeScript declaration files aren't ESM-compatible. This is due to the fact that imports are missing extensions, which is a requirement for ESM.At the same time, although Rollup is supposedly configured to emit both ESM- and CJS-compatible files, TypeScript does not know how to use the CJS variants. The
exportsfield is the way to instruct TypeScript on how to do this, butpackage.jsonis missing this field. This makes it impossible to use Superstruct in a non-ESM library.This commit fixes both of these issues by making the following changes:
.js. This may seem like an odd choice, as there are no JavaScript files in thesrc/directory, but TypeScript doesn't resolve a module by trying to find a file with the given extension; it tries to find a declaration file that maps to the module path, based on a set of rules. Therefore, the file extension is really just a formality. In the end, Rollup is consolidating implementation files into one, so all of the imports will go away..mjsextension and CJS variants with a.cjsextension, imports in declaration files also use either.mjsor.cjsdepending on the variant.package.jsonnow has anexportsfield which instructs TypeScript how to resolve modules from either an ESM or CJS perspective.In addition, the tests have been updated to use Vitest instead of Babel so that they can read directly from the TypeScript configuration instead of having to use an explicit transform step.
References
This commit is derived from the following PR on the Superstruct repo: ianstormtaylor#1211
For more information on how TypeScript resolves modules, the "Modules Reference" section of the TypeScript handbook is quite helpful, and in particular these sections:
Testing
To test this PR:
pnpm installpnpm buildpnpm testpnpm lintAlso see: MetaMask/utils#163. I've tested this against
utils, but you can do this too by following these steps:utils(maybe you did that in the previous steps)utils'stsconfig.jsonand changemoduleandmoduleResolutiontonodenextyarn link ../superstruct && yarnyarn build. You should see no errors.