Skip to content

fix: emit CommonJS declarations as .d.cts and point require.types at them - #433

Merged
ianwremmel merged 2 commits into
mainfrom
clc-1130-emit-commonjs-declarations-as-dcts-and-point-the-require
Sep 11, 2026
Merged

ianwremmel merged 2 commits into
mainfrom
clc-1130-emit-commonjs-declarations-as-dcts-and-point-the-require

Conversation

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator

All 32 packages under packages/@code-like-a-carpenter/* fail attw with "Masquerading as ESM" and fail publint with the matching error. Every package is "type": "module", so any .d.ts beneath it is read as ESM, and both shapes of exports["."].require.types point at a .d.ts.

Two causes:

  • 22 .ts-source packages have no CJS declarations at all — build:types runs scripts/dmts-to-dts only for src/index.mjs/src/index.mts entry points, so their require.types points at the same ./dist/types/index.d.ts as import.types.
  • 10 .mjs-source packages do produce dist/cjs-types, but .d.ts under type: module is still ESM, and dmts-to-dts strips extensions (sed -e "s#.mjs#${EXTENSION}#g" with EXTENSION unset) instead of rewriting them, which also mangles sourceMappingURL so every declaration map dangles.

This changes declaration emit only; runtime require()/import() already work for all 32 packages.

Ticket: https://linear.app/code-like-a-carpenter/issue/CLC-1130/emit-commonjs-declarations-as-dcts-and-point-the-require-condition-at

@linear-code

linear-code Bot commented Sep 7, 2026

Copy link
Copy Markdown

CLC-1130

@ianwremmel-ai-agent

ianwremmel-ai-agent commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Plan

  • Replace scripts/dmts-to-dts with a converter that emits dist/cjs-types/**/*.d.cts: rename .d.ts/.d.mts to .d.cts, rewrite relative specifiers (./x.mjs, ./x.mts, ./x.ts) to ./x.cjs instead of stripping the extension, and rewrite sourceMappingURL plus the map files' file field so declaration maps resolve.
  • Run the conversion for .ts-source packages too, not only src/index.mjs/src/index.mts entry points (packages/@clc/nx/src/create-nodes.mjs).
  • Point exports["."].require.types at ./dist/cjs-types/index.d.cts for every package in packages/@clc/nx/executors/package-json/executor.mjs; leave import.types and the top-level types field alone.
  • Regenerate the package.json files. 34 changed, not 32: the two private @clc packages (dependencies, test-helpers) come from the same executor. ./scripts/diff-lint is clean after a full build, so CI reproduces them exactly.
  • Verify: attw clean for all 32 packages, publint --strict free of require.types errors, and a "type": "commonjs" / moduleResolution: node16 fixture type-checking against packed tarballs of assert (.mjs source) and exception (.ts source). Packing used a synthetic version in a staged copy.

Verification notes

  • Both negative controls fail as they should: pointing require.types back at the .d.mts makes attw report FalseESM and makes the fixture fail to compile. The checks have teeth.
  • attw's own CLI cannot read any tarball in this environment — its extractTarball keeps only the last chunk fflate's streaming Gunzip emits, so it dies with Cannot read properties of undefined (reading 'filename') on lodash too. Checks ran through @arethetypeswrong/core's Package API against the extracted tarball instead.
  • publint --strict still reports pkg.bin is ./cli.mjs but the file does not exist for cli-core and cli-plugin-example. That is CLC-1131, not this PR.
  • All 230 declaration maps across the 34 packages have a correct file field and sources that resolve; no .d.cts retains a .mjs/.mts/.ts specifier.
  • npm test fails locally on a missing ts-node, which is absent from a pristine main checkout too. Unrelated to this change; CI runs the real suite.

Adversarial review (two Codex passes, one spec-aware and one spec-blind)

Acted on:

  • Extensionless (./x) and directory (./dir) specifiers resolve to nothing once the target is renamed to .d.cts. Reproduced with a real node16 CommonJS consumer: TS2307. No declaration in this repo emits one, so the converter now fails loudly instead of writing output that cannot resolve.
  • /// <reference path="x.d.ts" /> without a leading ./ was missed by the specifier regex. Reference paths now get their own pass.
  • An empty dist/types deleted dist/cjs-types and exited 0, leaving require.types dangling and letting nx cache the result. Now an error.
  • build:types inputs were wrong in two ways that decide whether these declarations get regenerated at all: the glob src/**/*.[jt]s?(x) matches no .mjs or .mts file, and the converter itself was not an input. Both fixed.

Dismissed:

  • String literal types that look like relative paths get rewritten. Real, and documented in the script. Every quoted relative string in the built declarations of all 34 packages ends in a source extension and is a module specifier; the alternative is parsing TypeScript here.
  • Map mappings drift by one column on lines where .ts became .cjs. Cosmetic, affects only editor navigation, and re-emitting maps costs far more than it returns.
  • Filenames containing a quote character (./a'b.mjs) are not rewritten. No such file exists or is likely to.
  • Stale declarations in dist/types survive a source deletion because tsc --build is incremental and never cleans. Pre-existing, unchanged by this PR, and out of scope.

CI annotation actions/checkout@v4 targets Node.js 20: dismissed. It fires on every workflow run in this repo, main included, and this PR touches no workflow file. Bumping actions/checkout is its own change.

Verification tooling itself lands in #427 (scripts/verify-pack.mjs), so this PR checks locally rather than adding a competing CI job.


Status 2026-09-08

Head 937e5c0 is unchanged and CI is unchanged: green everywhere except the five test-integration shards, which still die at ./scripts/crr-sam: line 35: testfile: unbound variable before a test runs. Same failure on main (run 34163408910).

Remaining CI annotations dismissed, none of them from this branch:

  • Disallow the any type in foundation-runtime/src/actions/unmarshall.ts, logger/src/console-logger.ts, types/dredd-transactions.d.ts, types/gavel.d.ts — none of these files is in this diff, which touches only packages/@clc/nx/executors/package-json/executor.mjs, packages/@clc/nx/src/create-nodes.mjs, scripts/dmts-to-dts, scripts/dts-to-dcts.mjs and regenerated manifests. lint and ESLint both pass.
  • Node.js 20 deprecation warnings for actions/setup-node, nrwl/nx-set-shas, check-run-reporter/action, codecov/codecov-action, aws-actions/configure-aws-credentials, alongside the actions/checkout one already dismissed above. Repo-wide, no workflow file in this diff.
  • Could not find any report files matching glob reports/junit/**/*.xml, Request ID: undefined, An unexpected error occurred, Process completed with exit code 1 — all downstream of the crr-sam failure, which kills each shard before any junit report is written.

Still waiting on the test-integration decision below. Nothing else gates review.

Rebased onto main at 3bff384, now head 398e2a2. The ten commits picked up are all dependabot bumps; the rebase was conflict-free and the diff hunks are byte-identical to 937e5c0 (only base blob hashes moved). CI is re-running, and the diff check now verifies the regenerated manifests against current main.

The test-integration question is settled and no longer gates this PR: crr-sam is CLC-1138 / #437, and branch protection on main requires one approving review and no status checks. Copilot review next, then out of draft.

CI on 398e2a2 after the rebase: build, fixup, nopush, diff, lint, test-unit, ESLint, TSC, Unit Tests, codecov/patch, codecov/project and both Socket checks green. The five test-integration shards still die at ./scripts/crr-sam: line 35: testfile: unbound variable, exactly as run 34182043796 does on main at 3bff384 — that is CLC-1138 / #437, not this diff. The only remaining CI annotation is the repo-wide Node.js 20 action deprecation; dismissed, no workflow file is in this diff.

Copilot review skipped: it is not installed as a reviewer on this repo. suggestedActors returns only ianwremmel and ianwremmel-ai-agent, and a requested_reviewers POST for the Copilot bot returns 200 while adding nobody. Going straight to operator review.

Copilot did review after all, on 398e2a2: "Approval recommended", 38/38 files
reviewed, 0 comments generated. The earlier note that Copilot is not installed
was wrong — the request landed late. Nothing to address.

Gates 1-5 hold apart from the test-integration shards, which are red on main
at this branch's base for the same crr-sam reason (CLC-1138 / #437) and do not
gate merge under branch protection. Waiting on @ianwremmel's approval.


Status 2026-09-09 — head a66aa2e

Rebased onto main at d2a3396, conflict-free. That base now carries #436
(ci: shard example tests in-repo instead of via check-run-reporter), which is
the crr-sam: line 35: testfile: unbound variable fix. The five
test-integration shards should go green on this run; they were the only red
checks and the only thing left blocking gate 1.

The rebase crossed a substantially rewritten create-nodes.mjs (#436 added the
workspace-refs executor, #435 reworked bin). The effective diff is unchanged:
git diff origin/main...HEAD over the manifests still shows exactly one line
moving, require.types to ./dist/cjs-types/index.d.cts, and the three source
edits are byte-identical apart from the comment fix below.

Answered @ianwremmel's thread on the script header: type: module is not the
cause — TypeScript reads a declaration's format from an explicit extension or
the nearest "type", never from exports, so one half needs a disambiguating
extension whichever "type" you pick. Separately, dist/cjs has exactly one
in-repo consumer (tool-foundation/executors.json loads
./dist/cjs/__generated__/foundation-executor.cjs), while the
exports["."].require condition has none — its audience is external consumers
of the published packages. Whether to keep serving them is a real call and I
offered to file it as its own ticket.

Two more Codex passes on the rebased diff, one spec-aware and one spec-blind.

Acted on:

  • Both "longest first" comments claimed an ordering safeguard that endsWith
    never needs: 'x.mts'.endsWith('.ts') and 'x.d.mts.map'.endsWith('.d.mts')
    are both false, so the arrays work in any order. Deleted rather than corrected.
  • The specifier rewrite applies to comments and template literal types too, not
    only string literal types as the header claimed. Header now says so, and says
    that an extensionless one throws.

Dismissed:

  • Widening the build:types input glob to ?(m|c) is scope creep for the c
    half. The m half is required — src/**/*.[jt]s?(x) matches no .mjs or
    .mts file, so these declarations were not being invalidated at all. The c
    half is one character and covers a src/*.cts if one ever lands. The glob
    belongs to build:types alone; it does not leak to other targets.
  • Specifiers containing a quote ('./it\'s.js') or a unicode-escaped dot are
    not rewritten. Same class as the filename-with-a-quote case already dismissed
    above; no such file exists.
  • A throw partway through the write loop leaves dist/cjs-types partially
    written, since the directory is cleared before the loop. Nx does not cache a
    failed task and the next run clears the directory again, so the partial state
    is never consumable.
  • 34 manifests against the ticket's 32: the two extras are the private @clc
    packages, which share the converter and would keep broken .d.ts require
    targets otherwise. Already noted above.

Smoke-tested the converter standalone after the comment edits: reference paths,
export * from, export type {} from, import type, sourceMappingURL and
the map file field all convert as expected.

Gates 2-5 hold. Waiting on gate 1 (this CI run) and @ianwremmel's approval.


Status 2026-09-09

Rebased onto main at d2a3396 (which carries #436), head a66aa2e. Every check is green, including all five test-integration shards and the new Examples check. Zero commits behind main, no conflicts.

CI annotations on this head are all notice-level deploy/teardown progress lines from the passing shards, plus the recurring Node.js 20 deprecation warning for third-party actions. Dismissed: no workflow file is in this diff, and the shards succeeded.

Gates 1-5 hold. Waiting on @ianwremmel's approval.

@nx-cloud

nx-cloud Bot commented Sep 8, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit a66aa2e

Command Status Duration Result
nx run-many --target build ✅ Succeeded 35s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-09-09 17:30:26 UTC

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.64%. Comparing base (d2a3396) to head (a66aa2e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #433   +/-   ##
=======================================
  Coverage   60.64%   60.64%           
=======================================
  Files          28       28           
  Lines        1601     1601           
  Branches      354      363    +9     
=======================================
  Hits          971      971           
  Misses        626      626           
  Partials        4        4           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

CI on 937e5c0: everything green except all five test-integration shards, which fail before running a test:

./scripts/crr-sam: line 35: testfile: unbound variable

This is not from this PR. Run 34163408910 on main fails on the same line with the same message, and so do the other open branches.

Root cause: steps.split.outputs.tests comes back empty from check-run-reporter/action, so crr-sam is invoked with no arguments. The loop at scripts/crr-sam:26 never runs and testfile is never set, but stack:list --as-projects with no --test filter returns every project, so the loop at line 34 does run and dereferences $testfile under set -u. Line 35 is also wrong on its own terms: $testfile is the loop variable from lines 26-28, so even on the happy path it holds the last test name rather than the one belonging to $projectName.

Two separate problems: the splitter returning nothing, and crr-sam reporting that as an unbound variable instead of the "No stacks deployed" error at line 42 that was written for it. Neither is something this PR can fix without turning a declaration-emit change into a CI change.

Green here: build, diff (a full build reproduces the 34 regenerated manifests exactly), lint, TSC, ESLint, test-unit, Unit Tests, codecov/patch, codecov/project.

@ianwremmel — how do you want to handle test-integration? Either it gets fixed in its own PR before this one merges, or you waive it here given it is red on main too. I have not requested review yet, since it gates on CI.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Resolving my own open question: no answer needed. crr-sam is now CLC-1138 with its own PR (#437), so test-integration gets fixed there rather than here. Branch protection on main requires one approving review and no status checks, so a red test-integration does not block this merge.

Proceeding to rebase onto main, take this out of draft, and request review.

@ianwremmel-ai-agent
ianwremmel-ai-agent force-pushed the clc-1130-emit-commonjs-declarations-as-dcts-and-point-the-require branch from 937e5c0 to 398e2a2 Compare September 8, 2026 16:16
@ianwremmel
ianwremmel requested a lite review from Copilot September 8, 2026 16:31
@ianwremmel-ai-agent
ianwremmel-ai-agent marked this pull request as ready for review September 8, 2026 16:31
@ianwremmel
ianwremmel self-requested a review September 8, 2026 16:31
@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Out of draft and ready for your review, @ianwremmel.

What changed: require.types for all 34 generated manifests now points at ./dist/cjs-types/index.d.cts, and scripts/dts-to-dcts.mjs (replacing scripts/dmts-to-dts) produces those declarations for every package rather than only the ten with an .mjs/.mts entry point. Relative specifiers are rewritten to .cjs instead of being stripped, and sourceMappingURL plus each map's file field are rewritten so the 230 declaration maps resolve.

Verified against packed tarballs with a synthetic version injected: attw reports no problems for all 32 published packages, publint --strict reports no require.types error, and a "type": "commonjs" / moduleResolution: node16 fixture type-checks against tarballs of @code-like-a-carpenter/assert (.mjs source) and @code-like-a-carpenter/exception (.ts source). Both negative controls fail as they should.

Two things to know before you approve:

  • The five test-integration shards are red, and so are they on main at 3bff384 (run 34182043796), with the same ./scripts/crr-sam: line 35: testfile: unbound variable. That is CLC-1138 / fix(ci): shard example tests in-repo instead of via check-run-reporter #437. Everything else is green, including diff, which confirms a full build reproduces the 34 regenerated manifests exactly.
  • Copilot is not installed as a reviewer on this repo, so this skipped the bot pass and came straight to you.

Reply lgtm or approve to merge.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes consistently update the build pipeline and package export metadata to emit and reference .d.cts CommonJS declarations without introducing unresolved references.

Pull request overview

This PR fixes “Masquerading as ESM” type-declaration issues across the monorepo by emitting a dedicated CommonJS declaration tree (.d.cts) and updating exports["."].require.types to point at it, while keeping runtime require()/import() behavior unchanged.

Changes:

  • Add scripts/dts-to-dcts.mjs to convert dist/typesdist/cjs-types with .d.cts filenames and rewritten relative specifiers/source maps.
  • Replace the old scripts/dmts-to-dts flow and update the Nx build:types command to always generate dist/cjs-types.
  • Update package exports.require.types entries to reference ./dist/cjs-types/index.d.cts.
File summaries
File Description
scripts/dts-to-dcts.mjs New converter script producing .d.cts CJS declarations and fixing relative specifiers + source maps.
scripts/dmts-to-dts Removed legacy bash-based conversion that stripped extensions / broke sourcemap URLs.
packages/@code-like-a-carpenter/workbench-config/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/wait-for/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tooling-common/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tool-tool/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tool-stack/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tool-json-schema/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tool-inliner/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tool-foundation/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/tool-deps/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/telemetry/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/sentry/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/parallel/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/logger/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/lambda-handlers/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/interact/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/graphql-codegen-helpers/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/foundation-transform-environment/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/foundation-runtime/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/foundation-plugin-typescript/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/foundation-plugin-cloudformation/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/foundation-parser/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/foundation-intermediate-representation/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/exception/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/errors/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/env/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/dotenv/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/contract-tests/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/cli/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/cli-plugin-example/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/cli-core/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/aws-env-loader/package.json Point exports.require.types to index.d.cts.
packages/@code-like-a-carpenter/assert/package.json Point exports.require.types to index.d.cts.
packages/@clc/test-helpers/package.json Point exports.require.types to index.d.cts.
packages/@clc/dependencies/package.json Point exports.require.types to index.d.cts.
packages/@clc/nx/src/create-nodes.mjs Always run dts-to-dcts after tsc for build:types and widen source input globs to include m/c variants.
packages/@clc/nx/executors/package-json/executor.mjs Generate exports.require.types as index.d.cts and document why.
Review details
  • Files reviewed: 38/38 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Correction to my engagement comment: Copilot did review after all, and it
recommends approval — 38/38 files reviewed, 0 comments generated. Nothing to
address, so the state is unchanged: everything green except the five
pre-existing test-integration shards (CLC-1138 / #437), waiting on your
approval.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

Merge-order correction. My earlier comment pointed at #437 for the red
test-integration shards; #437 is closed. #436 now carries both that fix
(CLC-1138) and the root tsconfig.json cache poisoning fix (CLC-1137), so #436
is the one that has to land first.

Nothing here changed. This branch is still at 398e2a2, no conflicts with
main, and every check except the five test-integration shards is green. Those
five die at ./scripts/crr-sam: line 35: testfile: unbound variable — the
check-run-reporter split step returns an empty test list, so crr-sam runs with
no --test arguments. That is repo-wide, reproduces on main, and no change in
this diff can move it; the diff touches no workflow file and no script under
scripts/ other than replacing dmts-to-dts with dts-to-dcts.mjs.

Copilot reviewed at this head: approval recommended, 38/38 files, 0 comments.

So this needs two things from you: approve here, and merge #436 before this so
CI can go green. Reply lgtm or approve when you're ready.

@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

@ianwremmel this PR is ready and needs your input to proceed:

  1. Approval — review-decision is REVIEW_REQUIRED; the agent account can't self-approve.
  2. fix: repair CI on main: root tsconfig.json cache poisoning and the dead test-split step #436 merged firsttest-integration is red here (all 5 shards), but that's a repo-wide break right now (including on main), not caused by this change. fix: repair CI on main: root tsconfig.json cache poisoning and the dead test-split step #436 fixes the root tsconfig.json cache-poisoning issue that's breaking CI everywhere. Once fix: repair CI on main: root tsconfig.json cache poisoning and the dead test-split step #436 merges, this PR's CI should go green on its own re-run.

Every other check (build, lint, diff, TSC, test-unit, codecov, Socket Security) is green. No other blockers on this PR.

Comment thread scripts/dts-to-dcts.mjs
/**
* Produces `dist/cjs-types` from `dist/types`.
*
* Every package is `"type": "module"`, so a `.d.ts` under it is read as ESM no

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the problem here more that everything is type:module? by your logic here, it sounds like our commonjs exports serve no purpose

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Two questions in there, and I think only the second one is really open.

type: module is not the cause. TypeScript decides a declaration's module
format the same way Node decides a .js file's: the file extension if it is
explicit, otherwise the nearest package.json "type". exports picks which
file gets loaded; it has no say in how that file is parsed. So under either
"type" one of the two halves has to carry a disambiguating extension:

  • "type": "module" — the ESM half is fine as .d.ts, the CommonJS half needs
    .d.cts.
  • "type": "commonjs" — mirror image: the CommonJS half is fine as .d.ts, the
    ESM half needs .d.mts.

Flipping type moves the problem rather than removing it, and it would be the
wrong way round, since the sources are ESM. The runtime half of exports
already gets this right — index.mjs and index.cjs, both explicit. The
declarations were the one place the extension got dropped: dmts-to-dts
renamed .d.mts to .d.ts when it should have renamed it to .d.cts.

"Our CommonJS exports serve no purpose" — partly right, and worth deciding
deliberately rather than by accident.

Two things are easy to conflate here:

dist/cjs itself is load-bearing.
packages/@code-like-a-carpenter/tool-foundation/executors.json points Nx at
./dist/cjs/__generated__/foundation-executor.cjs, so that executor runs only
because the CommonJS build exists. But Nx loads it by path — it never goes
through exports.

The exports["."].require condition is the part with no in-repo consumer. I
grepped for one and came up empty; every require() in the tree is relative.
Its only audience is external CommonJS consumers of the published packages, and
these are published (@code-like-a-carpenter/assert is on npm at 2.4.0,
logger at 2.5.0).

So the real question is whether you still want to serve those consumers. If the
answer is no, the honest change is bigger than this PR: drop the require
condition, drop build:cjs for everything except tool-foundation, and delete
this script along with it. engines.node is 22.x and require(esm) has been
unflagged since 22.12, so that is a defensible call — but it is a
publishing-compatibility decision, not a build bug, and I don't want to make it
silently inside a PR scoped to "stop the declarations lying about their format".

Say the word and I'll file it as its own ticket. Either way this PR is the right
intermediate step: it makes the require half tell the truth, and if you later
delete the condition you delete this script with it, so nothing here becomes
sunk cost.

Done.

Every package is `"type": "module"`, so a `.d.ts` beneath one is read as
ESM whatever `exports` says. `exports["."].require.types` pointed CommonJS
consumers at those ESM declarations: attw reported "Masquerading as ESM"
for all 32 published packages, and a `moduleResolution: node16` CommonJS
consumer got TS1479 against a packed tarball.

scripts/dts-to-dcts.mjs replaces scripts/dmts-to-dts. It writes
dist/cjs-types/**/*.d.cts and rewrites relative specifiers to `.cjs`
(`./x.mjs` and `./x.ts` alike), where the old script stripped the
extension outright — `sed -e "s#.mjs#${EXTENSION}#g"` with EXTENSION
unset turned `export * from "./assert.mjs"` into `"./assert"`, which is
what attw reported as an internal resolution error. The same substitution
mangled `//# sourceMappingURL=index.d.mts.map` into `index.d.map`; the
new script rewrites sourceMappingURL and each map's `file` field, so all
230 declaration maps now resolve.

build:types runs the conversion for every package, not only those whose
entry point is `src/index.mjs` or `src/index.mts`, so the 22 `.ts`-source
packages get CommonJS declarations for the first time.

The converter fails rather than emitting output that cannot resolve: an
extensionless or directory specifier has no working rewrite under node16
CommonJS resolution, a name collision between `x.d.ts` and `x.d.mts` is
reported, and an empty `dist/types` is an error because the require
condition names a file that would not exist.

Two build:types inputs were wrong and are corrected here because they
decide whether these declarations get regenerated: the source glob
`src/**/*.[jt]s?(x)` matched no `.mjs` or `.mts` file, and the converter
itself was not an input.

34 manifests change, the 32 published packages plus the two private
`@clc` ones the same executor generates. Only `require.types` moves; the
`import` condition and the top-level `types` field are untouched.

Verified against packed tarballs with a synthetic version: attw reports
no problems for all 32 packages, publint --strict reports no
`require.types` error (cli-core and cli-plugin-example still fail on
`pkg.bin`, which is CLC-1131), and a `"type": "commonjs"` /
`moduleResolution: node16` fixture type-checks against tarballs of
@code-like-a-carpenter/assert (.mjs source) and
@code-like-a-carpenter/exception (.ts source).

Refs CLC-1130
@ianwremmel-ai-agent
ianwremmel-ai-agent force-pushed the clc-1130-emit-commonjs-declarations-as-dcts-and-point-the-require branch from 398e2a2 to a66aa2e Compare September 9, 2026 17:23
@ianwremmel-ai-agent

Copy link
Copy Markdown
Collaborator Author

The precondition from my last comment is gone: #436 merged, this branch is rebased onto main at d2a3396, and head a66aa2e is fully green — all five test-integration shards pass, plus the new Examples check. Nothing is behind main and there are no conflicts.

That leaves only your approval. The one open question is on the exports["."].require thread: I answered that type: module is not the cause (TypeScript infers a declaration's format from its extension or the nearest package.json, so either setting leaves one half needing a disambiguating extension), and conceded the require condition has no in-repo consumer — but argued that dropping it is a publishing-compatibility decision for its own ticket rather than a silent change here. If you'd rather it go now, say so and I'll open one.

@ianwremmel
ianwremmel merged commit 7426f4b into main Sep 11, 2026
20 checks passed
@ianwremmel
ianwremmel deleted the clc-1130-emit-commonjs-declarations-as-dcts-and-point-the-require branch September 11, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants