Skip to content

TypeScript guidelines: Add Migrating JavaScript to TypeScript section on runtime neutrality - #179

Open
MajorLift wants to merge 3 commits into
mainfrom
jongsun/typescript/migration-runtime-neutrality
Open

TypeScript guidelines: Add Migrating JavaScript to TypeScript section on runtime neutrality#179
MajorLift wants to merge 3 commits into
mainfrom
jongsun/typescript/migration-runtime-neutrality

Conversation

@MajorLift

@MajorLift MajorLift commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Adds a Migrating JavaScript to TypeScript section with one guideline — a typing change should not change runtime behavior — plus a subsection on weighing a conversion's risk by observability.

Kept separate from #178 on purpose: that PR is about deriving types from authoritative sources, and this is a different rule. Bundling them would mean a reviewer of either has to litigate both.

Why it needs saying

A conversion PR is a type-level edit. The reason to state that explicitly is asymmetric scrutiny: a PR described as mechanical, rename-only, or no behavior change gets a lighter review than a behavior change would — so a runtime edit smuggled in as typing work receives the least attention of anything in the diff.

What it covers

  • Four patterns that read as typing work but are not:
    • a literal replaced by a runtime lookup (check the declaration first — many platform-API parameters are template-literal string types that accept the literal already);
    • a default parameter or fallback deleted, usually the | undefined a restated type just dropped;
    • a call made optional (obj.m()obj.m?.()), which converts a throw into a silent no-op;
    • a local widened to keep an existing check compiling, when the input type is the thing that's wrong.
  • A reachability requirement. Each hit gets a verdict either way — "traced, and inert on today's call paths" is a useful review outcome; "this might be a bug" is not.
  • Risk weighted by observability. A change inside catch { captureException(e); return; } degrades a feature with nothing going red, so an unlikely failure there can outrank a likely one in a loud path.

The example

A real conversion that replaced ['OFFSCREEN_DOCUMENT'] and ['IFRAME_SCRIPTING'] with runtime enum lookups and added a cast — against declarations that are template-literal string types and a function that already returns the cast-to type. The types had asked for none of it, and the change traded two constants for an assumption about the host environment inside a swallowed captureException path.

Worth noting for reviewers: the 🚫 version is not broken — the enum objects do exist at runtime. The point is the trade, not a defect, and the example says so rather than overclaiming.

Related


Note

Low Risk
Documentation-only change to TypeScript guidelines; no application code, build, or runtime behavior is modified.

Overview
Adds a Migrating JavaScript to TypeScript section to docs/typescript.md, after the selector guidance.

The core rule is that a typing change must not change runtime behavior, with emphasis on why conversion PRs get lighter review and runtime edits can slip through as “mechanical” typing work.

It lists anti-patterns that look like typing fixes (literal → runtime enum lookup, removed defaults/fallbacks, optional chaining that silences throws, widening locals to preserve dead checks, swapping thrown errors and breaking error-tracker grouping) and requires reviewers to trace reachability with a clear verdict.

A subsection Weigh a conversion's risk by observability argues swallowed catch + captureException paths can be worse than loud hot paths, illustrated with a Chrome offscreen API example contrasting unnecessary enum lookups/casts vs keeping string literals.

Reviewed by Cursor Bugbot for commit 99e7a68. Bugbot is set up for automated code reviews on this repo. Configure here.

A conversion PR is a type-level edit: it may add annotations and replace
JSDoc, but it may not change what the program does. The reason this needs
stating is asymmetric scrutiny — a PR described as mechanical or
"rename-only" is reviewed more lightly than a behavior change, so a runtime
edit smuggled in as typing work gets the least attention in the diff.
Names the four patterns that read as typing work but are not: a literal
replaced by a runtime enum lookup, a deleted default parameter, a call made
optional (turning a throw into a silent no-op), and a widened local propping
up an existing check.
Adds a subsection on weighing risk by observability: a change inside a
swallowed `catch { captureException }` degrades a feature with nothing going
red, so an unlikely failure there can outrank a likely one in a loud path.
Example is a real conversion that swapped two string literals for runtime
enum lookups and added a cast, against declarations that are template-literal
string types and a function already returning the cast-to type — i.e. the
types had asked for none of it.
@MajorLift
MajorLift force-pushed the jongsun/typescript/migration-runtime-neutrality branch from 745681d to 6cc465b Compare July 30, 2026 13:21
"Usually the conversion working around a type error" and "usually the
`| undefined` the new type just dropped" both assert a distribution from a
single observed case. Neither needs to be a frequency claim to do its job:
the first works as an instruction to check, the second as the first candidate
to rule out.
Guarding a possibly-undefined value and throwing a hand-written `Error` swaps
the native failure for a synthetic one. Both still reach the error tracker, but
type, message, and originating frame are the fingerprint it groups on, so the
substitute opens a fresh issue with no history and stops matching any ignore,
sampling, alert, or routing rule written against the old shape.

Distinct from the optional-call pattern already listed, which loses the error
entirely rather than replacing it.
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.

1 participant