Skip to content

TS => MJS: crypto/sha2 - #1472

Merged
sergey-shandar merged 7 commits into
mainfrom
claude/epic-fermi-3j7vxz
Aug 10, 2026
Merged

TS => MJS: crypto/sha2#1472
sergey-shandar merged 7 commits into
mainfrom
claude/epic-fermi-3j7vxz

Conversation

@sergey-shandar

Copy link
Copy Markdown
Contributor

Part of todo/migrate-typescript-to-mjs.md (stage 1).

fjs/crypto/sha2 was a dependency leaf: every relative import it makes (types/array, types/bigint, types/bit_vec, types/function/operator, types/list) is already .f.mjs, so it is eligible today, and it is the most depended-on remaining leaf — 16 authored TypeScript modules import it.

Changes

  • fjs/crypto/sha2/module.f.tsfjs/crypto/sha2/module.f.mjs, TypeScript syntax replaced by JSDoc.
  • Every importer updated to the .f.mjs specifier: crypto/hmac, crypto/pow, crypto/sign, cas, cas/cli, cas/evo, mcp, mcp/cas, mcp/evo, sul/id, plus the module's own proof.f.ts.
  • Prose references to crypto/sha2/module.f.ts in fjs/sul/todo/186.md and fjs/basen/cbase32/todo/178.md repointed at the new path and line numbers.

Type visibility

Public typedefs keep their names and emitted shapes: V8, V16, State, Base, Sha2. The three implementation-only aliases become _V3, _V4 and _BaseInit per the private JSDoc typedef convention — none of them was exported before, and none appears in a public declaration's expanded contract (Base and _BaseInit are separate types; _V3 / _V4 occur only inside base(...)).

Emitted declarations were diffed before and after: V8, V16, State, Base, Sha2, computeSync, base32, base64, sha224, sha256, sha384, sha512, sha512x224 and sha512x256 are unchanged, and the file gains only the three _-prefixed aliases that declaration emit cannot strip yet.

proof.f.ts stays TypeScript: it imports utf8 from fjs/text/module.f.ts, which is still authored TypeScript, so migrating it now would make a .f.mjs file depend on remaining .f.ts — explicitly disallowed by the migration plan. It follows once fjs/text migrates.

Verification

  • npx tsc — clean.
  • npm test — 2355 pass, 0 fail; 22 of them are the crypto/sha2 proof cases, which now run against the .f.mjs module.
  • npm run ci-update — no generated-config changes.
  • Declaration emit compared against the pre-change output as described above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQCvuSGZUjhuybbT1Fi2Sx


Generated by Claude Code

claude added 2 commits August 10, 2026 10:25
Migrate fjs/crypto/sha2/module.f.ts to JSDoc-typed module.f.mjs under the
stage-1 TypeScript-to-mjs migration, and update every importer.

Public typedefs (V8, V16, State, Base, Sha2) keep their names and emitted
shapes; the implementation-only V3, V4 and BaseInit become _V3, _V4 and
_BaseInit per the private JSDoc typedef convention. proof.f.ts stays
TypeScript for now because it still depends on fjs/text/module.f.ts.

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

cloudflare-workers-and-pages Bot commented Aug 10, 2026

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 c35a269 Commit Preview URL

Branch Preview URL
Aug 10 2026, 03:03 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.

Verified independently — clean. This is the widest-blast-radius module migrated so far (18 importer sites), so I checked the runtime and the public contract mechanically rather than by reading.

Runtime code is identical

I transpiled the base .f.ts to JS with tsc and diffed it against the new .f.mjs as a token stream (comments and imports normalized away). Result: zero non-cosmetic differences — the only non-punctuation blocks were three hex literals where tsc's emit lowercases digits (0x0eb72ddC81c52ca2n…dc81…); both sources carry the identical uppercase spelling, so that's my transpile step, not a change of yours. Everything else was TS-emit's added semicolons and single-parameter parens.

Public contract

Diffed the emitted declaration against a declaration emit at the base commit. V8, V16, State, Base, Sha2, base32, base64, sha224, sha256, sha384, sha512, sha512x224, sha512x256 are all byte-identical modulo comments. computeSync is type-identical but its declaration text did change — ({ append, init, end }: Sha2) => … became (sha2: Sha2) => …, since the @type names the parameter rather than restating the destructuring pattern. Parameter names in a function type don't affect assignability, so nothing downstream can see it; just noting it since the description says the declarations are unchanged.

_V3, _V4, _BaseInit were each non-exported types on the base, so the _ prefix is the right call per #1461. And the three new _-prefixed aliases are the only additions to the emitted file.

Everything else

  • All 18 importer sites moved, including the @example specifier inside crypto/hmac's module doc. No crypto/sha2/module.f.ts reference survives anywhere in the tree.
  • Both doc citations land. sul/todo/186.md:27module.f.mjs:239 opens exactly on the /** @type {(a: V8) => bigint} */ line of the quoted fromV8 block, and the quoted snippet matches the new JSDoc form verbatim. basen/cbase32/todo/178.md:78module.f.mjs:252 lands on end: hashLength => {, which is the padding/length-field code the sentence is about.
  • npm test — tsc clean, 2355 pass / 0 fail.

The annotations are load-bearing, which matters here because the variadic tuple parameters are the fiddly part of this file. Against a 0-error baseline, each of these produces an error: widening sigma to four parameters → TS2322: '(a, b, c, d) => …' is not assignable to '(__0: bigint, __1: bigint, __2: bigint) => …'; retyping sigma as ..._: _V4TS2554: Expected 4 arguments, but got 3; compress returning V16 and appendChunk folding over V8 instead of State both surface as TS2322 against (init: _BaseInit) => Base; and base annotated to return Sha2 likewise. So _V3/_V4/V16/V8 really are constraining the arities, not decorating them.

Approving.


One observation, not a blocker and not specific to this PR: declaration emit drops JSDoc comments attached to @typedef, so the published .d.mts loses the documentation on V8, V16, State and Sha2 — including Sha2's @example walkthrough, which is about the most useful doc string in the package. Your source keeps all of it (and folding the per-member docs into prose on the typedef reads well), and the same thing already happens in types/list, so this is established migration behavior rather than something you introduced. But sha2 is by far the doc-richest module migrated so far, so it's the first place the loss is really visible on the public surface — might be worth a line in todo/migrate-typescript-to-mjs.md next to the stripInternal caveat, since it's the same class of upstream gap.

Declaration emit drops the documentation written on a JSDoc @typedef, so a
migrated module loses its type documentation in the published .d.mts while
keeping it in the source. crypto/sha2 is the clearest case so far: V8, V16,
State and Sha2 were documented, and Sha2 carried the module's @example.

Blocks no migration group; recorded next to the stripInternal caveat with a
task to file the upstream issue.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQCvuSGZUjhuybbT1Fi2Sx

Copy link
Copy Markdown
Contributor Author

Thanks — both notes are right, and I've acted on the second one.

computeSync's declaration text. Correct, and my description overstated it. The parameter name moved from the destructuring pattern to sha2 because @type names a parameter rather than restating the pattern. Type-identical, invisible to assignability, but "byte-identical" was the wrong word for that one line.

Typedef documentation loss. I reproduced it: the old .d.ts carried /** 8-word SHA-2 state vector. */ above export type V8; the new .d.mts emits a bare export type V8 = Tuple<8, bigint>, and Sha2's @example is gone the same way. Documentation on export const survives, so a migrated module loses exactly its type documentation — invisible in-repo, visible only to a consumer of the published package.

Recorded in todo/migrate-typescript-to-mjs.md next to the stripInternal caveat (685cb5b), with crypto/sha2 as the worked example, a task to file the upstream issue, and an acceptance criterion so it isn't mistaken for a reason to keep a module in TypeScript or to stop documenting types.

I looked for an existing upstream issue and didn't find one that tracks this directly. #43534 is the adjacent misattachment bug — and it explains a quirk I hit while checking: in types/range/module.f.d.mts the bare /** @typedef {...} Range */ line does reappear, but attached to the following declaration rather than to the type. #61664 proposes dropping redundant type directives from declaration emit while keeping documentation, which presupposes the documentation is emitted. Neither is the issue; the todo says so rather than citing a near-miss.

The note is docs-only, so no CHANGELOG entry.


Generated by Claude Code

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

Delta since a9b20865 is the one doc commit recording the typedef documentation gap. No code touched, so the earlier verification still stands — re-approving on that basis.

Checked the new section's factual claims rather than taking them on trust:

  • Both upstream references are real and described correctly. #43534 is "Declaration-like jsdocs are incorrectly attached to the following statement" (closed, Fix Available), and #61664 is "Omit JSDoc type directives when emitting type declarations with documentation" (open, Suggestion). The framing is right too — #61664 assumes documentation is retained and only proposes dropping the redundant @type directive, so it genuinely doesn't cover this loss.
  • "Documentation on export const declarations is unaffected" is accurate. The emitted module.f.d.mts keeps the prose on base32, base64, sha224sha512x256, each with its @type directive carried through — which is exactly the shape #61664 is about.
  • The sha2 specifics check out: V8, V16, State and Sha2 were all documented types on the base, Sha2 carried the @example, all of it is present in the source and none of it in the emitted declaration.
  • ./blocked/jsdoc-typedef-strip-internal.md resolves.

Placing it under the _-prefix convention as a second symptom of the same upstream gap reads right, and I agree with the disposition — documentation fidelity, not type contract, so it blocks nothing.

One addition for whenever you file the upstream issue, since it widens the repro slightly: the module-level doc comment is dropped too. sha2's /** See https://www.rfc-editor.org/rfc/rfc6234 … @module */ header survives into the base module.f.d.ts but does not appear in module.f.d.mts — same header text, same position ahead of the imports, retained from .f.ts and lost from .f.mjs. Different attachment mechanism from the typedef case (in the .mjs the next thing is an @import tag rather than a declaration), but the same symptom and probably worth one sentence in the section so the record is complete.

The @module header was the leading comment of the first import statement.
Declaration emit drops runtime-only imports, so that statement did not
survive and took the header with it. A blank line detaches the header from
the import and restores it in module.f.d.mts, matching the old .f.ts emit.

Correct the migration note accordingly: this half is a source-formatting
requirement, not an upstream gap. The blank line predicts header retention
across all 16 migrated modules that carry an @module header.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQCvuSGZUjhuybbT1Fi2Sx

Copy link
Copy Markdown
Contributor Author

Good catch, and chasing it changed the answer: the module-header case turned out not to be an upstream gap. It's a source-formatting requirement, and it's fixed in a3d8cfc.

The discriminator is a blank line. Without one, the header is the leading comment of the first import statement — an @import tag is a comment, not a statement, so it doesn't separate them. Declaration emit rewrites the import list, dropping runtime-only imports and synthesizing import type for what the declarations actually reference. In sha2 the first statement was import { mask } from '../../types/bigint/module.f.mjs'; mask is implementation-only, so that statement didn't survive and took the header with it. Adding one blank line detaches the header, and it emits as the file's own leading comment.

That also explains why the loss looked intermittent. I checked every migrated .f.mjs carrying an @module header — the blank line predicts retention in all 16, no exceptions. text/utf16, asn.1, types/array and btree/find have the blank line and keep their header; crypto/sha2, types/list and common/monoid didn't and lost it. My first hypothesis was "the first import's specifier survives into the declarations", which held for 12 and failed on three — the blank line is the actual rule.

The fix on sha2 restores the header and changes nothing else; diff against the previous emit is exactly the five header lines added. So it's now at parity with what the old .f.ts emitted.

The todo section is corrected to keep the two cases apart:

  • typedef documentation — a real upstream gap, nothing to do in-source, task to file it upstream stands.
  • @module header — a formatting requirement, with the rule, the mechanism, and the blank-line fix written down.

types/list and common/monoid still lose their headers. I left them alone here — they're outside this PR's module, and §8.1 asks for one improvement with minimal changes — but they're recorded as a task so the one-line fix isn't lost.


Generated by Claude Code

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

Nice diagnosis — the blank line really is the whole mechanism, and I reproduced your survey. One module is missing from the list of casualties.

The rule holds, with zero exceptions

I emitted declarations for the whole tree and classified every .f.mjs carrying an @module header by whether a blank line separates that header from the first import statement (skipping over @import comment lines, per your point that they don't separate). Then checked whether the header actually reaches the emitted declaration:

separated header kept count
yes yes 21
no no 3

24 modules, 0 exceptions. The sha2 fix in this PR works — module.f.d.mts now opens with the rfc6234 header.

Worth flagging that I got this wrong on the first pass: I initially tested for a blank line immediately after the header and got three apparent counterexamples. All three dissolve on closer reading — types/btree/find has its blank line at line 8, after two @import comments and still ahead of the first import statement, and types/function/compare and types/btree/types have no import statement at all, so there is nothing for the header to attach to. Your framing (first import statement, @import tags don't count) is the one that predicts the data.

fjs/types/nullable is missing from the list

The prose and the checklist item both name fjs/types/list and fjs/common/monoid. There is a third:

// fjs/types/nullable/module.f.mjs
/**
 * Utilities for nullable (`null`/`undefined`) value handling.
 *
 * @module
 */
import { assert } from '../../asserts/module.f.mjs'   // <- header attaches here

Its module.f.d.mts contains no @module at all — the header is gone, same as the other two, and it wants the same one-line fix. Worth adding to both places so the checklist item is complete when someone works it.

Minor

  • The section says "all 16". I count 24 .f.mjs files with an @module header on this branch, 21 of them separated. Not important to the argument, but the number will read as a coverage claim to whoever revisits this.
  • I couldn't produce an instance of the case in "Modules whose first import happens to survive into the declarations keep their header either way" — every retained module here either has the separating blank line or has no import statement at all, and all three unseparated ones lose it. The sentence isn't contradicted by anything I found, just unexercised; the no-imports case is the one that actually shows up and might be the clearer thing to name.

npm test — tsc clean, 2355 pass / 0 fail. Approving; the nullable omission is a doc/checklist gap rather than anything that should hold up the migration.

The earlier count skipped modules with no import statement, which both
understated the survey (16 -> 24) and hid a third casualty,
fjs/types/nullable. Replace the unexercised claim about a surviving first
import with the case that actually accounts for the other retentions: a
module with no import statement has nothing for the header to attach to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQCvuSGZUjhuybbT1Fi2Sx

Copy link
Copy Markdown
Contributor Author

All three corrections confirmed and applied in 5017b35. My survey had a blind spot that produced every one of them.

The script skipped any module without an import statement (firstImp < 0 → continue). That silently dropped 8 modules, which both understated the count as 16 and — because the filter ran before the casualty check — hid types/nullable. Re-running without the skip reproduces your numbers exactly:

separated from first import statement header kept count
yes yes 13
no import statement at all yes 8
no no 3

24 modules, 0 exceptions. Casualties: types/list, types/nullable, common/monoid.

On the unexercised sentence — you were right to push on it, and it was worse than unexercised: it was a leftover from my discarded first hypothesis ("the first import's specifier survives into the declarations"), which I dropped when it failed on three modules. I rewrote the surrounding argument but left that explanatory sentence describing a mechanism I no longer believed. Zero modules fall into it. It's replaced with the no-imports case, which accounts for all 8 of the other retentions: no statement to attach to, so the header is already the file's own leading comment.

That also sharpens the point of the section — most migrated modules are safe by accident rather than by intent, which is exactly why the rule is worth writing down.

fjs/types/nullable is now in both the prose and the checklist item, so the follow-up is complete when someone picks it up. Still leaving the three fixes themselves out of this PR per §8.1.


Generated by Claude Code

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

Every number in the corrected table checks out. I re-ran the classification with the three-way split you now use and got an exact match:

category your count mine
separated from first import, header kept 13 13
no import statement at all, header kept 8 8
unseparated, header lost 3 3

24 total, nothing uncategorized. The eight no-import modules are asserts, types/btree/types, types/function/compare, types/function/operator, types/option, types/range, types/result, types/ts; the three casualties are types/list, types/nullable and common/monoid, matching the prose and the checklist item now that nullable is in both.

Splitting out "no import statement" as its own row is the right call — it's the category that actually occurs, and "safe by accident, not by intent" is the useful thing for the next migration to know.

npm test — tsc clean, 2355 pass / 0 fail.

Approving.

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Aug 10, 2026
Merging main brought fjs/types/btree/set and fjs/types/btree/remove onto the
branch. Both were migrated without the blank line, so the survey is now 26
modules with five casualties rather than 24 with three. The rule itself still
holds with no exceptions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQCvuSGZUjhuybbT1Fi2Sx

Copy link
Copy Markdown
Contributor Author

We resolved the main conflict at the same time — you got there first with 23f96d7, and our CHANGELOG resolutions were identical (both entries kept, #1472 above #1471). I discarded my duplicate merge and kept yours rather than force-pushing over it.

One thing the merge invalidated, which is why there's a commit on top (c35a269): merging main brought types/btree/set and types/btree/remove onto the branch, and both were migrated without the blank line. So the header survey in the todo went stale the moment the merge landed — 24 modules with three casualties became 26 with five.

category before merge after
separated, header kept 13 13
no import statement, header kept 8 8
unseparated, header lost 3 5

The rule itself still holds with no exceptions. The casualty list and the checklist item now read common/monoid, types/btree/remove, types/btree/set, types/list, types/nullable.

Worth noting the trend rather than just the number: two of the five casualties arrived after this rule was written down, from a PR in flight at the same time. Migrations will keep reintroducing it until something checks automatically — that's an argument for a lint rule over a checklist item, though I'd file that separately rather than grow this PR.

npx tsc clean and 2355 tests pass on the merged tree.


Generated by Claude Code

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 1ccab74 Aug 10, 2026
19 checks passed
@sergey-shandar
sergey-shandar deleted the claude/epic-fermi-3j7vxz branch August 10, 2026 15:14
sergey-shandar pushed a commit that referenced this pull request Aug 10, 2026
Add a blank line after the header comment so it attaches to the
module rather than being dropped by declaration emit (same issue
documented for PR #1472).
@o2alexanderfedin o2alexanderfedin mentioned this pull request Aug 10, 2026
2 tasks
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