Skip to content

fix(build): ship NodeNext-resolvable declarations from every package - #5532

Merged
jrusso1020 merged 4 commits into
mainfrom
rames/nodenext-declarations
Oct 15, 2026
Merged

jrusso1020 merged 4 commits into
mainfrom
rames/nodenext-declarations

Conversation

@jrusso1020

@jrusso1020 jrusso1020 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

What

Every package whose JavaScript or declarations come out of tsc now runs one shared post-emit step, scripts/rewrite-relative-import-extensions.mjs. It gives relative imports in emitted .js and .d.ts files an explicit .js or /index.js. verify:packed-manifests now fails when a published declaration can't be resolved by a strict Node ESM (NodeNext) consumer.

Why

tsc copies relative specifiers into its output exactly as written. Core and inspector write bundler-style imports with no extension (from "./types"), so their published .d.ts files carried those imports too. All packages are "type": "module", so a consumer compiling with module/moduleResolution: "nodenext" can't resolve them. Magi found this on #5530 (inspector, 16 errors). Core had the same bug, with 136 extensionless imports in dist/*.d.ts.

Core already rewrote its emitted JavaScript for this (core_esm_extensions_rewritten), but it skipped declarations. The packed-consumer check already used NodeNext. It missed the bug because it set skipLibCheck: true, which hides errors inside .d.ts files, and because it used bare import "x" side-effect imports, which tsc does not have to resolve.

Related work

Refs #5530, which published inspector with its own declaration rewrite in packages/inspector/build.mjs. This PR removes that local copy, so inspector goes through the shared step like every other tsc-emitting package.

How

  • Removed inspector's local addDeclarationExtensions/resolveDeclaration from build(inspector): publish @hyperframes/inspector to npm #5530 in favor of the shared step.
  • Moved core's scripts/rewrite-esm-extensions.ts (allowlisted in scripts/check-no-main-deletions.mjs) to the repo root as plain ESM, so packages without tsx can use it. It now covers .d.ts as well as .js. A declaration import resolves against foo.d.ts or foo/index.d.ts and becomes ./foo.js or ./foo/index.js. It handles import/export ... from, side-effect imports, and import("./x") type expressions. An unresolvable declaration import fails the build. Unresolvable JavaScript is left alone, as before, because it may sit inside a string, and the existing packed-JS check still rejects a real one.
  • The step runs after every tsc emit: core, engine and sdk (build script), producer, aws-lambda, gcp-cloud-run and inspector (build.mjs). The tsup packages (parsers, lint, studio-server, player, shader-transitions, studio) are left out on purpose. Their declarations are bundled by rollup-plugin-dts, which writes its own chunk specifiers with .js. Studio's bundled .d.ts also has import("./TimelineCanvas") inside a JSDoc link, which the strict mode would reject. The gate still covers those packages.
  • Mass-editing the roughly 750 extensionless source imports in core and inspector was the other option. The post-emit step is smaller, matches what core already did for JS, and keeps working for future source.
  • Dockerfiles that build core from source (Dockerfile.test, packages/gcp-cloud-run/Dockerfile, examples/k8s-jobs/Dockerfile.example) now copy the script. The gcp Dockerfile already copies scripts/package-subpaths.mjs the same way.
  • Gate: the packed consumer now uses import type * as ... from for every published subpath with types. It typechecks with module/moduleResolution: NodeNext, skipLibCheck: false, lib: ESNext + DOM, and types: ["node"]. It also installs @types/react and @types/react-dom from studio's devDependencies, because studio's declarations name React types. A failure lists error counts by owning package. The tsc step takes about 8s instead of 3s.

Test plan

NodeNext consumer errors per package (bun run verify:packed-manifests, using the new gate):

Package Before After
@hyperframes/core 94 0
@hyperframes/inspector 16 0
every other published package 0 0

tsc stops at the 94 core errors it can reach. A static count finds 136 extensionless specifiers across core's dist/*.d.ts before the change and 0 after.

  • Gate fails on current main: Errors by package: @hyperframes/core: 94. It passes with this change, now covering all 14 published packages (166 exports) including inspector. The inspector "before" count was measured before build(inspector): publish @hyperframes/inspector to npm #5530 merged, by packing it with build(inspector): publish @hyperframes/inspector to npm #5530's manifest and no rewrite.
  • Core's emitted JavaScript is byte-identical to main's rewritten output. Only its .d.ts files change. engine, sdk, producer, aws-lambda and gcp-cloud-run dist/ are byte-identical to main.
  • Unit tests: scripts/rewrite-relative-import-extensions.test.mjs (new) covers file, directory, parent, side-effect and import() specifiers, plus the strict-declaration and lenient-JS cases. scripts/verify-packed-manifests.test.mjs adds a real tsc run under the gate's config. It fails on an extensionless declaration import and passes on the .js one.
  • bun run build, bun run verify:packed-manifests, bun run lint, bun run format:check, bun run test:scripts (737 node tests + 109 vitest), fallow audit, and both comment checks pass.
  • Documentation updated (if applicable)
  • Comments follow CONTRIBUTING.md "Comments"

🤖 Generated with Claude Code

jrusso1020 and others added 2 commits October 13, 2026 05:12
tsc copies relative specifiers verbatim, so packages whose sources use
bundler-style extensionless imports (core, inspector) published .d.ts files
that a module/moduleResolution nodenext consumer cannot resolve. Core already
rewrote its emitted JavaScript for this, but not its declarations.

Move that rewrite to scripts/rewrite-relative-import-extensions.mjs, make it
cover .d.ts too (adding .js or /index.js, and failing the build when a
declaration import has no declaration), and run it after every tsc emit:
core, engine, sdk, producer, aws-lambda, gcp-cloud-run and inspector.

verify:packed-manifests now typechecks the clean packed consumer with
skipLibCheck false and type-only imports of every typed subpath, so an
unresolvable import inside a published declaration fails the gate and is
attributed to its package.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@xuanruli xuanruli left a comment

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.

Approving. One required step before merge: the rebase onto main, which currently conflicts in packages/inspector/build.mjs.

Rebase resolution (required). #5530 merged with an inspector-only rewriter: addDeclarationExtensions and resolveDeclaration at the bottom of packages/inspector/build.mjs. Keep your rewriteRelativeImportExtensions(at("./dist/types")) call, then delete both of those functions and their call. Also drop the imports that only they used: existsSync, readdirSync and writeFileSync from node:fs, and dirname and join from node:path. The @hyperframes/parsers entry #5530 added to .fallowrc.jsonc ignoreDependencies should stay; it's about the dependency, not the rewriter.

I checked that dropping the inspector copy loses nothing. On current main, I built inspector's declarations, then re-emitted them with tsc and ran this PR's scripts/rewrite-relative-import-extensions.mjs over dist/types instead. diff -r of the two trees is empty (18 files rewritten, identical output). That includes the parent-directory imports into the bundled core sources.

What I checked

  • The rewriter: file before index resolution, which matches TypeScript. Declarations fail loudly when unresolved, while JS is left alone. import() type expressions and side-effect imports are covered. EXPLICIT_EXTENSION passes .json/.node through.
  • The gate: import type * as plus skipLibCheck: false is the right fix. The old bare import "x" with skipLibCheck: true is exactly how the core and inspector bugs got through. The per-package error count makes a failure easy to route.
  • Dockerfiles that build core from source now copy the script. The ALLOWED_DELETIONS entry for the moved core script has a reason.
  • Locally at fc8c568: scripts/rewrite-relative-import-extensions.test.mjs and scripts/verify-packed-manifests.test.mjs pass (21/21). CI had no failures when I looked.

Non-blocking

  • The specifier patterns also match text inside comments. A .d.ts whose JSDoc happens to contain from "./something" that doesn't resolve will now fail the build. You already excluded studio for its import("./TimelineCanvas") JSDoc link. Since it fails loudly at build time rather than shipping a broken type, I think that's acceptable. It may be worth a line in the script's header so the next person who hits it knows why.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 157 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • textselect-none-px-r0-nested-z100: tracking 0, pressJump -, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0, pressJump -, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0, pressJump -, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@jrusso1020
jrusso1020 force-pushed the rames/nodenext-declarations branch from fc8c568 to 27bd266 Compare October 13, 2026 05:16

@miguel-heygen miguel-heygen left a comment

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.

Reviewed at 27bd2663. The #5530 conflict is resolved in the actual head: inspector now has only the shared rewrite call, and its direct-parsers Fallow entry remains.

The shared resolution preserves file-before-directory lookup and explicit runtime extensions (scripts/rewrite-relative-import-extensions.mjs:23–28). The stricter consumer names typed exports and checks dependencies with skipLibCheck: false (scripts/verify-packed-manifests.mjs:339–362), closing the gap that hid the original declaration bug.

Evidence: 21 focused Node tests pass. In an isolated copy of the published core 0.8.150 package, a strict NodeNext consumer importing all 83 typed exports reproduces 94 core errors before the shared rewrite and zero afterward. The second rewrite changes zero files. All 195 published core JavaScript files remain byte-identical after the rewrite. No shared cache/package files were changed.

I read the completed fresh-head Build job 115141763453: all 14 packed manifests and size budgets pass; the clean consumer resolves 166 exports, executes 156 Node exports, and passes the NodeNext declaration check, Vite resolution and CLI startup. Required checks currently reported by GitHub pass; other CI jobs are still running. The cancelled captures attempt was superseded by a passing required captures check. No local Docker/Windows build or full monorepo rebuild.

Prior coverage: Xuanru already covered rewriter edge cases, the inspector equivalence, Docker copy hooks, deletion allowlist and comment matching limitation. No additional blocker from this pass.

Audited: all 17 changed files, seven post-tsc build hooks, source-copy Docker paths, shared resolver and strict gate/fixture. Trusting: full package builds and packed runtime execution from the fresh CI Build log; native rendering behavior.

Verdict: APPROVE
Reasoning: The current head consolidates the inspector fix without losing its dependency setup, repairs published core declarations in a real strict consumer, and adds a gate that detects the original regression.
— Magi

@jrusso1020
jrusso1020 enabled auto-merge October 13, 2026 05:25
@jrusso1020
jrusso1020 disabled auto-merge October 13, 2026 05:48
…r changes

The package builds now run scripts/rewrite-relative-import-extensions.mjs, so main's
edit_accuracy paths filter has to include it (check-edit-accuracy-filter test).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit f55b366 Oct 15, 2026
78 checks passed
@jrusso1020
jrusso1020 deleted the rames/nodenext-declarations branch October 15, 2026 06:19
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