Migrate fjs/media/json/proof.f.ts to .f.mjs - #1504
Conversation
Two nested `as unknown as null` double-casts become nested inline `@type` casts. Doc reference updated in fjs/media/json/todo/stringify-sorted-canonical.md.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 3f7ceb2 | Commit Preview URL Branch Preview URL |
Aug 12 2026, 08:38 PM |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. Verified at head 4623270e against origin/main (241043b2).
What I ran
npx tsc --noEmit— exit 0.npm run prepackfrom a freshly cleaned tree — exit 0 (both passes). Also exit 0 onorigin/main.npm test— 2495 pass / 0 fail / 2495 total, byte-identical toorigin/main(2495/0/2495). Both trees weregit cleaned before testing, so no emitted.f.jscontamination.node bin/linkcheck.mjsin both trees — the broken-link sets are identical, not merely equal in count. Thetodo/stringify-sorted-canonical.mdreference was updated toproof.f.mjsin the same PR, so the rename strands nothing.
Public surface
prepack in both trees, then extract.mjs (exported type aliases) and consts.mjs (exported const signatures): both diffs are empty. I also did a file-aware diff of every export type / interface in every emitted .d.mts / .d.ts under fjs/ — 574 declarations on each side, identical including their file paths, so nothing is being masked by same-named types in different modules.
This PR adds no exported type at all — the module's only export is proof, and there is no @typedef in the file. So the unprefixed-export type problem that recurred through #1503 does not arise here.
Behavioural equivalence
Tokenized main's compiled fjs/media/json/proof.f.js against this PR's authored fjs/media/json/proof.f.mjs with acorn (acorn.tokenizer, values compared for identifiers/literals, ; ignored since tsc emits ASI semicolons the source doesn't have). The only difference is the extra parentheses the two JSDoc casts require: 606 vs 598 tokens, exactly the 8 parens from (/** @type {unknown} */ ( … )) × 2 sites. Substituting /** @type {null} */ (/** @type {unknown} */ (42)) → 42 and the [1, 2] site likewise gives token-identical output, 598 vs 598. I negative-controlled the comparator in both directions with a one-token mutation (a string literal on the PR side, an identifier on the compiled side); each produced exactly one extra differing position, so the comparator is not vacuously passing.
The casts still check
as unknown as null → /** @type {null} */ (/** @type {unknown} */ (…)) is a faithful translation, and the casts are honoured rather than inert. Three mutation probes from a cleaned tree (so no emitted .d.mts intercepted resolution), each verified to have actually landed:
- dropping the inner
unknowncast on the42site →TS2352: Conversion of type 'number' to type 'null' may be a mistake… - dropping the inner cast on the
[1, 2]site →TS2352: Conversion of type 'number[]' to type 'null'… - restoring the file →
tsc --noEmitexit 0 again.
Since the original was already a double cast, no checking is lost relative to main — this is not the "cast replacing a plain annotation on an object literal" case.
AGENTS.md §2 / §3.2 precondition for a proof.f.mjs
Verified rather than assumed. I walked the transitive relative runtime import graph from fjs/media/json/proof.f.mjs — 31 files — and there is not a single relative .ts runtime edge anywhere in it. The four direct imports (./module.f.mjs, ../../types/object/module.f.mjs, ../../types/function/module.f.mjs, ../../asserts/module.f.mjs) are all already migrated, so the module.f.mjs + proof.f.mjs row applies.
Conventions
- §8.3 CHANGELOG: entry is at the top of
## Unreleased, links only/pull/1504, ~230 characters, and no released section was touched. §8.4**BREAKING CHANGES:**prefix is correct for the.f.ts→.f.mjsspecifier change, matching the rest of this series. - §4 module header: this file has none, but neither did
proof.f.tsonmain, and 81 of the 105 existingproof.f.mjsfiles have none either — the header is an implementation-module convention, so this is not a gap introduced here. - §3.3
assert/assertEq: thesetPropertyandstringifygroups still use hand-writtenif (…) { throw x }. That is pre-existing text carried across verbatim (24 of 105 already-migrated proofs are in the same state), and converting it would make this PR more than a migration, so I am not flagging it — noting it only in case you want it swept separately.
Not verified
npm run cov is unusable in this environment — it reports a vacuous 100.00 over 0 tests on main too (Node v23), so I have no coverage number to report either way. Rust gates were not run; nanvm-lib/ is untouched.
No findings.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18299d04c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Per AGENTS.md's JSDoc convention: module-level @import tags go in a leading block before runtime imports, not interleaved as a standalone comment.
@module belongs only to module.* entry-point files, not proof.* or other files. A JavaScript file's @import tags always group into one leading JSDoc block -- sharing it with @module where present, standing alone otherwise -- never scattered between individual import statements. Prompted by Codex flagging a proof file for lacking a @module block it was never supposed to have.
|
The Generated by Claude Code |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
3 participants
Summary
Continues the stage-1 TypeScript-to-
.mjsmigration (following #1503) with the next leaf module:fjs/media/json/proof.f.ts→fjs/media/json/proof.f.mjs— two nestedas unknown as nulldouble-casts become nested inline@typecasts (/** @type {null} */ (/** @type {unknown} */ (...)))fjs/media/json/todo/stringify-sorted-canonical.md## UnreleasedTest plan
npm run prepack— clean, no type errorsnpm test— 3004/3004 tests passGenerated by Claude Code