Skip to content

dts-export-coverage checks 3 of 15 overlays, not every exports subpath #1291

Description

@vivek7405

Problem

framework-dev.md:98 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.

The gap is not small. packages/core alone 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 to (say) src/router-client.js, src/component.js, or src/signal.js with no matching declaration in its sibling .d.ts is invisible to CI. 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.

This surfaced while reviewing a change that added an export to packages/core/src/router-client.js. Both guards were reported green, which was true and proved nothing about ./client-router, because that subpath is not in the coverage test's list. (No fix is owed to that change: the export in question was renamed _setHardNavigate and joined the underscore-prefixed test-only block, and none of the roughly ten existing _-prefixed exports in that file carry declarations either. The convention is fine; the guard's advertised scope is what is wrong.)

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.

The reverse test is the model to copy, since it already solved the hard parts: it maps each overlay to its runtime .js by SIBLING (foo.d.ts overlays foo.js) rather than trusting a source field, and it keeps a per-package entry-count floor so a resolution break fails loudly instead of passing vacuously.

Two things need deciding, and both should be settled deliberately rather than discovered mid-implementation:

  1. Underscore-prefixed test-only exports must be excluded. packages/core/src/router-client.js ends with a block of them (_prefetchPeek, _prefetchInflightSize, _resetPrefetch, _navToken, _bumpNavToken, _currentPageUrl, _setCurrentPageUrl, _resetWarnOnce, _shouldFullLoadDuringParse, _isNonHtmlPath, _setHardNavigate), none declared in src/router-client.d.ts, deliberately: they are a test seam, not API. Widening the guard without an exclusion turns every one of them into a failure and invites the wrong fix (adding declarations, which would publish them as API). Exclude a leading _ and say so in a comment.

  2. The browser-versus-node surface split. The . overlay for core is already dual-surface in the reverse test (checked against index.js AND index-browser.js, with an allowlist for the three intentional server-only strips). A subpath like ./client-router resolves to the browser bundle under default and to src/ under source, so decide which surface the forward check reads and write the reason down.

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, and twelve unguarded overlays is a real hole rather than a documentation nicety.

Implementation notes (for the implementing agent)

Where to edit:

  • test/types/dts-export-coverage.test.mjs, the ENTRIES constant around L29 and the for (const { spec, min } of ENTRIES) loop below it. Today it imports the spec, lists Object.keys(mod), generates a .ts fixture importing every name, and runs tsc --noEmit --strict, scraping no exported member '(...)' out of the output. That mechanism works and can stay; what changes is how the entry list is built.
  • test/types/dts-no-phantom-exports.test.mjs around L148 for the enumeration pattern to copy, and its KNOWN_PHANTOMS map (L77) for how a deliberate exception is recorded with an issue link.
  • framework-dev.md:98, the sentence describing the family. Update it to match whatever the code ends up doing.
  • packages/core/package.json and packages/server/package.json exports blocks are the source of truth for the entry list; do not hardcode the twelve/three paths above, they will drift.

Landmines / gotchas:

  • Do not add declarations for _-prefixed exports to make the guard pass. They are intentionally undeclared so they stay out of the API surface; packages/core/AGENTS.md documents the test-only block. Excluding them is the fix.
  • The existing test writes a generated fixture next to itself and removes it in a finally. Keep that, and keep the generated filename derived from the spec, or parallel entries will race on one path.
  • The min count floor is load-bearing: a subpath whose import silently yields zero names would otherwise pass vacuously. Every entry needs a floor, and a subpath's floor is necessarily small, so consider asserting >= 1 plus a total across all entries rather than a per-entry number that is noise.
  • Some subpaths collapse onto the same browser bundle under the default condition (./directives, ./context, ./task all map to dist/webjs-core-browser.js), so importing the bare specifier may hand back the WHOLE bundle's exports rather than that module's. That would make the check assert the wrong thing (an overlay for one module judged against every export in the bundle). Resolve through the source condition, or read the sibling .js directly the way the reverse test does.
  • @webjsdev/core . is dual-surface; see the reverse test's handling and the three-strip allowlist (renderToString / renderToStream / setCspNonceProvider).

Invariants to respect:

  • packages/ is plain .js with JSDoc; no .ts there. Test files under test/ are .mjs.
  • AGENTS.md invariant 11 applies to all prose added (no em-dash, no pause-hyphen or pause-semicolon, WebJs capitalized in prose, lowercase webjs only as a literal code token).
  • The guard must fail loudly rather than vacuously. A resolution error is a failure, never a silent zero-export pass.

Tests + docs surfaces:

  • Unit: the change IS a test, so the proof is a counterfactual. Delete a declaration from one currently-unguarded overlay (for example a name in packages/core/src/signal.d.ts or src/component.d.ts), confirm the widened guard reds naming that export, and restore. Reverting the widening must make that same deletion pass again, which is what proves the new coverage is real.
  • Also assert the _-prefixed exclusion positively: with the exclusion in place the router-client test-only block must NOT trip the guard, and that expectation should be visible in the test rather than implicit.
  • Docs: framework-dev.md:98. Check whether packages/core/AGENTS.md and packages/server/AGENTS.md repeat the claim before assuming they do not.
  • Browser / e2e / Bun: N/A, this is a Node-side type guard with no runtime surface.

Acceptance criteria

  • dts-export-coverage derives its entry list from each package's published exports rather than a hardcoded array
  • All fifteen overlays currently carrying a types entry are forward-checked, not three
  • Underscore-prefixed test-only exports are excluded, with the reason stated in the test
  • The surface a subpath is checked against (source versus the collapsed browser bundle) is chosen deliberately and documented in a comment
  • A counterfactual proves the widened guard actually fires: removing a declaration from a previously-unguarded overlay reds it, and reverting the widening makes that same removal pass
  • No _-prefixed export gains a declaration as a side effect of this change
  • framework-dev.md:98 describes what the tests actually do

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions