You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Every line anchor below was re-verified at HEAD e5806e24. Where the original statement of this issue was stale, the correction is called out inline.
Problem
framework-dev.md describes the .d.ts guard family as running "per published exports entry (the overlay types for . plus every subpath, mapped to its sibling .js)". That is true of ONE of the two directions and false of the other.
test/types/dts-no-phantom-exports.test.mjs (the REVERSE direction, Guard .d.ts overlays against .js JSDoc signature drift #1031) genuinely enumerates every entry. entryPairs() iterates Object.entries(pkg.exports || {}) at L148, so every subpath overlay is checked for declaring a value the runtime sibling does not have.
test/types/dts-export-coverage.test.mjs (the FORWARD direction, dogfood: package .d.ts files drift from runtime exports (many import type errors) #388) does not. The whole file is 69 lines and its scope is the hardcoded three-element ENTRIES array at L29 to L33: @webjsdev/core, @webjsdev/server, @webjsdev/server/testing. Every other published subpath is unchecked.
packages/core publishes twelve exports entries carrying a types overlay (., ./directives, ./context, ./task, ./client-router, ./lazy-loader, ./testing, ./client, ./server, ./component, ./registry, ./signals) and packages/server publishes three (., ./check, ./testing). So fifteen overlays exist and three are forward-checked. For the other twelve, a runtime export added with no matching declaration in its sibling .d.ts is invisible to CI, and editor intelligence silently loses the symbol, which is exactly the drift #388 was filed to stop after the @webjsdev/core overlay had gone missing 36 of 82 exports.
Three corrections to the original statement of this issue.
The doc sentence lives at framework-dev.md:132, not :98. The file has grown since the issue was filed.
The issue said packages/core/src/router-client.js carries "roughly ten" underscore-prefixed test-only exports. It carries 63, all in the Internal exports for unit testing block at L5260 to L5300. None of the 63 is declared in src/router-client.d.ts, which is deliberate.
The reverse test's browser allowlist (BROWSER_SURFACES[0].intentionalAbsent, L99) now holds five names (renderToString, renderToStream, setCspNonceProvider, setAssetUrlProvider, setFormActionResolver). Both the issue text and framework-dev.md:132 still say "three".
Measured size of the gap
The expanded check was run read-only before this plan was written, resolving each overlay to its sibling .js the way the reverse test does, importing that file in Node for the real runtime export names, and tsc-checking a fixture that imports every name from the overlay. Result per subpath:
package
subpath
overlay
runtime sibling
runtime exports
_-prefixed
undeclared, non-_
@webjsdev/core
.
index.d.ts
index.js
106
0
0
@webjsdev/core
./directives
src/directives.d.ts
src/directives.js
25
0
0
@webjsdev/core
./context
src/context.d.ts
src/context.js
4
0
0
@webjsdev/core
./task
src/task.d.ts
src/task.js
2
0
0
@webjsdev/core
./client-router
src/router-client.d.ts
src/router-client.js
70
63
2
@webjsdev/core
./lazy-loader
src/lazy-loader.d.ts
src/lazy-loader.js
2
0
0
@webjsdev/core
./testing
src/testing.d.ts
src/testing.js
7
0
0
@webjsdev/core
./client
src/render-client.d.ts
src/render-client.js
1
0
0
@webjsdev/core
./server
src/render-server.d.ts
src/render-server.js
2
0
0
@webjsdev/core
./component
src/component.d.ts
src/component.js
2
0
0
@webjsdev/core
./registry
src/registry.d.ts
src/registry.js
7
0
0
@webjsdev/core
./signals
src/signal.d.ts
src/signal.js
6
0
0
@webjsdev/server
.
index.d.ts
index.js
132
0
0
@webjsdev/server
./check
src/check.d.ts
src/check.js
2
0
0
@webjsdev/server
./testing
src/testing.d.ts
src/testing.js
12
0
0
Totals: 234 runtime exports across the twelve core entries (63 of them _-prefixed, so 171 checked) and 146 across the three server entries (0 _-prefixed).
The whole gap is two exports, both on ./client-router: collectBoundaries and planBoundarySwap. So widening the guard is a small, bounded change and CI will not go red on twelve subpaths. Sizing it up front is the point of this plan.
What those two exports are
packages/core/src/router-client.js declares them at L1540 (export function collectBoundaries(root) {) and L1657 (export function planBoundarySwap(here, there) {), and then re-exports the SAME two functions under aliases inside the Internal exports for unit testing block at L5278 and L5279:
Both the bare export and the _ alias landed in the same commit (01b21276, #1016). Nothing in the repo imports the bare names. packages/core/index.js:31 and packages/core/index-browser.js:55 re-export only enableClientRouter, disableClientRouter, navigate, revalidate, and every test uses the _ alias (packages/core/test/routing/router-client.test.js:80, packages/core/test/routing/browser/partial-fragment-css.test.js:26, packages/core/test/routing/browser/orphaned-marker-navbar.test.js:30). src/router-client.d.ts declares exactly the five intended public functions (enableClientRouter, disableClientRouter, navigate, loadFrame, revalidate). So the bare export keyword on those two is redundant leakage, not API.
Design / approach
Make the forward direction enumerate exports the way the reverse one already does, so the two halves agree and framework-dev.md becomes true rather than aspirational.
test/types/dts-no-phantom-exports.test.mjs is the prior art to copy, because it already solved the hard parts.
entryPairs(pkgDir) at L145 to L155 reads pkg.exports, keeps every entry whose types ends in .d.ts, and derives the runtime .js as the SIBLING (foo.d.ts overlays foo.js). Its comment at L137 to L144 records why a source field is not trusted: only some entries carry one, so keying on source silently skipped every server entry and five core subpaths. Copy this function's logic verbatim in spirit.
PACKAGES at L65 to L68 carries a per-package minEntries floor (12 for core, 3 for server) so a resolution or mapping regression fails loudly instead of checking nothing.
phantomExports() at L194 to L200 throws on error TS2307 / Cannot find module rather than returning a falsely-empty set.
Settled decision 1: the sibling .js is the surface, never the bare specifier
The current forward test does await import(spec) on the bare package specifier. Under Node that resolves the default condition, and for the core subpaths default points at a BUILT bundle, not at the module. Measured at HEAD:
import('@webjsdev/core/directives') -> 101 export names (src/directives.js has 25)
import('@webjsdev/core/context') -> 101 export names (src/context.js has 4)
import('@webjsdev/core/task') -> 101 export names (src/task.js has 2)
import('@webjsdev/core/client-router') -> 101 export names (src/router-client.js has 70)
All four collapse onto dist/webjs-core-browser.js, so a bare-specifier check would judge each overlay against the WHOLE bundle's export set. That is not a stricter check, it is a different and wrong one. Resolving to the sibling .js gives the real per-module surface and matches the reverse test.
A second benefit is that the guard stops depending on a built artifact. Today the . core entry resolves through dist/webjs-core.js, so the forward test cannot run in a fresh worktree until packages/core/dist is built (framework-dev.md and AGENTS.md both note that dist is built rather than committed). In this checkout packages/core/dist/webjs-core-browser.js is already older than src/component.js and src/render-server.js, so it is stale right now. After this change the forward guard reads only committed source. For the record, the switch is a no-op for the . entry today: dist/webjs-core.js and index.js expose the identical 106 names, verified by set difference in both directions.
The source surface is also the correct forward surface for the browser question. The forward direction asks whether the overlay declares everything the runtime has, and the source module's export set is a superset of what any bundle ships, so checking the source is the strictest forward check available. The browser surface matters only in the REVERSE direction (a declaration the browser bundle drops), which #1035 already covers via BROWSER_SURFACES.
Rejected: keep the bare specifier and special-case the collapsing subpaths. That is a hand-maintained list of exactly the entries most likely to change, and it would still leave the guard unable to run without dist.
Settled decision 2: a leading _ is exempt, expressed as a rule, not a list
Exempt every export name matching /^_/ from the forward check, filtered in code with a comment stating the convention.
Why a rule beats a list. There are 63 such names today in one module, and every new client-router unit test can add another, so a hand-maintained ignore list would be edited on unrelated PRs and would rot the moment someone forgot. The _ prefix is already the module's own stated convention (the block is literally headed Internal exports for unit testing), so encoding the convention is encoding what the code already means. This is the standard JavaScript and TypeScript treatment of an underscore-prefixed member, and it is the same shape as noUnusedLocals honouring a leading underscore.
Why not the alternative of declaring them. Adding 63 declarations to src/router-client.d.ts would publish a test seam as API in the editor's autocomplete, which is the opposite of the intent, and the acceptance criteria below forbid it.
The two halves agree with no change to the reverse test. The reverse test computes Exclude<keyof Decl, keyof Impl>, which is declarations the runtime lacks. An underscore export that exists at runtime and is undeclared is invisible to it by construction, so there is nothing there to contradict. It has no underscore handling today (verified by grep) and needs none: if someone ever DECLARES a _foo that the runtime lacks, the reverse test correctly flags it as a phantom, and the forward exemption never fires on that path. So the exemption is one-sided by design and cannot desynchronise the pair.
Settled decision 3: fix the two gaps by removing the redundant export, not by declaring them
Drop the export keyword from the two function declarations in packages/core/src/router-client.js. They remain reachable to tests through the existing _collectBoundaries / _planBoundarySwap aliases, which is how every caller already reaches them.
Evidence, all verified at HEAD:
nothing imports the bare names anywhere in the repo,
index.js and index-browser.js re-export only the four public router functions,
src/router-client.d.ts deliberately declares only the five public functions,
both the bare export and the _ alias were added in the same commit, so the bare form was redundant from birth.
Rejected: declare collectBoundaries and planBoundarySwap in src/router-client.d.ts. That grows the published API surface of @webjsdev/core/client-router by two internal functions purely to satisfy a guard, and it contradicts the alias block that already exists to keep them out. WebJs has no users yet, so a clean removal beats a compatibility shim (see AGENTS.md).
Settled decision 4: three floors, so the widened guard cannot pass vacuously
Vacuity is the real risk when a guard grows from 3 checks to 15, and one floor is not enough because the failure modes differ.
Per-package entry-count floor, matching the reverse test exactly: 12 for @webjsdev/core, 3 for @webjsdev/server. These are today's counts, so a renamed or dropped exports entry fails loudly. Raising an export count only makes the floor stricter, which is the same rationale recorded at test/types/dts-no-phantom-exports.test.mjs:61 to :64.
Per-entry checked-name floor of >= 1. A per-entry number would be pure noise here, since real entries range from 1 name (./client) to 132 (@webjsdev/server root). What matters is that an entry never resolves to zero names, which is what a broken import or a wrong path looks like.
Per-package checked-name total. Today's totals are 171 for core (234 minus the 63 exempt) and 146 for server. Set the floors at 160 and 140. This catches the failure the per-entry floor cannot see, which is an entry silently resolving to a smaller module rather than to nothing.
Settled decision 5: one test() per entry, not one per package
The tsc resolution mechanism is unchanged from today: write a fixture importing every runtime name from the overlay path with its extension stripped, run tsc --noEmit --strict, and scrape no exported member '(...)'. TypeScript resolves an extensionless specifier to .d.ts BEFORE falling through to .js under allowJs, so the fixture reads the overlay and not the implementation. This was verified two ways: on the real corpus (the ./client-router fixture reported exactly the two undeclared names, which only a .d.ts resolution can produce) and synthetically (a foo.js exporting a and b beside a foo.d.ts declaring only a yields TS2305: Module './foo' has no exported member 'b').
Keep the current file's shape of one test() per entry rather than one test() per package looping over entries. Each test() then spawns exactly one tsc process (measured 0.5s to 0.9s each, so the whole file goes from about 2.4s to roughly 12s). The reverse test loops all 12 entries inside a single test(), and that is precisely why it sits on the scripts/run-bun-tests.js denylist at L53 for exceeding bun test's 5 second default per-test timeout. The forward test is NOT on that denylist and must stay off it, so do not collapse the loop.
Rejected: just softening framework-dev.md to match the narrower reality. It is the cheaper change and it is the wrong one, since the sentence describes the guarantee the guard family exists to provide.
Implementation plan
NO FOLLOW-UP ISSUES. Every declaration gap the widened guard turns up is fixed inside THIS PR. The measurement above already enumerates the complete finding set (two exports on ./client-router, listed as step 2 below), so there is nothing to defer. If the implementer's own run surfaces something the measurement did not, it is fixed here as an additional commit in this same PR, and if any single finding genuinely cannot be closed here it is reported as a note in the PR description for the owner to decide, never filed as a new issue.
Suggested commit sequence inside the one PR: (1) remove the two redundant exports, (2) widen the guard, (3) sync the docs.
Step 1: remove the two redundant bare exports
packages/core/src/router-client.js:1540 reads:
exportfunctioncollectBoundaries(root){
Change it to:
functioncollectBoundaries(root){
packages/core/src/router-client.js:1657 reads:
exportfunctionplanBoundarySwap(here,there){
Change it to:
functionplanBoundarySwap(here,there){
Leave the alias block at L5278 and L5279 untouched. Both functions stay exported as _collectBoundaries and _planBoundarySwap, which is what every caller already imports. Do NOT touch src/router-client.d.ts; it already declares exactly the intended public five.
Verify with node --test packages/core/test/routing/router-client.test.js plus the browser suite for the client router, and confirm by grep that no bare-name import exists.
Step 2: rewrite the entry list in test/types/dts-export-coverage.test.mjs
Replace the hardcoded ENTRIES at L29 to L33, which today reads:
// Each published entry point with a hand-maintained `.d.ts` overlay, plus a// minimum-count sanity bound so a botched runtime import (0 names) can't make// the guard vacuously pass.constENTRIES=[{spec: '@webjsdev/core',min: 50},{spec: '@webjsdev/server',min: 50},{spec: '@webjsdev/server/testing',min: 5},];
with a package list plus an entryPairs() reader modelled on test/types/dts-no-phantom-exports.test.mjs:145. The shape to write:
// Published packages whose `.d.ts` are HAND-WRITTEN overlays over `.js` JSDoc.// The entry list is READ from each package's own `exports`, so a new subpath is// covered with no edit here (#1291). `minEntries` is a sanity floor matching the// reverse guard's (`dts-no-phantom-exports.test.mjs`), and `minNames` is the// per-package total of CHECKED export names, so an entry that resolves to a// smaller module than intended fails instead of quietly shrinking the check.constPACKAGES=[{name: '@webjsdev/core',dir: 'packages/core',minEntries: 12,minNames: 160},{name: '@webjsdev/server',dir: 'packages/server',minEntries: 3,minNames: 140},];
Add the reader, deriving the impl by SIBLING rather than from a source field, for the reason recorded in the reverse test's comment:
/** * The `.d.ts` overlay + its runtime `.js` for every package export that declares * a `types`. The impl `.js` is DERIVED from the overlay path (a sibling * `foo.d.ts` overlays `foo.js`), NOT read from a `source` field: only some * entries carry `source`. Same mapping as the reverse guard, so the two halves * check the same surface. */functionentryPairs(pkgDir){constpkg=JSON.parse(readFileSync(join(ROOT,pkgDir,'package.json'),'utf8'));constpairs=[];for(const[key,val]ofObject.entries(pkg.exports||{})){if(!val||typeofval!=='object'||!val.types||!val.types.endsWith('.d.ts'))continue;consttypes=val.types.replace(/^\.\//,'');pairs.push({ key, types,impl: types.replace(/\.d\.ts$/,'.js')});}returnpairs;}
readFileSync must be added to the existing node:fs import at L18.
Step 3: add the underscore exemption as a shared pure function
Add beside entryPairs(), so the real check and its counterfactual exercise the SAME filter (the pattern unexpectedBrowserPhantoms establishes at test/types/dts-no-phantom-exports.test.mjs:118):
/** * Runtime export names the overlay is REQUIRED to declare. A leading `_` marks a * test-only seam that is deliberately NOT part of the published API (the * `Internal exports for unit testing` block in `src/router-client.js` is 63 such * names), so declaring them would publish a test seam as API. The convention is * expressed as a RULE rather than an ignore list, because the list would grow * with every new unit test and rot the moment someone forgot to update it. */functioncheckedNames(mod){returnObject.keys(mod).filter((n)=>n!=='default'&&!n.startsWith('_'));}
Step 4: rewrite the test loop
Replace the loop at L35 to L69. It currently imports the bare spec and generates a fixture importing from that same spec. It must instead import the sibling .js by absolute path (pathToFileURL) and generate a fixture importing from the overlay path with .d.ts stripped. Keep every part of the existing tsc invocation at L47 to L58 and the no exported member scrape at L58 unchanged, keep the generated-fixture write plus rmSync in a finally (L45, L65 to L67), and keep the fixture filename derived from the entry so parallel entries cannot race on one path (today's derivation at L41 is from spec; derive from <package>/<subpath> instead, sanitised with replace(/[^A-Za-z0-9]/g, '_')).
Per package, assert the entry-count floor once, then emit one test() per entry, and assert the package name-total floor after the per-entry tests have contributed their counts (accumulate into a module-scope counter and assert it in a final test() per package, so the assertion is a real test rather than a top-level throw).
Per entry the test must:
await import(pathToFileURL(join(ROOT, dir, impl)).href) and take checkedNames(mod).
Assert names.length >= 1, failing with the entry key and the impl path.
Write the fixture, run tsc, scrape no exported member, and assert.deepEqual(missing, []) with a message naming the package, the subpath, the overlay path and the missing names.
Throw on error TS2307 / Cannot find module in the tsc output, mirroring test/types/dts-no-phantom-exports.test.mjs:198, so a resolution break is a loud failure and never a silent empty missing.
Keep assert.equal(res.status, 0, ...) from L64 so an unrelated tsc error still fails the entry.
Step 5: docs
Covered in the Docs section below. Do these in the same PR.
Tests
The deliverable IS a test, so the proof is the counterfactual and the vacuity guards, not new coverage of a runtime behaviour. Layers: unit only. There is no browser, e2e, Bun-parity or smoke surface here, because nothing runtime-sensitive changes. Step 1 does touch packages/core/src/router-client.js, but only by removing two redundant export keywords, so the existing client-router unit and browser suites are the regression proof for it and must be run.
Counterfactual A, the widened coverage is real. Temporarily delete one declaration from a previously-unguarded overlay, for example the signal declaration in packages/core/src/signal.d.ts or a name in packages/core/src/component.d.ts, and confirm the guard reds naming that export against that subpath. Then git stash the test-file widening alone and confirm the SAME deletion passes, which is what proves the new coverage rather than the old three-entry list caught it. Restore both. Record the observed failure message in the PR description.
Counterfactual B, the underscore exemption is exercised and correct. Add a permanent synthetic test in the same file, modelled on the counterfactual tests at test/types/dts-no-phantom-exports.test.mjs:311 onwards, asserting checkedNames({ a: 1, _b: 2, default: 3 }) returns exactly ['a']. Additionally assert, against the REAL corpus, that the exemption actually fires somewhere: the total exempt count across all entries must be >= 1 (63 today). Without that, a rename of the router-client seam would leave the exemption dead code and nobody would notice.
Counterfactual C, the .d.ts beats .js resolution assumption holds. Add a permanent synthetic test writing a temp foo.js exporting a and b beside a foo.d.ts declaring only a, then run the same tsc invocation on a fixture importing both and assert b is reported. This is the assumption the entire mechanism rests on. It was verified by hand while writing this plan (error TS2305: Module './foo' has no exported member 'b') and belongs in the file so it stays verified.
Vacuity guards, permanent. The three floors from Design decision 4 (entry count 12 and 3, per-entry >= 1, per-package checked-name totals 160 and 140) plus the TS2307 throw. Prove the entry-count floor fires by temporarily pointing entryPairs() at a package with no exports and confirming the loud failure, then restore.
Commands to run and report.
node --test test/types/dts-export-coverage.test.mjs, which must report 15 entry tests plus the counterfactuals and the per-package total tests.
node --test test/types/dts-no-phantom-exports.test.mjs, unchanged and still green.
node --test packages/core/test/routing/router-client.test.js and the client-router browser tests via npm run test:browser, for step 1.
node --test packages/server/test/types/exports-drift.test.mjs, the third package-local guard, unaffected but cheap to confirm.
npm test for the full Node suite.
node scripts/run-bun-tests.js scoped with WEBJS_BUN_TESTS=test/types/dts-export-coverage, confirming the file still passes under Bun and does not need a denylist entry. If any single entry test exceeds bun test's 5 second default timeout, the fix is to keep one tsc spawn per test(), not to add a denylist entry.
webjs check from an in-repo app.
Docs
framework-dev.md:132 is the only sentence that makes the false claim. The clause "per published exports entry (the overlay types for . plus every subpath, mapped to its sibling .js)" becomes true for both directions once step 2 lands, so it stays. Add to the dts-export-coverage.test.mjs half of the sentence that the forward guard now reads the entry list from each package's own exports, resolves each overlay to its sibling .js rather than to the bare specifier (which collapses several core subpaths onto one built bundle), exempts a leading _ as a test-only seam, and carries the three floors. While editing this exact sentence, also fix the stale "three intentional server-only strips (renderToString / renderToStream / setCspNonceProvider)" to the five names now in intentionalAbsent (adding setAssetUrlProvider and setFormActionResolver). That is a one-clause correction in the sentence the PR already rewrites, so it belongs here rather than anywhere else.
packages/core/AGENTS.md:66 to :72 currently scopes the enforcement to index.d.ts ("The index.d.ts overlay must declare every runtime named export" and "a new export in index.js without a matching declaration fails CI"). Widen it to say every overlay behind a published exports entry, and add the underscore convention in one sentence, so someone adding a test seam knows the prefix is what keeps it out of the API.
packages/server/AGENTS.md:132 to :135 describes a DIFFERENT guard, the package-local packages/server/test/types/exports-drift.test.mjs, whose claim is about index.js and index.d.ts only and stays true. Verify and leave unchanged.
.agents/skills/webjs/references/typescript.md:217 states the consumer-facing fact that both packages ship hand-authored overlays with a types condition. That is unchanged by this work. Verify and leave unchanged.
No docs-site, marketing website, README or scaffold-template surface applies. This is a repo-internal CI guard with no public API, no CLI flag and no config key, so AGENTS.md at the repo root is not a surface either. Commit with WEBJS_NO_DOC_GATE=1 only if the doc hook still fires after the two edits above, which it should not.
Acceptance criteria
test/types/dts-export-coverage.test.mjs derives its entry list from each package's published exports rather than a hardcoded array, using the same sibling mapping as test/types/dts-no-phantom-exports.test.mjs:145
All fifteen overlays carrying a types entry are forward-checked, and the run reports fifteen entry tests
Each entry is checked against its sibling .js, never against the bare package specifier, and the reason (the four core subpaths collapsing onto dist/webjs-core-browser.js) is stated in a comment
The forward guard no longer requires packages/core/dist to be built in order to run
Underscore-prefixed exports are exempt via a /^_/ rule in a shared pure function, with the reason in a comment, and no ignore list of names exists anywhere in the file
packages/core/src/router-client.js no longer bare-exports collectBoundaries or planBoundarySwap; both remain available as _collectBoundaries and _planBoundarySwap, and the client-router unit and browser suites are green
No _-prefixed export gains a declaration as a side effect of this change, and packages/core/src/router-client.d.ts still declares exactly its five public functions
test/types/dts-no-phantom-exports.test.mjs is unchanged and still green
Three floors are in place and each has been shown to fire: entry count (12 core, 3 server), per-entry >= 1 checked name, per-package checked-name totals (160 core, 140 server)
A resolution failure throws rather than yielding an empty missing set (the TS2307 guard)
Permanent synthetic tests cover the underscore filter and the .d.ts-beats-.js resolution assumption
The Counterfactual A result (a deleted declaration on a previously-unguarded overlay reds the widened guard and passes the un-widened one) is recorded in the PR description
The file still passes under node scripts/run-bun-tests.js without a new denylist entry, because it keeps one tsc spawn per test()
framework-dev.md:132 describes what the tests actually do, including the corrected five-name browser strip list
packages/core/AGENTS.md describes the widened scope and the underscore convention
No follow-up issue was filed for anything this change turned up
Out of scope
Do not widen into the reverse guard.test/types/dts-no-phantom-exports.test.mjs already enumerates every entry and needs no change. Adding underscore handling there is unnecessary (the direction cannot see an undeclared runtime export) and would only create a second place to keep in sync.
Do not touch the third guard, packages/server/test/types/exports-drift.test.mjs. It is package-local, covers index.js against index.d.ts only, and its documented claim stays true.
Do not add signature checking. This guard is about export EXISTENCE. Per-signature correctness is covered positively by test/types/complex-export-signatures.test-d.ts through the type-fixtures.test.mjs runner, and the reverse test's header at L11 to L21 records why a structural shape diff was rejected.
Do not add a browser-surface pass to the forward direction. The source module's export set is a superset of any bundle's, so the source check is already the strictest forward check. The browser surface matters only in reverse, where Extend the phantom-export guard to the browser bundle surface #1035 covers it.
Do not rebuild or commit packages/core/dist. It is a build output, and after this change the forward guard does not read it.
Do not change the published API surface beyond removing the two redundant bare exports named above. No new export, no new declaration, no rename of a _-prefixed seam.
Do not add a .ts file under packages/. The repo is buildless there. Test fixtures stay under test/types/ as .mjs and .test-d.ts, matching the existing naming.
Do not extend the guard to other packages (cli, mcp, ui, intellisense). This issue is scoped to the two packages that ship hand-maintained overlays, which is the same scope the reverse guard uses.
Problem
framework-dev.mddescribes the.d.tsguard family as running "per publishedexportsentry (the overlaytypesfor.plus every subpath, mapped to its sibling.js)". That is true of ONE of the two directions and false of the other.test/types/dts-no-phantom-exports.test.mjs(the REVERSE direction, Guard .d.ts overlays against .js JSDoc signature drift #1031) genuinely enumerates every entry.entryPairs()iteratesObject.entries(pkg.exports || {})at L148, so every subpath overlay is checked for declaring a value the runtime sibling does not have.test/types/dts-export-coverage.test.mjs(the FORWARD direction, dogfood: package .d.ts files drift from runtime exports (many import type errors) #388) does not. The whole file is 69 lines and its scope is the hardcoded three-elementENTRIESarray at L29 to L33:@webjsdev/core,@webjsdev/server,@webjsdev/server/testing. Every other published subpath is unchecked.packages/corepublishes twelveexportsentries carrying atypesoverlay (.,./directives,./context,./task,./client-router,./lazy-loader,./testing,./client,./server,./component,./registry,./signals) andpackages/serverpublishes three (.,./check,./testing). So fifteen overlays exist and three are forward-checked. For the other twelve, a runtime export added with no matching declaration in its sibling.d.tsis invisible to CI, and editor intelligence silently loses the symbol, which is exactly the drift #388 was filed to stop after the@webjsdev/coreoverlay had gone missing 36 of 82 exports.Three corrections to the original statement of this issue.
framework-dev.md:132, not:98. The file has grown since the issue was filed.packages/core/src/router-client.jscarries "roughly ten" underscore-prefixed test-only exports. It carries 63, all in theInternal exports for unit testingblock at L5260 to L5300. None of the 63 is declared insrc/router-client.d.ts, which is deliberate.BROWSER_SURFACES[0].intentionalAbsent, L99) now holds five names (renderToString,renderToStream,setCspNonceProvider,setAssetUrlProvider,setFormActionResolver). Both the issue text andframework-dev.md:132still say "three".Measured size of the gap
The expanded check was run read-only before this plan was written, resolving each overlay to its sibling
.jsthe way the reverse test does, importing that file in Node for the real runtime export names, and tsc-checking a fixture that imports every name from the overlay. Result per subpath:_-prefixed_@webjsdev/core.index.d.tsindex.js@webjsdev/core./directivessrc/directives.d.tssrc/directives.js@webjsdev/core./contextsrc/context.d.tssrc/context.js@webjsdev/core./tasksrc/task.d.tssrc/task.js@webjsdev/core./client-routersrc/router-client.d.tssrc/router-client.js@webjsdev/core./lazy-loadersrc/lazy-loader.d.tssrc/lazy-loader.js@webjsdev/core./testingsrc/testing.d.tssrc/testing.js@webjsdev/core./clientsrc/render-client.d.tssrc/render-client.js@webjsdev/core./serversrc/render-server.d.tssrc/render-server.js@webjsdev/core./componentsrc/component.d.tssrc/component.js@webjsdev/core./registrysrc/registry.d.tssrc/registry.js@webjsdev/core./signalssrc/signal.d.tssrc/signal.js@webjsdev/server.index.d.tsindex.js@webjsdev/server./checksrc/check.d.tssrc/check.js@webjsdev/server./testingsrc/testing.d.tssrc/testing.jsTotals: 234 runtime exports across the twelve core entries (63 of them
_-prefixed, so 171 checked) and 146 across the three server entries (0_-prefixed).The whole gap is two exports, both on
./client-router:collectBoundariesandplanBoundarySwap. So widening the guard is a small, bounded change and CI will not go red on twelve subpaths. Sizing it up front is the point of this plan.What those two exports are
packages/core/src/router-client.jsdeclares them at L1540 (export function collectBoundaries(root) {) and L1657 (export function planBoundarySwap(here, there) {), and then re-exports the SAME two functions under aliases inside theInternal exports for unit testingblock at L5278 and L5279:Both the bare
exportand the_alias landed in the same commit (01b21276, #1016). Nothing in the repo imports the bare names.packages/core/index.js:31andpackages/core/index-browser.js:55re-export onlyenableClientRouter,disableClientRouter,navigate,revalidate, and every test uses the_alias (packages/core/test/routing/router-client.test.js:80,packages/core/test/routing/browser/partial-fragment-css.test.js:26,packages/core/test/routing/browser/orphaned-marker-navbar.test.js:30).src/router-client.d.tsdeclares exactly the five intended public functions (enableClientRouter,disableClientRouter,navigate,loadFrame,revalidate). So the bareexportkeyword on those two is redundant leakage, not API.Design / approach
Make the forward direction enumerate
exportsthe way the reverse one already does, so the two halves agree andframework-dev.mdbecomes true rather than aspirational.test/types/dts-no-phantom-exports.test.mjsis the prior art to copy, because it already solved the hard parts.entryPairs(pkgDir)at L145 to L155 readspkg.exports, keeps every entry whosetypesends in.d.ts, and derives the runtime.jsas the SIBLING (foo.d.tsoverlaysfoo.js). Its comment at L137 to L144 records why asourcefield is not trusted: only some entries carry one, so keying onsourcesilently skipped every server entry and five core subpaths. Copy this function's logic verbatim in spirit.PACKAGESat L65 to L68 carries a per-packageminEntriesfloor (12 for core, 3 for server) so a resolution or mapping regression fails loudly instead of checking nothing.phantomExports()at L194 to L200 throws onerror TS2307/Cannot find modulerather than returning a falsely-empty set.Settled decision 1: the sibling
.jsis the surface, never the bare specifierThe current forward test does
await import(spec)on the bare package specifier. Under Node that resolves thedefaultcondition, and for the core subpathsdefaultpoints at a BUILT bundle, not at the module. Measured at HEAD:All four collapse onto
dist/webjs-core-browser.js, so a bare-specifier check would judge each overlay against the WHOLE bundle's export set. That is not a stricter check, it is a different and wrong one. Resolving to the sibling.jsgives the real per-module surface and matches the reverse test.A second benefit is that the guard stops depending on a built artifact. Today the
.core entry resolves throughdist/webjs-core.js, so the forward test cannot run in a fresh worktree untilpackages/core/distis built (framework-dev.mdandAGENTS.mdboth note thatdistis built rather than committed). In this checkoutpackages/core/dist/webjs-core-browser.jsis already older thansrc/component.jsandsrc/render-server.js, so it is stale right now. After this change the forward guard reads only committed source. For the record, the switch is a no-op for the.entry today:dist/webjs-core.jsandindex.jsexpose the identical 106 names, verified by set difference in both directions.The source surface is also the correct forward surface for the browser question. The forward direction asks whether the overlay declares everything the runtime has, and the source module's export set is a superset of what any bundle ships, so checking the source is the strictest forward check available. The browser surface matters only in the REVERSE direction (a declaration the browser bundle drops), which #1035 already covers via
BROWSER_SURFACES.Rejected: keep the bare specifier and special-case the collapsing subpaths. That is a hand-maintained list of exactly the entries most likely to change, and it would still leave the guard unable to run without
dist.Settled decision 2: a leading
_is exempt, expressed as a rule, not a listExempt every export name matching
/^_/from the forward check, filtered in code with a comment stating the convention.Why a rule beats a list. There are 63 such names today in one module, and every new client-router unit test can add another, so a hand-maintained ignore list would be edited on unrelated PRs and would rot the moment someone forgot. The
_prefix is already the module's own stated convention (the block is literally headedInternal exports for unit testing), so encoding the convention is encoding what the code already means. This is the standard JavaScript and TypeScript treatment of an underscore-prefixed member, and it is the same shape asnoUnusedLocalshonouring a leading underscore.Why not the alternative of declaring them. Adding 63 declarations to
src/router-client.d.tswould publish a test seam as API in the editor's autocomplete, which is the opposite of the intent, and the acceptance criteria below forbid it.The two halves agree with no change to the reverse test. The reverse test computes
Exclude<keyof Decl, keyof Impl>, which is declarations the runtime lacks. An underscore export that exists at runtime and is undeclared is invisible to it by construction, so there is nothing there to contradict. It has no underscore handling today (verified by grep) and needs none: if someone ever DECLARES a_foothat the runtime lacks, the reverse test correctly flags it as a phantom, and the forward exemption never fires on that path. So the exemption is one-sided by design and cannot desynchronise the pair.Settled decision 3: fix the two gaps by removing the redundant
export, not by declaring themDrop the
exportkeyword from the twofunctiondeclarations inpackages/core/src/router-client.js. They remain reachable to tests through the existing_collectBoundaries/_planBoundarySwapaliases, which is how every caller already reaches them.Evidence, all verified at HEAD:
index.jsandindex-browser.jsre-export only the four public router functions,src/router-client.d.tsdeliberately declares only the five public functions,_alias were added in the same commit, so the bare form was redundant from birth.Rejected: declare
collectBoundariesandplanBoundarySwapinsrc/router-client.d.ts. That grows the published API surface of@webjsdev/core/client-routerby two internal functions purely to satisfy a guard, and it contradicts the alias block that already exists to keep them out. WebJs has no users yet, so a clean removal beats a compatibility shim (seeAGENTS.md).Settled decision 4: three floors, so the widened guard cannot pass vacuously
Vacuity is the real risk when a guard grows from 3 checks to 15, and one floor is not enough because the failure modes differ.
@webjsdev/core, 3 for@webjsdev/server. These are today's counts, so a renamed or droppedexportsentry fails loudly. Raising an export count only makes the floor stricter, which is the same rationale recorded attest/types/dts-no-phantom-exports.test.mjs:61to:64.>= 1. A per-entry number would be pure noise here, since real entries range from 1 name (./client) to 132 (@webjsdev/serverroot). What matters is that an entry never resolves to zero names, which is what a broken import or a wrong path looks like.Settled decision 5: one
test()per entry, not one per packageThe tsc resolution mechanism is unchanged from today: write a fixture importing every runtime name from the overlay path with its extension stripped, run
tsc --noEmit --strict, and scrapeno exported member '(...)'. TypeScript resolves an extensionless specifier to.d.tsBEFORE falling through to.jsunderallowJs, so the fixture reads the overlay and not the implementation. This was verified two ways: on the real corpus (the./client-routerfixture reported exactly the two undeclared names, which only a.d.tsresolution can produce) and synthetically (afoo.jsexportingaandbbeside afoo.d.tsdeclaring onlyayieldsTS2305: Module './foo' has no exported member 'b').Keep the current file's shape of one
test()per entry rather than onetest()per package looping over entries. Eachtest()then spawns exactly one tsc process (measured 0.5s to 0.9s each, so the whole file goes from about 2.4s to roughly 12s). The reverse test loops all 12 entries inside a singletest(), and that is precisely why it sits on thescripts/run-bun-tests.jsdenylist at L53 for exceedingbun test's 5 second default per-test timeout. The forward test is NOT on that denylist and must stay off it, so do not collapse the loop.Rejected: just softening
framework-dev.mdto match the narrower reality. It is the cheaper change and it is the wrong one, since the sentence describes the guarantee the guard family exists to provide.Implementation plan
NO FOLLOW-UP ISSUES. Every declaration gap the widened guard turns up is fixed inside THIS PR. The measurement above already enumerates the complete finding set (two exports on
./client-router, listed as step 2 below), so there is nothing to defer. If the implementer's own run surfaces something the measurement did not, it is fixed here as an additional commit in this same PR, and if any single finding genuinely cannot be closed here it is reported as a note in the PR description for the owner to decide, never filed as a new issue.Suggested commit sequence inside the one PR: (1) remove the two redundant exports, (2) widen the guard, (3) sync the docs.
Step 1: remove the two redundant bare exports
packages/core/src/router-client.js:1540reads:Change it to:
packages/core/src/router-client.js:1657reads:Change it to:
Leave the alias block at L5278 and L5279 untouched. Both functions stay exported as
_collectBoundariesand_planBoundarySwap, which is what every caller already imports. Do NOT touchsrc/router-client.d.ts; it already declares exactly the intended public five.Verify with
node --test packages/core/test/routing/router-client.test.jsplus the browser suite for the client router, and confirm by grep that no bare-name import exists.Step 2: rewrite the entry list in
test/types/dts-export-coverage.test.mjsReplace the hardcoded
ENTRIESat L29 to L33, which today reads:with a package list plus an
entryPairs()reader modelled ontest/types/dts-no-phantom-exports.test.mjs:145. The shape to write:Add the reader, deriving the impl by SIBLING rather than from a
sourcefield, for the reason recorded in the reverse test's comment:readFileSyncmust be added to the existingnode:fsimport at L18.Step 3: add the underscore exemption as a shared pure function
Add beside
entryPairs(), so the real check and its counterfactual exercise the SAME filter (the patternunexpectedBrowserPhantomsestablishes attest/types/dts-no-phantom-exports.test.mjs:118):Step 4: rewrite the test loop
Replace the loop at L35 to L69. It currently imports the bare
specand generates a fixture importing from that samespec. It must instead import the sibling.jsby absolute path (pathToFileURL) and generate a fixture importing from the overlay path with.d.tsstripped. Keep every part of the existing tsc invocation at L47 to L58 and theno exported memberscrape at L58 unchanged, keep the generated-fixture write plusrmSyncin afinally(L45, L65 to L67), and keep the fixture filename derived from the entry so parallel entries cannot race on one path (today's derivation at L41 is fromspec; derive from<package>/<subpath>instead, sanitised withreplace(/[^A-Za-z0-9]/g, '_')).Per package, assert the entry-count floor once, then emit one
test()per entry, and assert the package name-total floor after the per-entry tests have contributed their counts (accumulate into a module-scope counter and assert it in a finaltest()per package, so the assertion is a real test rather than a top-level throw).Per entry the test must:
await import(pathToFileURL(join(ROOT, dir, impl)).href)and takecheckedNames(mod).names.length >= 1, failing with the entry key and the impl path.no exported member, andassert.deepEqual(missing, [])with a message naming the package, the subpath, the overlay path and the missing names.error TS2307/Cannot find modulein the tsc output, mirroringtest/types/dts-no-phantom-exports.test.mjs:198, so a resolution break is a loud failure and never a silent emptymissing.assert.equal(res.status, 0, ...)from L64 so an unrelated tsc error still fails the entry.Step 5: docs
Covered in the Docs section below. Do these in the same PR.
Tests
The deliverable IS a test, so the proof is the counterfactual and the vacuity guards, not new coverage of a runtime behaviour. Layers: unit only. There is no browser, e2e, Bun-parity or smoke surface here, because nothing runtime-sensitive changes. Step 1 does touch
packages/core/src/router-client.js, but only by removing two redundant export keywords, so the existing client-router unit and browser suites are the regression proof for it and must be run.Counterfactual A, the widened coverage is real. Temporarily delete one declaration from a previously-unguarded overlay, for example the
signaldeclaration inpackages/core/src/signal.d.tsor a name inpackages/core/src/component.d.ts, and confirm the guard reds naming that export against that subpath. Thengit stashthe test-file widening alone and confirm the SAME deletion passes, which is what proves the new coverage rather than the old three-entry list caught it. Restore both. Record the observed failure message in the PR description.Counterfactual B, the underscore exemption is exercised and correct. Add a permanent synthetic test in the same file, modelled on the counterfactual tests at
test/types/dts-no-phantom-exports.test.mjs:311onwards, assertingcheckedNames({ a: 1, _b: 2, default: 3 })returns exactly['a']. Additionally assert, against the REAL corpus, that the exemption actually fires somewhere: the total exempt count across all entries must be>= 1(63 today). Without that, a rename of the router-client seam would leave the exemption dead code and nobody would notice.Counterfactual C, the
.d.tsbeats.jsresolution assumption holds. Add a permanent synthetic test writing a tempfoo.jsexportingaandbbeside afoo.d.tsdeclaring onlya, then run the same tsc invocation on a fixture importing both and assertbis reported. This is the assumption the entire mechanism rests on. It was verified by hand while writing this plan (error TS2305: Module './foo' has no exported member 'b') and belongs in the file so it stays verified.Vacuity guards, permanent. The three floors from Design decision 4 (entry count 12 and 3, per-entry
>= 1, per-package checked-name totals 160 and 140) plus the TS2307 throw. Prove the entry-count floor fires by temporarily pointingentryPairs()at a package with noexportsand confirming the loud failure, then restore.Commands to run and report.
node --test test/types/dts-export-coverage.test.mjs, which must report 15 entry tests plus the counterfactuals and the per-package total tests.node --test test/types/dts-no-phantom-exports.test.mjs, unchanged and still green.node --test packages/core/test/routing/router-client.test.jsand the client-router browser tests vianpm run test:browser, for step 1.node --test packages/server/test/types/exports-drift.test.mjs, the third package-local guard, unaffected but cheap to confirm.npm testfor the full Node suite.node scripts/run-bun-tests.jsscoped withWEBJS_BUN_TESTS=test/types/dts-export-coverage, confirming the file still passes under Bun and does not need a denylist entry. If any single entry test exceedsbun test's 5 second default timeout, the fix is to keep one tsc spawn pertest(), not to add a denylist entry.webjs checkfrom an in-repo app.Docs
framework-dev.md:132is the only sentence that makes the false claim. The clause "per publishedexportsentry (the overlaytypesfor.plus every subpath, mapped to its sibling.js)" becomes true for both directions once step 2 lands, so it stays. Add to thedts-export-coverage.test.mjshalf of the sentence that the forward guard now reads the entry list from each package's ownexports, resolves each overlay to its sibling.jsrather than to the bare specifier (which collapses several core subpaths onto one built bundle), exempts a leading_as a test-only seam, and carries the three floors. While editing this exact sentence, also fix the stale "three intentional server-only strips (renderToString/renderToStream/setCspNonceProvider)" to the five names now inintentionalAbsent(addingsetAssetUrlProviderandsetFormActionResolver). That is a one-clause correction in the sentence the PR already rewrites, so it belongs here rather than anywhere else.packages/core/AGENTS.md:66to:72currently scopes the enforcement toindex.d.ts("Theindex.d.tsoverlay must declare every runtime named export" and "a newexportinindex.jswithout a matching declaration fails CI"). Widen it to say every overlay behind a publishedexportsentry, and add the underscore convention in one sentence, so someone adding a test seam knows the prefix is what keeps it out of the API.packages/server/AGENTS.md:132to:135describes a DIFFERENT guard, the package-localpackages/server/test/types/exports-drift.test.mjs, whose claim is aboutindex.jsandindex.d.tsonly and stays true. Verify and leave unchanged..agents/skills/webjs/references/typescript.md:217states the consumer-facing fact that both packages ship hand-authored overlays with atypescondition. That is unchanged by this work. Verify and leave unchanged.AGENTS.mdat the repo root is not a surface either. Commit withWEBJS_NO_DOC_GATE=1only if the doc hook still fires after the two edits above, which it should not.Acceptance criteria
test/types/dts-export-coverage.test.mjsderives its entry list from each package's publishedexportsrather than a hardcoded array, using the same sibling mapping astest/types/dts-no-phantom-exports.test.mjs:145typesentry are forward-checked, and the run reports fifteen entry tests.js, never against the bare package specifier, and the reason (the four core subpaths collapsing ontodist/webjs-core-browser.js) is stated in a commentpackages/core/distto be built in order to run/^_/rule in a shared pure function, with the reason in a comment, and no ignore list of names exists anywhere in the filepackages/core/src/router-client.jsno longer bare-exportscollectBoundariesorplanBoundarySwap; both remain available as_collectBoundariesand_planBoundarySwap, and the client-router unit and browser suites are green_-prefixed export gains a declaration as a side effect of this change, andpackages/core/src/router-client.d.tsstill declares exactly its five public functionstest/types/dts-no-phantom-exports.test.mjsis unchanged and still green>= 1checked name, per-package checked-name totals (160 core, 140 server)missingset (the TS2307 guard).d.ts-beats-.jsresolution assumptionnode scripts/run-bun-tests.jswithout a new denylist entry, because it keeps one tsc spawn pertest()framework-dev.md:132describes what the tests actually do, including the corrected five-name browser strip listpackages/core/AGENTS.mddescribes the widened scope and the underscore conventionOut of scope
test/types/dts-no-phantom-exports.test.mjsalready enumerates every entry and needs no change. Adding underscore handling there is unnecessary (the direction cannot see an undeclared runtime export) and would only create a second place to keep in sync.packages/server/test/types/exports-drift.test.mjs. It is package-local, coversindex.jsagainstindex.d.tsonly, and its documented claim stays true.test/types/complex-export-signatures.test-d.tsthrough thetype-fixtures.test.mjsrunner, and the reverse test's header at L11 to L21 records why a structural shape diff was rejected.packages/core/dist. It is a build output, and after this change the forward guard does not read it._-prefixed seam..tsfile underpackages/. The repo is buildless there. Test fixtures stay undertest/types/as.mjsand.test-d.ts, matching the existing naming.cli,mcp,ui,intellisense). This issue is scoped to the two packages that ship hand-maintained overlays, which is the same scope the reverse guard uses.