Skip to content
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

Set tsconfig module{,Resolution} options to NodeNext #2514

Closed
wants to merge 52 commits into from

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Jun 25, 2024

As part of the Wallet Framework Team's OKR (Q2 2024 O3KR4) for upgrading TypeScript to v5.0+ in the core monorepo, we are updating dependencies of the core repo so that they generate builds and type declarations for both CJS and ESM.

The purpose of this update is to ensure that these dependencies can be imported into projects using the Node16 or NodeNext options for their moduleResolution TypeScript compiler setting.

Because the effects of these changes apply to nested dependencies as well, we are enabling the NodeNext moduleResolution option in all affected repos, to ensure that all projects further downstream can be set to NodeNext without issue.

Looking upstream, we also need to make changes for dependencies that are ESM-only. Because snaps is a CJS project, we either need to update these dependencies to expose a CJS build, or replace our static import statements for these dependencies with dynamic import syntax.

See https://www.typescriptlang.org/docs/handbook/modules/reference.html#interoperability-rules

References

@MajorLift MajorLift self-assigned this Jun 25, 2024
@MajorLift MajorLift linked an issue Jun 25, 2024 that may be closed by this pull request
@MajorLift MajorLift force-pushed the set-moduleResolution-to-NodeNext branch 2 times, most recently from 98f35d6 to 86eaa95 Compare June 25, 2024 00:48
Copy link

socket-security bot commented Jun 25, 2024

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

Package New capabilities Transitives Size Publisher
npm/@metamask/abi-utils@2.0.3 Transitive: network +9 1.81 MB metamaskbot

🚮 Removed packages: npm/@metamask/abi-utils@2.0.2

View full report↗︎

Copy link

socket-security bot commented Jun 25, 2024

👍 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.

View full report↗︎

@MajorLift MajorLift force-pushed the set-moduleResolution-to-NodeNext branch from 9c137e7 to a65d016 Compare June 25, 2024 14:34
@MajorLift MajorLift force-pushed the replace-superstruct-with-fork branch from ac248e2 to 5a56e9b Compare June 25, 2024 14:56
@MajorLift MajorLift force-pushed the set-moduleResolution-to-NodeNext branch 3 times, most recently from e634370 to 798e1b4 Compare June 25, 2024 16:36
@MajorLift MajorLift force-pushed the replace-superstruct-with-fork branch from de01f9d to 5a56e9b Compare June 25, 2024 16:42
@MajorLift MajorLift force-pushed the set-moduleResolution-to-NodeNext branch from de74e82 to c395916 Compare July 5, 2024 22:58
@MajorLift MajorLift force-pushed the set-moduleResolution-to-NodeNext branch from 1dc7108 to 3c4210e Compare July 6, 2024 03:28
@MajorLift MajorLift force-pushed the replace-superstruct-with-fork branch 3 times, most recently from 69abbc0 to 68cae24 Compare July 15, 2024 20:33
MajorLift added a commit 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.
```
Base automatically changed from replace-superstruct-with-fork to main July 17, 2024 18:00
@MajorLift
Copy link
Contributor Author

Closed by #2682

@MajorLift MajorLift closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace superstruct with ESM-compatible fork @metamask/superstruct
1 participant