Skip to content

Implement private type module separation and prepack validation - #1743

Closed
sergey-shandar wants to merge 1 commit into
mainfrom
claude/private-types-migration-zpoo70
Closed

Implement private type module separation and prepack validation#1743
sergey-shandar wants to merge 1 commit into
mainfrom
claude/private-types-migration-zpoo70

Conversation

@sergey-shandar

Copy link
Copy Markdown
Contributor

Summary

This change implements the private type module separation strategy to keep implementation-private types out of public declarations. Rather than waiting for TypeScript's @internal / stripInternal support for JSDoc typedefs, the repository now enforces that authored .mjs files contain no file-scope @typedef declarations, moving private types to optional private.ts modules that are deleted during packaging.

Key Changes

  • Repository-wide rule: No authored .mjs file may contain a file-scope JSDoc @typedef. Typedefs inside functions (for compile-time proofs) remain allowed. This prevents declaration emit from leaking implementation details as exported type aliases.

  • Private type placement strategy:

    • Types required by public declarations stay in types.ts (the public declaration closure)
    • Implementation-private types move to optional private.ts siblings
    • Proof-specific types live inside proof functions
    • Optional meta/module.f.mjs for metaprogramming constants
  • Packaging validation (fjs/ci/prepack.mjs):

    • Runs as the final prepack step after declaration emit
    • Deletes all generated private.d.ts files (never shipped)
    • Validates that remaining declarations have no semantic dependencies on private modules
    • Uses token-based specifier parsing to avoid false positives from JSDoc comments
  • First migration: fjs/djs/tokenizer now uses private.ts for _Token, _FlatToken, _TokenScanState, _StringDecodeState, and _DjsScanState, with the implementation updated to import these types via JSDoc @import.

  • Documentation updates:

    • AGENTS.md: Repository-wide prohibition on file-scope @typedef
    • fjs/AGENTS.md: Public declaration closure, optional private.ts, dependency ordering, and optional meta/ modules
    • fjs/fsc/README.md: Replaces leak-tolerance policy with the new private-type contract
    • fjs/todo/separate-private-types.md: Marked as WIP with progress tracking
    • Deleted todo/blocked/jsdoc-typedef-strip-internal.md: The @internal wait is superseded
  • Package.json: Updated prepack script to run node ./fjs/ci/prepack.mjs after declaration emit

Implementation Details

The prepack validation is semantic rather than textual: it reads static module specifiers as tokens using the existing specifiers utility, so retained JSDoc @import comments in declarations are correctly identified as comments and not mistaken for actual dependencies. This allows TypeScript to preserve source documentation while the validation ensures no shipped declaration actually depends on a private module.

The dependency direction within a module directory is: types.tsprivate.tsmodule.f.mjsproof.f.mjsmodule.mjsproof.mjs, guiding where types and verification belong without requiring every file to exist.

https://claude.ai/code/session_01BHDDUXrAnbCeGBuGgbuPuw

Establishes the rule, the documents, and the packaging step that
fjs/todo/separate-private-types.md needs, then migrates one module to prove
the shape end to end.

Rule: no authored .mjs anywhere in the repository carries a file-scope JSDoc
@typedef, since declaration emit turns one into an exported type alias. A
typedef inside a function is unaffected. Root AGENTS.md carries the
repository-wide rule; fjs/AGENTS.md gains a "Private types" section with the
public declaration closure, the optional private.ts, the optional
meta/module.f.mjs, and the intra-directory dependency order;
fjs/fsc/README.md replaces its `_`-leak-tolerance policy with the private-type
contract. todo/blocked/jsdoc-typedef-strip-internal.md is deleted rather than
narrowed - splitting private types out closes the leak, so the repository no
longer waits on @internal/stripInternal or keeps two conflicting strategies -
and its referrers now point at the migration.

Packaging: prepack ends with node ./fjs/ci/prepack.mjs, which deletes every
private.d.ts generated from an authored private.ts and then fails packaging if
a remaining declaration still imports a private module. The check is semantic,
not textual: it reads static module specifiers as tokens with `specifiers`
from fjs/website/browser-source.mjs, so a JSDoc @import comment TypeScript kept
in a declaration is read as a comment and no emitted text is rewritten.

First private.ts: fjs/djs/tokenizer. _Token, _FlatToken, _TokenScanState,
_StringDecodeState and _DjsScanState are reached only by module-private
constants, so they move out of the public surface entirely - the emitted
module.f.d.mts names none of them, and _StringDecodeState loses the spurious
`any |` arm the multi-line @typedef form used to emit.

Verified: npx tsc, the full proof suite (3477 pass), npm pack (no private
artifact in the tarball), and a clean TypeScript consumer of
fjs/djs/tokenizer installed from that tarball, with its negative control still
failing TS2322.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHDDUXrAnbCeGBuGgbuPuw
@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
functionalscript 0dc4169 Commit Preview URL

Branch Preview URL
Aug 27 2026, 06:35 PM

@o2alexanderfedin o2alexanderfedin 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.

Measured at 0dc41692f. Gates match main: npm test 3477/3477 exit 0, tsc --noEmit exit 0, npm run prepack exit 0 ("private type modules removed: 1; declarations checked: 375").

The tokenizer conversion is real, not nominal: main's fjs/djs/tokenizer/module.f.d.mts:54-70 emits _Token, _FlatToken, _TokenScanState, _StringDecodeState, _DjsScanState; at this head none of those _ exports remain in the emitted declaration. The reconciliation is honest too — fsc/README.md's section is rewritten rather than patched, todo/blocked/jsdoc-typedef-strip-internal.md is deleted outright, and separate-private-types.md checks off exactly what shipped and no more.

Two things before it leaves draft:

  1. The validator does not enforce the rule it is presented alongside. fjs/ci/prepack.mjs checks only that no remaining declaration statically imports a private.ts — it never looks for file-scope @typedef. I tested all three ways: adding /** @typedef {number} _LeakedTypedef */ at file scope in untouched fjs/edag/module.f.mjs gives npm run prepack exit 0 while fjs/edag/module.f.d.mts:35 literally emits export type _LeakedTypedef = number; leaking a private.ts type into a public signature gives exit 1 with the right message; unmodified code passes. So it is not a check that cannot fail — but "no file-scope @typedef in authored .mjs", which this PR adds to both AGENTS.md and fjs/AGENTS.md as the headline rule, has zero enforcement. Either widen the check or say plainly in fjs/ci/README.md that it guards only the private.ts path, so a reviewer does not over-trust "prepack validation".

  2. Changelog is owed and missing. This changes shipped .d.mts output (five exported _ types leave the packed tokenizer declaration) and gives prepack a new failure mode on npm pack/publish — both are the "affects behavior or the public API" case. There is no changelog/unreleased/1743.md and no Changelog: section in the body.

Worth recording since it is easy to miss: grep .github/workflows/ for prepack finds nothing, but ci.yml's node26 job runs npm pack, which fires the prepack lifecycle hook — so the check does run in CI, just not under that name.

@sergey-shandar
sergey-shandar deleted the claude/private-types-migration-zpoo70 branch August 27, 2026 20:50
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