From 6cc465bd5ed463fdfbf31b17241bf87d448c7253 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 30 Jul 2026 09:20:53 -0400 Subject: [PATCH 1/3] =?UTF-8?q?Add=20`Migrating=20JavaScript=20to=20TypeSc?= =?UTF-8?q?ript`=20section=20on=20runtime=20neutrality=20A=20conversion=20?= =?UTF-8?q?PR=20is=20a=20type-level=20edit:=20it=20may=20add=20annotations?= =?UTF-8?q?=20and=20replace=20JSDoc,=20but=20it=20may=20not=20change=20wha?= =?UTF-8?q?t=20the=20program=20does.=20The=20reason=20this=20needs=20stati?= =?UTF-8?q?ng=20is=20asymmetric=20scrutiny=20=E2=80=94=20a=20PR=20describe?= =?UTF-8?q?d=20as=20mechanical=20or=20"rename-only"=20is=20reviewed=20more?= =?UTF-8?q?=20lightly=20than=20a=20behavior=20change,=20so=20a=20runtime?= =?UTF-8?q?=20edit=20smuggled=20in=20as=20typing=20work=20gets=20the=20lea?= =?UTF-8?q?st=20attention=20in=20the=20diff.=20Names=20the=20four=20patter?= =?UTF-8?q?ns=20that=20read=20as=20typing=20work=20but=20are=20not:=20a=20?= =?UTF-8?q?literal=20replaced=20by=20a=20runtime=20enum=20lookup,=20a=20de?= =?UTF-8?q?leted=20default=20parameter,=20a=20call=20made=20optional=20(tu?= =?UTF-8?q?rning=20a=20throw=20into=20a=20silent=20no-op),=20and=20a=20wid?= =?UTF-8?q?ened=20local=20propping=20up=20an=20existing=20check.=20Adds=20?= =?UTF-8?q?a=20subsection=20on=20weighing=20risk=20by=20observability:=20a?= =?UTF-8?q?=20change=20inside=20a=20swallowed=20`catch=20{=20captureExcept?= =?UTF-8?q?ion=20}`=20degrades=20a=20feature=20with=20nothing=20going=20re?= =?UTF-8?q?d,=20so=20an=20unlikely=20failure=20there=20can=20outrank=20a?= =?UTF-8?q?=20likely=20one=20in=20a=20loud=20path.=20Example=20is=20a=20re?= =?UTF-8?q?al=20conversion=20that=20swapped=20two=20string=20literals=20fo?= =?UTF-8?q?r=20runtime=20enum=20lookups=20and=20added=20a=20cast,=20agains?= =?UTF-8?q?t=20declarations=20that=20are=20template-literal=20string=20typ?= =?UTF-8?q?es=20and=20a=20function=20already=20returning=20the=20cast-to?= =?UTF-8?q?=20type=20=E2=80=94=20i.e.=20the=20types=20had=20asked=20for=20?= =?UTF-8?q?none=20of=20it.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/typescript.md | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/docs/typescript.md b/docs/typescript.md index 8435cae..46f317d 100644 --- a/docs/typescript.md +++ b/docs/typescript.md @@ -1159,3 +1159,70 @@ Enabling composed selectors to accept different, even disjoint state types resol > Following this guideline does not affect selector memoization. When background state updates are dispatched to the Redux store, the Redux state object is shallow copied. This does not mutate the references of nested composite data structures, which makes cache invalidation a non-concern. > > The only exception to this is an idempotent selector that returns the entire Redux state (e.g. `(state: RootState) => state`). This pattern should be avoided if possible, as it will cause a performance hit to any downstream selector. + +## Migrating JavaScript to TypeScript + +### A typing change should not change runtime behavior + +Converting a file to TypeScript is a type-level edit. It may add annotations, rename the file, and replace JSDoc — it may not change what the program does. + +When a conversion diff also changes a runtime value, that change is usually the conversion working _around_ a type error rather than fixing one. This matters more than the size of the change suggests: a PR described as mechanical, rename-only, or "no behavior change" is reviewed more lightly than a behavior change would be, so a runtime edit smuggled in as typing work gets the least scrutiny of anything in the diff. + +Four patterns to look for, all of which read as typing work: + +- **A literal replaced by a runtime lookup.** `['IFRAME_SCRIPTING']` becoming `[SomeApi.Reason.IFRAME_SCRIPTING]` adds a dependency on that object existing at runtime. Read the declaration before "fixing" the type: many platform-API parameters are template-literal string types (`` `${SomeEnum}`[] ``), which accept the plain literal already, so the swap buys nothing and costs a runtime assumption. +- **A default parameter or fallback deleted.** `function f(x = {})` becoming `function f(x: T)` removes a guard. Ask what the guard was _for_ — usually the `| undefined` the new type just dropped (see [Derive types from authoritative sources instead of re-declaring them](#derive-types-from-authoritative-sources-instead-of-re-declaring-them)). +- **A call made optional.** `obj.method()` becoming `obj.method?.()` to satisfy a hand-written `| undefined` converts a **throw into a silent no-op**. The loud failure was load-bearing; the same broken state now produces no signal at all. +- **A local widened to keep a check alive.** Annotating `let name: string | undefined` on a value the authoritative type calls `string`, so that an existing `=== undefined` branch still compiles. If the runtime check is genuinely needed, the _input_ type is wrong — widen that deliberately instead of widening downstream to match. + +For each one, establish whether it is reachable and say so either way. "Traced, and inert on today's call paths" is a useful review outcome; "this might be a bug" is not. + +#### Weigh a conversion's risk by observability, not just likelihood + +Ask where a newly introduced failure would _surface_. A change inside `try { … } catch (e) { captureException(e); return; }` degrades a feature without crashing: nothing goes red, no test fails, and the only signal is an error-tracker entry nobody is watching. The same edit in a hot path is caught in minutes. + +So an unlikely failure in a swallowed path can outrank a likely one in a loud path — the cost of being wrong includes how long it takes to find out. + +**Example ([🔗 permalink](#example-63f43d96-a73e-4259-b4bb-1c4833368bf5)):** + +🚫 A conversion replaces two string literals with runtime enum lookups, and adds a cast, to satisfy types that never required any of it: + +```typescript +const contexts = (await chrome.runtime.getContexts({ + contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], +})) as chrome.runtime.ExtensionContext[]; + +await chrome.offscreen.createDocument({ + url: './offscreen.html', + reasons: [chrome.offscreen.Reason.IFRAME_SCRIPTING], + justification: '…', +}); +``` + +The declarations these lines are checked against are template-literal _string_ types, and `getContexts` already returns the cast-to type: + +```typescript +interface ContextFilter { + contextTypes?: `${ContextType}`[] | undefined; +} +interface CreateParameters { + reasons: `${Reason}`[]; +} +function getContexts(filter: ContextFilter): Promise; +``` + +✅ The original literals type-check unchanged, and the cast is redundant: + +```typescript +const contexts = await chrome.runtime.getContexts({ + contextTypes: ['OFFSCREEN_DOCUMENT'], +}); + +await chrome.offscreen.createDocument({ + url: './offscreen.html', + reasons: ['IFRAME_SCRIPTING'], + justification: '…', +}); +``` + +The 🚫 version is not wrong today — the enum objects do exist at runtime. But it converts two constants into an assumption about the host environment, in a function whose failure path is a swallowed `captureException`, in exchange for nothing the compiler asked for. From eac6dcbaf938c4097db82ac67c9c2d3d00c7e692 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 30 Jul 2026 09:52:02 -0400 Subject: [PATCH 2/3] Replace unsupported frequency claims with a check and a candidate "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. --- docs/typescript.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/typescript.md b/docs/typescript.md index 46f317d..6331764 100644 --- a/docs/typescript.md +++ b/docs/typescript.md @@ -1166,12 +1166,12 @@ Enabling composed selectors to accept different, even disjoint state types resol Converting a file to TypeScript is a type-level edit. It may add annotations, rename the file, and replace JSDoc — it may not change what the program does. -When a conversion diff also changes a runtime value, that change is usually the conversion working _around_ a type error rather than fixing one. This matters more than the size of the change suggests: a PR described as mechanical, rename-only, or "no behavior change" is reviewed more lightly than a behavior change would be, so a runtime edit smuggled in as typing work gets the least scrutiny of anything in the diff. +When a conversion diff also changes a runtime value, check whether the change is the conversion working _around_ a type error rather than fixing one. This matters more than the size of the change suggests: a PR described as mechanical, rename-only, or "no behavior change" is reviewed more lightly than a behavior change would be, so a runtime edit smuggled in as typing work gets the least scrutiny of anything in the diff. Four patterns to look for, all of which read as typing work: - **A literal replaced by a runtime lookup.** `['IFRAME_SCRIPTING']` becoming `[SomeApi.Reason.IFRAME_SCRIPTING]` adds a dependency on that object existing at runtime. Read the declaration before "fixing" the type: many platform-API parameters are template-literal string types (`` `${SomeEnum}`[] ``), which accept the plain literal already, so the swap buys nothing and costs a runtime assumption. -- **A default parameter or fallback deleted.** `function f(x = {})` becoming `function f(x: T)` removes a guard. Ask what the guard was _for_ — usually the `| undefined` the new type just dropped (see [Derive types from authoritative sources instead of re-declaring them](#derive-types-from-authoritative-sources-instead-of-re-declaring-them)). +- **A default parameter or fallback deleted.** `function f(x = {})` becoming `function f(x: T)` removes a guard. Ask what the guard was _for_ — a `| undefined` dropped by the new type is one answer worth checking first (see [Derive types from authoritative sources instead of re-declaring them](#derive-types-from-authoritative-sources-instead-of-re-declaring-them)). - **A call made optional.** `obj.method()` becoming `obj.method?.()` to satisfy a hand-written `| undefined` converts a **throw into a silent no-op**. The loud failure was load-bearing; the same broken state now produces no signal at all. - **A local widened to keep a check alive.** Annotating `let name: string | undefined` on a value the authoritative type calls `string`, so that an existing `=== undefined` branch still compiles. If the runtime check is genuinely needed, the _input_ type is wrong — widen that deliberately instead of widening downstream to match. From 99e7a687dcd48e7c7d36736bc3deaee5954304f9 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 31 Jul 2026 10:10:15 -0400 Subject: [PATCH 3/3] Add error substitution to the disguised-runtime-change patterns 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. --- docs/typescript.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/typescript.md b/docs/typescript.md index 6331764..928ae8d 100644 --- a/docs/typescript.md +++ b/docs/typescript.md @@ -1174,6 +1174,7 @@ Four patterns to look for, all of which read as typing work: - **A default parameter or fallback deleted.** `function f(x = {})` becoming `function f(x: T)` removes a guard. Ask what the guard was _for_ — a `| undefined` dropped by the new type is one answer worth checking first (see [Derive types from authoritative sources instead of re-declaring them](#derive-types-from-authoritative-sources-instead-of-re-declaring-them)). - **A call made optional.** `obj.method()` becoming `obj.method?.()` to satisfy a hand-written `| undefined` converts a **throw into a silent no-op**. The loud failure was load-bearing; the same broken state now produces no signal at all. - **A local widened to keep a check alive.** Annotating `let name: string | undefined` on a value the authoritative type calls `string`, so that an existing `=== undefined` branch still compiles. If the runtime check is genuinely needed, the _input_ type is wrong — widen that deliberately instead of widening downstream to match. +- **A thrown error replaced with a different one.** Narrowing a possibly-undefined value by guarding it 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 any ignore rule, sampling rule, alert condition, or ownership routing written against the old shape stops matching. Prefer a form that leaves the original failure intact; if a custom error is genuinely wanted, use a named class and rethrow with `cause` so the original survives. For each one, establish whether it is reachable and say so either way. "Traced, and inert on today's call paths" is a useful review outcome; "this might be a bug" is not.