Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
4 changes: 2 additions & 2 deletions .github/workflows/lint-build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ jobs:
echo "Building changed packages:"
jq -r '"- " + (.references[].path | ltrimstr("./") | rtrimstr("/tsconfig.build.json"))' "$TSCONFIG"
echo ""
yarn ts-bridge --project "$TSCONFIG" --verbose
yarn tsc --build --project "$TSCONFIG" --verbose
Comment thread
cursor[bot] marked this conversation as resolved.
else
echo "No packages to build."
fi
Expand Down Expand Up @@ -344,7 +344,7 @@ jobs:
needs: prepare
strategy:
matrix:
node-version: [20.x, 22.x, 24.x]
node-version: [22.x, 24.x]
steps:
- name: Checkout and setup environment
uses: MetaMask/action-checkout-and-setup@v3
Expand Down
File renamed without changes.
1 change: 0 additions & 1 deletion .yarnrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ npmPreapprovedPackages:
- "@metamask/*"
- "@metamask-previews/*"
- "@lavamoat/*"
- "@ts-bridge/*"

plugins:
- path: .yarn/plugins/@yarnpkg/plugin-allow-scripts.cjs
Expand Down
2 changes: 0 additions & 2 deletions docs/processes/building.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# Building packages

[`ts-bridge`](https://github.com/ts-bridge/ts-bridge) is used to build packages in both CommonJS- and ESM-compatible formats.

Built files show up in the `dist/` directory in each package. These are the files which will ultimately be published to NPM.

- Run `yarn build` to build all packages in the monorepo.
Expand Down
7 changes: 2 additions & 5 deletions eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
"count": 1
}
},
"packages/assets-controllers/jest.environment.js": {
"packages/assets-controllers/jest.environment.cjs": {
"n/prefer-global/text-decoder": {
"count": 1
},
Expand Down Expand Up @@ -599,7 +599,7 @@
"count": 25
},
"@typescript-eslint/prefer-nullish-coalescing": {
"count": 1
"count": 2
},
"no-negated-condition": {
"count": 1
Expand Down Expand Up @@ -2111,9 +2111,6 @@
"@typescript-eslint/explicit-function-return-type": {
"count": 10
},
"@typescript-eslint/naming-convention": {
"count": 1
},
"@typescript-eslint/no-explicit-any": {
"count": 4
},
Expand Down
10 changes: 7 additions & 3 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ const config = createConfig([
extends: [nodejs],
},
{
files: ['**/*.{js,cjs}'],
files: ['**/*.cjs'],
languageOptions: {
sourceType: 'script',
ecmaVersion: 2020,
Expand Down Expand Up @@ -181,6 +181,10 @@ const config = createConfig([
'jest/no-alias-methods': 'error',
'jest/no-commented-out-tests': 'error',
'jest/no-disabled-tests': 'error',

// `import { jest } from '@jest/globals'` is required in ESM test files
// and intentionally shadows the Jest-injected global.
'@typescript-eslint/no-shadow': ['error', { allow: ['jest'] }],
},
settings: {
node: {
Expand Down Expand Up @@ -221,14 +225,14 @@ const config = createConfig([
},
},
{
files: ['**/jest.environment.js'],
files: ['**/jest.environment.cjs'],
rules: {
// These files run under Node, and thus `require(...)` is expected.
'n/global-require': 'off',
},
},
{
files: ['**/*.mjs'],
files: ['**/*.js', '**/*.mjs'],
languageOptions: {
sourceType: 'module',
},
Expand Down
22 changes: 21 additions & 1 deletion jest.config.packages.js → jest.config.packages.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ module.exports = {
'<rootDir>/../../node_modules/@metamask/$1',
],
'^uuid$': require.resolve('uuid'),
// The sources import `lodash-es`, because `lodash` is CommonJS and its

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good work figuring this out 👏🏻

// named exports cannot be imported from an ES module. The tests compile to
// CommonJS, where the reverse is true, so they are pointed back at
// `lodash`. The two have the same API.
'^lodash-es$': require.resolve('lodash'),
},

// An array of regexp pattern strings, matched against all module paths before considered 'visible' to the module loader
Expand Down Expand Up @@ -190,7 +195,22 @@ module.exports = {
// testURL: "http://localhost",

// A map from regular expressions to paths to transformers
// transform: undefined,
// The packages are ESM, but Jest runs the tests as CommonJS: Jest only
// treats TypeScript as ESM when the extension is listed in
// `extensionsToTreatAsEsm`, so overriding the module settings here compiles
// the sources to CommonJS for tests without affecting the published build.
transform: {
'^.+\\.tsx?$': [
'ts-jest',
{
tsconfig: {
module: 'CommonJS',
moduleResolution: 'Node',
verbatimModuleSyntax: false,
},
},
],
},

// An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation
// transformIgnorePatterns: undefined
Expand Down
9 changes: 9 additions & 0 deletions jest.config.scripts.js → jest.config.scripts.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ module.exports = {

preset: 'ts-jest',

// Unlike the packages, the scripts use `import.meta`, which has no CommonJS

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting. I wonder if we can clean this up in the future so we don't have to make these overrides. Or maybe this is just how it has to work from now on until we switch to Vitest...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we should just start migrating to Vitest at some point. Cleaning this up otherwise would likely mean migrating all Jest tests to ESM.

// equivalent, so their tests run as ESM. Jest only treats TypeScript as ESM
// when the extension is listed here.
extensionsToTreatAsEsm: ['.ts', '.mts'],

transform: {
'^.+\\.m?tsx?$': ['ts-jest', { useESM: true }],
},

// "resetMocks" resets all mocks, including mocked modules, to jest.fn(),
// between each test case.
resetMocks: true,
Expand Down
12 changes: 6 additions & 6 deletions knip.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const config: KnipConfig = {
'scripts/**/*.{ts,js,sh}',
'tests/**/*.ts',
'*.config.{js,cjs,mjs,ts}',
'.prettierrc.js',
'.prettierrc.cjs',
],
project: ['scripts/**/*.ts', 'tests/**/*.ts', '*.{js,cjs,mjs,ts}'],
ignore: ['scripts/create-package/package-template/**'],
Expand Down Expand Up @@ -41,7 +41,7 @@ const config: KnipConfig = {
ignoreDependencies: [
'@metamask/controller-utils',
'@metamask/keyring-internal-api',
'lodash',
'lodash-es',
],
},
'packages/analytics-data-regulation-controller': {
Expand All @@ -66,7 +66,7 @@ const config: KnipConfig = {
ignoreDependencies: [
'@metamask/gas-fee-controller',
'@metamask/remote-feature-flag-controller',
'lodash',
'lodash-es',
'nock',
],
},
Expand Down Expand Up @@ -127,7 +127,7 @@ const config: KnipConfig = {
ignoreDependencies: [
'@metamask/base-controller',
'@metamask/superstruct',
'lodash',
'lodash-es',
],
},
'packages/multichain-network-controller': {
Expand Down Expand Up @@ -183,10 +183,10 @@ const config: KnipConfig = {
ignoreDependencies: ['cockatiel', 'nock'],
},
'packages/selected-network-controller': {
ignoreDependencies: ['immer', 'lodash', 'nock'],
ignoreDependencies: ['immer', 'lodash-es', 'nock'],
},
'packages/signature-controller': {
ignoreDependencies: ['lodash'],
ignoreDependencies: ['lodash-es'],
},
'packages/snap-account-service': {
ignoreDependencies: [
Expand Down
10 changes: 6 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@
"packages/*"
],
"files": [],
"type": "module",
"scripts": {
"build": "yarn ts-bridge --project tsconfig.build.json --verbose",
"build": "yarn tsc --build tsconfig.build.json --verbose",
"build:clean": "yarn build:only-clean && yarn build",
"build:docs": "yarn workspaces foreach --all --no-private --parallel --interlaced --verbose run build:docs",
"build:only-clean": "rimraf -g 'packages/*/dist'",
"build:only-clean": "rimraf -g 'packages/*/dist' 'packages/*/tsconfig.build.tsbuildinfo'",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, I had always meant to add this but never did 🤦🏻

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It would be useless anyway for ts-bridge 😅.

"build:types": "tsc --build tsconfig.build.json --verbose",
"changelog:update": "yarn workspaces foreach --all --no-private --parallel --interlaced --verbose run changelog:update",
"changelog:validate": "yarn workspaces foreach --all --no-private --parallel --interlaced --verbose run changelog:validate",
Expand Down Expand Up @@ -53,14 +54,15 @@
"test": "yarn test:scripts --silent --collectCoverage=false --reporters=jest-silent-reporter && yarn test:scripts:shell && yarn test:packages",
"test:clean": "yarn workspaces foreach --all --parallel --verbose run test:clean && yarn test",
"test:packages": "yarn test:verbose --silent --collectCoverage=false --reporters=jest-silent-reporter",
"test:scripts": "NODE_OPTIONS=--experimental-vm-modules yarn jest --config ./jest.config.scripts.js --silent",
"test:scripts": "NODE_OPTIONS=--experimental-vm-modules yarn jest --config ./jest.config.scripts.cjs --silent",
"test:scripts:shell": "node_modules/bats/bin/bats scripts/*.bats",
"test:verbose": "yarn workspaces foreach --all --parallel --verbose run test:verbose",
"workspaces:list-versions": "./scripts/list-workspace-versions.sh"
},
"devDependencies": {
"@actions/core": "^3.0.1",
"@actions/github": "^9.1.1",
"@jest/globals": "^30.4.1",
"@lavamoat/allow-scripts": "^3.0.4",
"@lavamoat/preinstall-always-fail": "^2.1.0",
"@metamask/create-release-branch": "^4.2.2",
Expand All @@ -74,7 +76,6 @@
"@metamask/network-controller": "^36.0.0",
"@metamask/skills": "^0.1.0",
"@metamask/utils": "^11.11.0",
"@ts-bridge/cli": "^0.6.4",
"@types/jest": "^30.0.0",
"@types/lodash": "^4.14.191",
"@types/node": "^22.13.14",
Expand Down Expand Up @@ -109,6 +110,7 @@
"rimraf": "^5.0.5",
"semver": "^7.6.3",
"simple-git-hooks": "^2.8.0",
"ts-jest": "^29.4.11",
"tsx": "^4.20.5",
"typescript": "~5.3.3",
"typescript-eslint": "^8.48.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
const merge = require('deepmerge');
const path = require('path');

const baseConfig = require('../../jest.config.packages');
const baseConfig = require('../../jest.config.packages.cjs');

const displayName = path.basename(__dirname);

Expand Down
20 changes: 6 additions & 14 deletions packages/account-tree-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,12 @@
"files": [
"dist/"
],
"type": "module",
"sideEffects": false,
"main": "./dist/index.cjs",
"types": "./dist/index.d.cts",
"exports": {
".": {
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
}
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
},
Comment on lines 24 to 27

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed import/require distinction to have Node.js resolve files regardless of whether it's running in ESM or CommonJS mode. This should work most of the time since require(esm) is possible now.

"./package.json": "./package.json"
},
Expand All @@ -39,8 +32,8 @@
"registry": "https://registry.npmjs.org/"
},
"scripts": {
"build": "ts-bridge --project tsconfig.build.json --verbose --clean --no-references",
"build:all": "ts-bridge --project tsconfig.build.json --verbose --clean",
"build": "tsc --project tsconfig.build.json",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Previously yarn build would automatically clean dist. With these changes it appears that is no longer the case. Is it worth adding a build:clean script to all packages in case that is needed? Or should we drop it for now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, we can add a build:clean script to all packages. If we make it clean by default we don't get the benefits of incremental builds.

"build:all": "tsc --build tsconfig.build.json --verbose",
"build:docs": "typedoc",
"changelog:update": "../../scripts/update-changelog.sh @metamask/account-tree-controller",
"changelog:validate": "../../scripts/validate-changelog.sh @metamask/account-tree-controller",
Expand Down Expand Up @@ -68,14 +61,13 @@
"@metamask/superstruct": "^3.4.1",
"@metamask/utils": "^11.11.0",
"fast-deep-equal": "^3.1.3",
"lodash": "^4.17.21"
"lodash-es": "^4.17.21"
},
"devDependencies": {
"@metamask/account-api": "^2.0.0",
"@metamask/auto-changelog": "^6.1.0",
"@metamask/eth-hd-keyring": "^15.0.0",
"@metamask/providers": "^22.1.0",
"@ts-bridge/cli": "^0.6.4",
"@types/jest": "^30.0.0",
"deepmerge": "^4.2.2",
"jest": "^30.4.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
const merge = require('deepmerge');
const path = require('path');

const baseConfig = require('../../jest.config.packages');
const baseConfig = require('../../jest.config.packages.cjs');

const displayName = path.basename(__dirname);

Expand Down
20 changes: 6 additions & 14 deletions packages/accounts-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,12 @@
"files": [
"dist/"
],
"type": "module",
"sideEffects": false,
"main": "./dist/index.cjs",
"types": "./dist/index.d.cts",
"exports": {
".": {
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
}
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
},
"./package.json": "./package.json"
},
Expand All @@ -39,8 +32,8 @@
"registry": "https://registry.npmjs.org/"
},
"scripts": {
"build": "ts-bridge --project tsconfig.build.json --verbose --clean --no-references",
"build:all": "ts-bridge --project tsconfig.build.json --verbose --clean",
"build": "tsc --project tsconfig.build.json",
"build:all": "tsc --build tsconfig.build.json --verbose",
"build:docs": "typedoc",
"changelog:update": "../../scripts/update-changelog.sh @metamask/accounts-controller",
"changelog:validate": "../../scripts/validate-changelog.sh @metamask/accounts-controller",
Expand Down Expand Up @@ -70,14 +63,13 @@
"deepmerge": "^4.2.2",
"ethereum-cryptography": "^2.1.2",
"immer": "^9.0.6",
"lodash": "^4.17.21",
"lodash-es": "^4.17.21",
"uuid": "^8.3.2"
},
"devDependencies": {
"@metamask/auto-changelog": "^6.1.0",
"@metamask/controller-utils": "^12.3.0",
"@metamask/providers": "^22.1.0",
"@ts-bridge/cli": "^0.6.4",
"@types/jest": "^30.0.0",
"@types/readable-stream": "^2.3.0",
"jest": "^30.4.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import type { NetworkClientId } from '@metamask/network-controller';
import { isCaipChainId } from '@metamask/utils';
import type { CaipChainId } from '@metamask/utils';
import type { WritableDraft } from 'immer/dist/internal.js';
import { cloneDeep } from 'lodash';
import { cloneDeep } from 'lodash-es';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

lodash does not work in ESM.


import { AccountsControllerMethodActions } from './AccountsController-method-action-types.js';
import { projectLogger as log } from './logger.js';
Expand Down
4 changes: 3 additions & 1 deletion packages/accounts-controller/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import type { Infer } from '@metamask/superstruct';
import { is, number, string, type } from '@metamask/superstruct';
import { hexToBytes } from '@metamask/utils';
import { sha256 } from 'ethereum-cryptography/sha256';
import type { V4Options } from 'uuid';
import { v4 as uuid } from 'uuid';

import type { AccountId } from './AccountsController.js';

// This is exported by the `uuid` package, but not importable in ESM.
type V4Options = Exclude<Parameters<typeof uuid>[0], undefined>;

/**
* Returns the name of the keyring type.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
const merge = require('deepmerge');
const path = require('path');

const baseConfig = require('../../jest.config.packages');
const baseConfig = require('../../jest.config.packages.cjs');

const displayName = path.basename(__dirname);

Expand Down
Loading
Loading