build: incremental CJS to ESM migration (Phase 1-3)#8749
Open
haraldschilly wants to merge 3 commits intomasterfrom
Open
build: incremental CJS to ESM migration (Phase 1-3)#8749haraldschilly wants to merge 3 commits intomasterfrom
haraldschilly wants to merge 3 commits intomasterfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 646346492c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
10ee853 to
216b369
Compare
Add dual CJS+ESM output for leaf packages (util, conat, backend, sync, sync-client, sync-fs). Each package now compiles twice: once to dist/ (CJS with declarations) and once to dist-esm/ (ESM). Package.json exports maps route "import" to dist-esm/ and "default" to dist/. Key changes: - Add tsconfig-esm.json for each leaf package (module: ESNext, moduleResolution: bundler, no declarations) - Add "types" condition (first position) in package.json exports so TypeScript always resolves a single canonical .d.ts from dist/ - Add tsconfig paths self-references for packages that import themselves - Convert all CJS .js files in util to TypeScript: message, regex-split, mathjax-utils, mathjax-utils-2, immutable-types, smc-version, heartbeat, mathjax-config, upgrades - Replace underscore with lodash in message.ts; remove underscore dep - Update update_version script to emit TypeScript - Add fullySpecified:false rspack rule for dist-esm/ (tsc doesn't emit .js extensions, but "type":"module" in dist-esm/ requires them) - Fix ghost export signal_sent in project/hub/handle-message.ts - Add regex-split.test.ts with 15 tests - Add add-esm-extensions.py utility script Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 3 of incremental CJS→ESM migration: - Add tsconfig-esm.json and conditional exports for @cocalc/comm and @cocalc/api-client packages - Standardize build scripts across all ESM-migrated packages to use `pnpm exec tsc` instead of `../node_modules/.bin/tsc` - Each package now produces both dist/ (CJS) and dist-esm/ (ESM) output Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or 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
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.
Summary
util,sync,sync-client,sync-fs,conat,backend,comm,api-clientdist/(CJS, unchanged) anddist-esm/(ESM) via a secondtsconfig-esm.json.jsfiles inutilto TypeScriptscripts/add-esm-extensions.pyutility for future phases requiring.jsextension insertionStrategy: Why Dual Output with Minimal Source Changes
The core constraint is that Node.js
require()cannot synchronously load native ESM — it throwsERR_REQUIRE_ESM. Sincehubandfrontendcurrently compile to CJS (and have CoffeeScript files blocking full migration), making any dependency pure ESM would break them immediately.The chosen approach uses
module: "ESNext"+moduleResolution: "bundler"in the ESM tsconfig. This combination:typefield (unlikeNodeNextwhich readspackage.jsonand may fall back to CJS).jsextensions on relative imports in source files — this means zero changes to.tssource files across the entire monorepoThe trade-off:
dist-esm/output lacks.jsextensions on relative imports, so it cannot be loaded directly by Node.js native ESM (node --input-type=module). This is acceptable because:hubis CoffeeScript-free and migrated to ESM, the.jsextension script (scripts/add-esm-extensions.py) can be used to enable native Node.js ESM support in a later phaseConditional Exports:
"import"/"default"Each package's
package.jsonuses:"types"(first) ensures TypeScript always resolves a single canonical.d.tsfromdist/, preventing type identity conflicts between CJS and ESM declarations"import"routes ESM consumers todist-esm/"default"routes CJS consumers (and TypeScriptmoduleResolution: "node") todist/Per-Package Changes
Each migrated package gets:
tsconfig-esm.json— ESM build config (module: ESNext,moduleResolution: bundler,declaration: false,composite: false)tsconfig.json— Added"dist-esm"toexcludepackage.json— Conditional exports, updatedbuild/cleanscripts,dist-esm/**infilestsc --build && tsc -p tsconfig-esm.json && echo '{"type":"module"}' > dist-esm/package.jsonAdditional Fixes
.jsto TypeScript inutil:message,regex-split,mathjax-utils,mathjax-utils-2,immutable-types,smc-version,heartbeat,mathjax-config,upgradesunderscorewithlodashinmessage.ts; removeunderscoredependencyupdate_versionscript to emit TypeScript (export const version = ...)fullySpecified: falserspack rule fordist-esm/files (tsc doesn't emit.jsextensions but"type":"module"requires them)pnpm exec tscinstead of../node_modules/.bin/tscsignal_sentinproject/hub/handle-message.ts— was never defined inmessage.js, replaced withmessage.success()regex-split.test.tswith 15 testsMigration Phases
@cocalc/util@cocalc/sync,@cocalc/sync-client,@cocalc/sync-fs,@cocalc/conat,@cocalc/backend@cocalc/comm,@cocalc/api-client@cocalc/database@cocalc/server,@cocalc/project,@cocalc/file-server@cocalc/next"import"condition automatically@cocalc/hub.coffeefiles)@cocalc/frontend.coffeefiles)Test plan
pnpm build-dev— full workspace build passespnpm test— CI passes (depcheck, version-check, tests)dist/anddist-esm/dist-esm/package.jsoncontains{"type":"module"}export/import(notexports.xxx =)🤖 Generated with Claude Code