TypeScript guidelines: Add Migrating JavaScript to TypeScript section on runtime neutrality - #179
Open
MajorLift wants to merge 3 commits into
Open
TypeScript guidelines: Add Migrating JavaScript to TypeScript section on runtime neutrality#179MajorLift wants to merge 3 commits into
Migrating JavaScript to TypeScript section on runtime neutrality#179MajorLift wants to merge 3 commits into
Conversation
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
force-pushed
the
jongsun/typescript/migration-runtime-neutrality
branch
from
July 30, 2026 13:21
745681d to
6cc465b
Compare
"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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
Migrating JavaScript to TypeScriptsection 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
| undefineda restated type just dropped;obj.m()→obj.m?.()), which converts a throw into a silent no-op;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 swallowedcaptureExceptionpath.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
Derive types from authoritative sourcesguideline #178 —Derive types from authoritative sources, the other half of reviewing a migration PR. The deleted-default-parameter bullet links into it.MetaMask/skills—typescript-type-proof— the review-side procedure that makes these checkable.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+captureExceptionpaths 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.