Document module header and import conventions - #1502
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | bea596e | Commit Preview URL Branch Preview URL |
Aug 12 2026, 03:08 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: 9d80e0c163
ℹ️ 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.
Reviewed at 9d80e0c1630a20e6852e8dc154a856b7d02a41b9. Approving. Docs-only, and it writes
down the conventions I've been checking by hand on every migration PR for the past day, which
is the right place for them.
The blank-line rule's justification is correct — I tested it rather than agreeing with it.
The text claims the blank line after the module JSDoc block "keeps the header detached from the
first import/declaration and preserves it through declaration emit." On
fjs/media/json/rtti/module.f.mjs:
with the blank line @module in emitted .d.mts = 1
without the blank line @module in emitted .d.mts = 0
Deleting that one line silently drops the module documentation from the published declaration.
That's exactly the failure that cost this migration series 18 files' worth of docs earlier, and
it's the kind of thing that no test catches and review only catches if someone happens to look
— so making it an explicit rule rather than folklore is worth doing.
The @import-placement rule matches where the tree is already heading. #1500 moved 21
files to the consolidated form and merged, so this documents settled practice rather than
proposing something new.
One number worth having in view: 90 of 147 .f.mjs files still use the standalone
@import comment form the rule now forbids — including fjs/media/json/rtti/proof.f.mjs
from #1498, merged only hours ago. That's not an objection; an incremental convention landing
ahead of the cleanup is normal, and todo/migrate-typescript-to-mjs.md gains a tracked
checklist item for it in this same PR, which is the right way to handle it. I mention it only
so the size of the tail is known — roughly 61% of migrated modules, and new files are still
being written in the old style, which argues for doing the sweep sooner rather than later.
The [§4](#4-documentation) cross-reference resolves, and broken relative markdown links are
144 — identical to main, none added.
#1502 documents it in AGENTS.md §4: module-level `@import` tags belong in the same JSDoc block as `@module`, not in separate comments, and a blank line separates that block from the first import. These modules predate the rule. Moves 6 `@import` tags into their headers across 5 files — two of them were interleaved between runtime imports — and adds the required blank line to the two headers that carry no tags. Generated output is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FyMg2KPgfJeQ5FXGXjyFmd
2 participants
Summary
import type, migrated.mjsruntime imports, then remaining.tsruntime imports@importtags in the same leading JSDoc block as@module, followed by one blank line and runtime.mjsimports@importexamples inAGENTS.mdand the migration TODO to match the conventionImpact
Documentation/convention change only. No runtime code or public package API changes.
Validation
main; onlyAGENTS.mdandtodo/migrate-typescript-to-mjs.mdare changed