First module.f.mjs file. - #1452
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 612330f | Commit Preview URL Branch Preview URL |
Aug 07 2026, 09:09 PM |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e89cc9e6da
ℹ️ 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".
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. The risky part of this change is that a TS→JSDoc translation can silently weaken a type contract while the suite stays green, so I mutated rather than read.
Proof discovery is intact
npm test on this branch and on main, same machine:
main: Number of tests: pass: 2354, fail: 0, total: 2354
second: Number of tests: pass: 2354, fail: 0, total: 2354
No proofs were lost to the rename — fjs/dev/module.f.ts already recognises .f.mjs, and proof.f.ts next to the renamed module still resolves it.
All three type contracts survive, each confirmed by mutation
asserts vnarrowing is live. A probe that returns a narrowedstring | nullcompiles clean; stripasserts vfrom the JSDoc@typeand it goes red withTS2322: Type 'string | null' is not assignable to type 'string'. The assertion signature really is carried by the@typeannotation, not quietly dropped.assertNotNullishstill returnsT. The same mutation turns its own body red ('T | null | undefined' is not assignable to 'T').Assert<T>is still exported and still constrained.Assert<true>compiles;Assert<false>errorsTS2344: Type 'false' does not satisfy the constraint 'true'. Worth confirming explicitly, since a JSDoc@typedefhas noexportkeyword and it would be easy to assume the export was lost.
The other ~100 files are mechanical module.f.ts → module.f.mjs import rewrites.
Nit, non-blocking
assertNotNullish = (a, msg = undefined) adds a default the original msg?: unknown didn't have. It's a no-op — msg is already undefined when omitted — so it just adds a token. Fine either way.
One note: this is stacked on #1451, so it inherits the packaging point I raised there.
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Re-verified at 1c6039da after the rebase onto the updated first. Still good.
- Nit addressed —
assertNotNullishis back to(a, msg)with no redundant default. - Proof count unchanged:
npm test→pass: 2354, fail: 0, total: 2354, identical tomain. - The three type contracts still hold, re-confirmed by mutation on this head, not carried over from the last review:
- probe relying on
asserts vnarrowing compiles clean; stripasserts vfrom the@typeand it goes red (TS2322: Type 'string | null' is not assignable to type 'string'). - the same mutation reddens
assertNotNullish's own body, so itsTreturn is still real. Assert<true>compiles,Assert<false>errorsTS2344.
- probe relying on
The packaging point I raised on #1451 is resolved there (the rule was corrected rather than the code — see my comment on that PR), so nothing carries over to this one.
Summary
Migrate
fjs/asserts/module.f.tsto authoredmodule.f.mjswith equivalent JSDoc types and update its importers.Dependency
This PR is stacked on #1451 (
first) and must land after it. #1451 enablesallowJs/checkJs, adds.mjs/.d.mtspackage support, and uses the verified two-passprepackrequired before authored.mjsenters the repository.