Plan TypeScript to mjs migration - #1449
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | b093a36 | Commit Preview URL Branch Preview URL |
Aug 07 2026, 08:46 PM |
|
@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: 30b60026c0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 948a58883c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32d05600af
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d31900e33f
ℹ️ 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
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. Documentation and planning only, CI green. Rather than take the technical claims on faith, I checked the load-bearing one — the JSDoc variance support used to justify removing the Effects variance blocker — against the pinned TypeScript 7.0.2.
The variance claim holds, on all three specifics
@template out Ton a@typedefis genuinely enforced, not merely parsed. A deliberately wrong annotation (out Ton a structurally contravariant alias) produces the identical diagnostic in.mjsas the.tscontrol:TS2636: Type 'Sink<sub-T>' is not assignable to type 'Sink<super-T>' as implied by variance annotation. This was the claim most worth testing, since "TypeScript accepts the syntax" and "TypeScript checks it" are very different guarantees.- The constrained form
@template {Operation} out Ois accepted, as documented. - Variance on an ordinary function's
@templateis rejected:TS1274: 'out' modifier can only appear on a type parameter of a class, interface or type alias— exactly whatAGENTS.mdnow says.
I also wrote a faithful JSDoc translation of the real Operation / Pr / Cont / Do / Effect from fjs/effects/module.f.ts. It type-checks clean and still permits the widening the annotation exists for (Effect<ReadOp> → Effect<ReadOp | WriteOp>). So the Cont<out O, T> / Do<out O, T> contract really is representable in JSDoc and dropping the blocker is justified.
One observation, not blocking
out isn't currently load-bearing even in the TypeScript source: removing it from both Cont and Do on main leaves npx tsc clean. That matches the comment at fjs/effects/module.f.ts:149, which presents it as an asserted covariance TypeScript cannot derive through Pr — so it's a deliberate forward-looking annotation and keeping it through the migration is right. Just worth knowing that if the translation ever drops an out by accident, the type-checker will not be the thing that catches it. The plan's "public assignability is not weakened" checklist item is therefore a manual review step, not a tsc step.
Cross-PR note
The packaging prerequisite this plan retains — "Non-package .mjs files must remain excluded from the packed archive" — is contradicted by #1451 as it currently stands, which packs fjs/types/bigint/benchmark.mjs. Details in my comment there. The plan is right; the implementation needs a one-line fix.
The two-stage split (drop TypeScript into .f.mjs first, treat .f.js as the compiler-compatibility marker only in stage 2) reads as the correct decomposition, and keeping the .f.mjs runtime/coverage fixture as a hard prerequisite before the first real source conversion is the right ordering.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fb6df2182
ℹ️ 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".
| | `.f.mjs` | Authored FunctionalScript-intent ESM JavaScript with JSDoc types. It may use FunctionalScript features the current parser/compiler does not support yet. | | ||
| | `.f.js` | During stage 1, generated JavaScript emitted from `.f.ts` and never authored. After stage 1 and authored-`.f.js` package support are complete, authored FunctionalScript that the current parser/compiler must accept. | |
There was a problem hiding this comment.
This new authoritative meaning still conflicts with todo/lang/README.md:20-25 and nanvm-lib/todo/mvp-roadmap.md:293-297,344-348, which continue to say that .f.mjs marks compiler-supported modules and that .f.ts moves only as parser support grows. Fresh evidence beyond the previously addressed todo/plan/roadmap.md is that these two repository-wide language/MVP documents still prescribe the old migration, so contributors following them will incorrectly gate Stage 1 on compiler coverage; update them to the two-stage contract atomically.
AGENTS.md reference: AGENTS.md:L322-L329
Useful? React with 👍 / 👎.
| Before stage 2 renames any repository source, complete the focused | ||
| [`f-js-package-support.md`](../fjs/ci/todo/f-js-package-support.md) prerequisite. | ||
| A standalone authored `.f.js` must be directly included in TypeScript checking, | ||
| receive a generated `.d.ts`, survive cleanup, be included in the packed NPM |
There was a problem hiding this comment.
The referenced f-js-package-support.md:44-48,80-83 explicitly says Stage 2 runs from clean CI and requires no generated-output cleanup or repeated-pack safety, but this integration plan newly requires authored .f.js to “survive cleanup” and repeats “cleanup safety” at line 111. Fresh evidence after the earlier cleanup rebuttal is this direct contradiction inside the revised Stage-2 design; an implementer following this checklist could reintroduce the rejected cleanup work, so align the prerequisite with the focused issue.
AGENTS.md reference: AGENTS.md:L322-L329
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2 participants
Summary
Plan the repository migration in two separate stages:
.ts/.f.tsgradually to.mjs/.f.mjswith JSDoc, independently of FunctionalScript parser support;.f.jspackage/tooling support exists, migrate compiler-supported.f.mjsmodules to.f.js.Stage 1 proceeds dependency-first: migrate files that do not depend on authored TypeScript, then their callers. Before the first real repository
.f.ts->.f.mjsconversion, complete authored.mjspackage support and then the.f.mjsruntime test/coverage fixture task that is blocked by it. The fixture proves an actualmodule.f.mjsworks with normal proof execution and Node/Deno coverage before production source starts moving.Proof migration is part of Stage 1 and is compiler-independent too. A migrated
module.f.mjsmay temporarily keepproof.f.ts, whileproof.f.mjsis allowed as soon as the proof is valid JavaScript/JSDoc and its authored FunctionalScript dependencies are already.f.mjs. Current FunctionalScript compiler support is not a gate; Stage 1 still ends with no authored.ts/.f.ts, including proofs.TypeScript-only type syntax must be translated without weakening the public type contract. TypeScript 7 supports variance annotations in JSDoc type aliases, so
out O extends Operationbecomes@template {Operation} out Oon the corresponding@typedef.AGENTS.mddocuments named types, generic constraints, andin/outvariance.Known TypeScript-to-JSDoc hard cases do not block unrelated Stage-1 work. Record them explicitly and postpone only the affected migration group until a focused design exists. The first known case is
fjs/types/phantom/module.f.ts, whose type-onlydeclare const ...: unique symbolcontract cannot be translated mechanically without deciding its runtime/declaration representation.After the last authored
.ts/.f.tsfile is removed, simplifyprepackto declaration-only emission, remove the TypeScript-to-JavaScript emit path, remove obsolete generated.jsfrom the transition working tree, and remove the blanket**/*.jsignore so.jsbecomes authorable and trackable again.Package-build invariant
Package and publish jobs run in CI from a clean checkout; normal development should type-check and test without generating package artifacts. Stage-1 packaging therefore does not need generated-output cleanup or legacy-output tracking across
.ts->.mjsrenames.Keep the two TypeScript emission passes inline in the NPM lifecycle instead of exposing separately callable
emit:*scripts while TypeScript source remains:The declaration pass runs first for
.ts+.mjs. With the generated declarations present, the second TypeScript invocation resolves.d.mtsfor authored.mjsmodules and emits the remaining TypeScript runtime JavaScript without overwriting authored.mjs.This exact setup is already validated by PR #1451:
allowJs/checkJsare enabled,benchmark.mjsremains authored source, and the Node 26 CI job completesnpm packsuccessfully with the same two-passprepack.benchmark.mjsmay remain in the packed archive during the transition. It exposes no documented public API, so special-casing it out of the package is unnecessary; removing it later is independent cleanup.As soon as no authored
.ts/.f.tsremains, remove the second pass:Review follow-up
todo/migrate-typescript-to-mjs.mdand used the repository-standardProblem/Proposalstructure.fjs/fsc/README.mdextension contract and package roadmap atomically:.f.mjsis stage-1 FunctionalScript-intent JavaScript, not a compiler-compatibility marker..mjspackage support as the stage-1 prerequisite.fjs/emergent_testing/todo/f-mjs-test-and-coverage.mdas the second prerequisite before the first real repository.f.ts->.f.mjsconversion.module.f.mjs+proof.f.tsfixture, avoiding a circular dependency on migrating assertion helpers merely to prove the first Stage-1 runtime/coverage layout.AGENTS.mdandCONTRIBUTING.mdsoproof.f.mjsis allowed during Stage 1 based on JavaScript/JSDoc and dependency readiness, not current compiler support.todo/plan/roadmap.mdto the two-stage contract: Stage 1 removes TypeScript into.f.mjs; Stage 2 uses authored.f.jsas the compiler-compatibility marker.todo/lang/README.mdandnanvm-lib/todo/mvp-roadmap.mdto the same two-stage extension contract, removing the remaining language/MVP guidance that gated.f.ts -> .f.mjson parser support.f-js-package-support.mdstage-2 prerequisite covering direct.f.jstype checking, declaration emission, NPM packing, and clean-consumer runtime/type tests.todo/fjs-nanvm-integration.mdso repository compiler-compatibility migration is blocked by both stage 1 and authored.f.jspackage support before any.f.mjs->.f.jsrename..f.jsvalidation runs from a clean CI checkout and requires checking, declarations, packing, and clean-consumer tests, not cleanup safety.prepack; no user-facingemit:*scripts are needed..ts/.f.tsfiles are gone,prepackbecomes declaration-only..mjsfiles as sources. #1451 instead of adding an unnecessary second TypeScript configuration.AGENTS.md, including constrained generics andin/outvariance on@typedeftype parameters.Cont<out O, T>/Do<out O, T>contract can be represented directly in JSDoc..mjsfiles from NPM;benchmark.mjsmay stay packed until it is removed later.Phantom'sunique symbol, defer the concrete redesign, and block only the affected migration group.Validation
Documentation-only change. The exact mixed
.ts+.mjstwo-pass package command is exercised by PR #1451, whose Node 26 CI job completesnpm packsuccessfully. The.f.mjsruntime proof/coverage fixture remains an explicit prerequisite to implement before the first real repository source conversion.