Skip to content

Migrate build/commands to ESM#34222

Merged
goodov merged 7 commits intomasterfrom
build-commands-esm
Feb 28, 2026
Merged

Migrate build/commands to ESM#34222
goodov merged 7 commits intomasterfrom
build-commands-esm

Conversation

@goodov
Copy link
Member

@goodov goodov commented Feb 26, 2026

Migrate build/commands code to ESM to allow .js/.ts imports to work correctly in any direction. This allows for gradual migration to .ts while keeping .js files around.

Few notes on migration:

  1. import.meta.* cannot be used reliably with Jest. I tried different versions and approaches, eventually came up with a .cjs module that still uses __dirName, but can be imported into ESM and CommonJS modules without issues. I also considered rewriting Jest tests into built-in Node test runner, but since I was able to make Jest work, this was abandoned.
  2. tsc accepts both file.js/file.ts in imports and passes a compiler check without errors, but Node requires an import of a real file (type stripping is based on a file type). This means an incorrect import will pass test:scripts, but fail on real execution (see related discussion). An eslint rule was added to enforce that.
  3. local require() calls replaced with async/global imports depending on context.

Resolves brave/brave-browser#53172

@github-actions
Copy link
Contributor

github-actions bot commented Feb 26, 2026

📋 Code Owners Summary

58 file(s) changed, 57 with assigned owners

3 team(s) affected: @brave/branding-code-reviewer, @brave/build-commands-reviewers, @brave/js-deps-reviewers


Owners and Their Files

@brave/build-commands-reviewers — 54 file(s)

... and 49 more files

@brave/js-deps-reviewers — 2 file(s)

@brave/branding-code-reviewer — 1 file(s)

@goodov goodov force-pushed the build-commands-esm branch from 8a3987b to aa22c01 Compare February 26, 2026 17:28
@goodov goodov added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Feb 26, 2026
@goodov goodov force-pushed the build-commands-esm branch 2 times, most recently from 4a1bd8a to 62a7383 Compare February 26, 2026 19:06
@github-actions
Copy link
Contributor

Chromium major version is behind target branch (145.0.7632.120 vs 146.0.7680.32). Please rebase.

@github-actions github-actions bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Feb 26, 2026
@goodov goodov force-pushed the build-commands-esm branch from 62a7383 to 89f5fbf Compare February 26, 2026 19:11
@github-actions github-actions bot removed the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Feb 26, 2026
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@goodov goodov marked this pull request as ready for review February 26, 2026 20:15
@goodov goodov requested review from a team as code owners February 26, 2026 20:15
@github-actions
Copy link
Contributor

[puLL-Merge] - brave/brave-core@34222

Description

This PR migrates the build/commands directory from CommonJS (require/module.exports) to ES Modules (import/export). The package.json in build/commands is changed from "type": "commonjs" to "type": "module". A new rootDir.cjs file is introduced as a CommonJS escape hatch to use __dirname (unavailable in ESM) for resolving the root directory, since import.meta.dirname isn't supported by Jest. The PR also adds an ESLint import/no-unresolved rule for the build/commands directory, refactors prettier config resolution to use prettier.resolveConfig() instead of directly requiring .prettierrc, and consolidates the test:scripts command into a new TypeScript script that runs tsc, eslint, and jest sequentially.

Possible Issues

  1. Circular dependency risk: config.js now imports util.js at the top level (import util from './util.js'), and util.js imports config.js. Previously, config.js used a lazy require('./util') inside readArgsGn() to avoid the circular dependency. With ESM, circular imports are handled differently (partially initialized modules), and util may be undefined when config.js first evaluates if the cycle resolution order is unfavorable. This could cause subtle runtime failures.

  2. teamcity-service-messages always imported: In logging.js, tsm was previously conditionally required only when TEAMCITY_VERSION was set. Now it's unconditionally imported, meaning the module must always be installed. If it's not available in all environments (e.g., developer machines), this will cause an import failure at module load time rather than gracefully degrading.

  3. Copyright year typo: rootDir.cjs and testScripts.ts both have Copyright (c) 2026 — this should be 2025.

  4. perfTests export change: perfTests.js changed from module.exports = { runPerfTests } to export default { runPerfTests }, but in commands.js it's used as perfTests.runPerfTests(...). While this works, it's inconsistent with the rest of the codebase where named exports are preferred over default object exports.

  5. genTsConfig.js dynamic import: The change to await import('../../../components/webpack/path-map.js') assumes that file is also ESM-compatible. If path-map.js is still CommonJS and not under the build/commands package scope, this could behave differently than expected depending on the module system of that file.

Security Hotspots

None identified.

Changes

Changes

  • build/commands/package.json: Changed "type" from "commonjs" to "module", making all .js files in the directory ESM by default.
  • build/commands/lib/rootDir.cjs: New CommonJS module that uses __dirname to resolve the project root directory (ESM doesn't support __dirname, and import.meta.dirname isn't Jest-compatible).
  • build/commands/lib/config.js: Converted to ESM; moved rootDir resolution to rootDir.cjs; added top-level import util (previously lazy-required to avoid circular dependency).
  • build/commands/lib/logging.js: Converted to ESM; imports teamcity-service-messages unconditionally and uses a boolean flag isTeamcity; imports patchApplyReasonMessages and getReasonName directly from gitPatcher.js instead of lazy-requiring.
  • build/commands/lib/gitPatcher.js: Converted to ESM with named exports (export class GitPatcher, export const patchApplyReasons, etc.).
  • build/commands/lib/util.js: Converted to ESM; imports GitPatcher at top level instead of lazy-requiring.
  • build/commands/lib/genTsConfig.js: Uses dynamic await import() for path-map.js instead of require().
  • build/commands/scripts/format.js: Refactored to use prettier.resolveConfig(file) per-file instead of requiring .prettierrc once.
  • build/commands/scripts/generateCoverageReport.js: Changed from destructured fs-extra imports to a default import, using fs.mkdirp() and fs.writeJSON().
  • build/commands/scripts/testScripts.ts: New TypeScript script that runs tsc, eslint, and jest sequentially for the build/commands directory.
  • eslint.config.mjs: Added eslint-plugin-import with import/no-unresolved rule for build/commands/**.
  • All other files in build/commands/: Mechanical conversion from require()/module.exports to import/export with .js extensions on relative imports.
sequenceDiagram
    participant User
    participant npm as npm run test:scripts
    participant ts as testScripts.ts
    participant tsc as tsc --project build/commands
    participant eslint as eslint --quiet build/commands
    participant jest as jest build/commands

    User->>npm: npm run test:scripts
    npm->>ts: node ./build/commands/scripts/testScripts.ts
    ts->>tsc: execSync("tsc --project build/commands")
    tsc-->>ts: success/fail
    ts->>eslint: execSync("eslint --quiet build/commands")
    eslint-->>ts: success/fail
    ts->>jest: execSync("jest build/commands")
    jest-->>ts: success/fail
    ts-->>User: exit code
Loading

@github-actions github-actions bot added puLL-Merge CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) and removed puLL-Merge labels Feb 27, 2026
@goodov goodov force-pushed the build-commands-esm branch from d355a3f to e42e197 Compare February 27, 2026 12:25
@goodov goodov force-pushed the build-commands-esm branch from c1dd8a9 to 0ff3562 Compare February 27, 2026 12:36
@goodov goodov removed the CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) label Feb 27, 2026
Copy link
Member

@diracdeltas diracdeltas left a comment

Choose a reason for hiding this comment

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

unblocking js-deps

@goodov goodov merged commit 1ca2ad1 into master Feb 28, 2026
23 checks passed
@goodov goodov deleted the build-commands-esm branch February 28, 2026 13:56
@github-actions github-actions bot added this to the 1.89.x - Nightly milestone Feb 28, 2026
@brave-builds
Copy link
Collaborator

Released in v1.89.89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/storybook-url Deploy storybook and provide a unique URL for each build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate build/commands to ESM

6 participants