From b058b4525004482970698bca07eb5775fa43d2b7 Mon Sep 17 00:00:00 2001 From: Lucas Braum Date: Sat, 17 Oct 2026 10:33:31 -0700 Subject: [PATCH] fix(react): report RefObject declarations that need `| null` (UPT-87) Fixes UPT-87. The committed verification.json (69%, 89 TP / 40 FN) was a stale tally. A fresh `pack test react` on main measures 107 TP / 22 FN (83%), and so does CI's `pack react` job on 4c3dc23: the environment is not the cause. #198 left the react tally as it was, and the scorer changed in #143/#155. Reach, then rules, for the 22 breaking misses: | cause | misses | |---|---| | reached, reported at the use site; truth marks the `RefObject` declaration (`ref-object-nullable`) | 16 | | global-jsx-namespace, no finding in the file | 4 | | `generic` in dev-docs (tsconfig "not used in compilation") | 2 | Most of the 16 declarations use `React.RefObject` without importing react, so `detect` never read them. `ref-object-nullable` gets a syntax-tree detector (element type or a type parameter bounded by one, no `null`, not a cast) and the pack reads every project file that contains `RefObject<`. Casts are a near-miss: one would be a breaking false positive at TldrawUiContextualToolbar.tsx:56, so :55 stays missed. Breaking recall 83% -> 94.6% (122/129), 0 breaking FP, unverified FP unchanged (8), still verified. ref-object-nullable 38 -> 53 TP. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Lucas Braum --- .changeset/react-ref-object-declarations.md | 5 ++ README.md | 2 +- docs/compatibility.md | 2 +- packages/core/src/packs/react/detect.ts | 75 ++++++++++++++++++- .../fixtures/ref-object-nullable/before.ts | 24 ++++++ packages/core/src/packs/react/index.ts | 26 ++++++- .../core/src/packs/react/verification.json | 6 +- 7 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 .changeset/react-ref-object-declarations.md create mode 100644 packages/core/src/packs/react/fixtures/ref-object-nullable/before.ts diff --git a/.changeset/react-ref-object-declarations.md b/.changeset/react-ref-object-declarations.md new file mode 100644 index 0000000..b7daa49 --- /dev/null +++ b/.changeset/react-ref-object-declarations.md @@ -0,0 +1,5 @@ +--- +"uptide": patch +--- + +The react pack now reports `RefObject` annotations that need `| null` at the declaration, including in files that type a prop with the global `React.RefObject` and never import react. The compiler reports these where a ref is passed in, so the declaration to edit was missed. diff --git a/README.md b/README.md index ca49f23..680ec07 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ See the [pack compatibility matrix](docs/compatibility.md) and the [migration pa | `express` | 4 → 5 | 8 | 100% | 85% | 86 | netlify/cli, bcgov/tenant-management-system, amiantos/lurker, openhab/openhab-cloud | verified | | `lucide-react` | 0 → 1 | 5 | 100% | 100% | 11 | tradeblocks-org/tradeblocks, spizeck/SFPCA, cubicsteak/keep | verified | | `next` | 15 → 16 | 9 | 100% | 88% | 34 | sambecker/exif-photo-blog, stoczec/wortkarte, benshabbat/GamesForMyKids, HiroshiIgarash/koremina | verified | -| `react` | 18 → 19 | 8 | 100% | 69% | 130 | excalidraw/excalidraw, tldraw/tldraw | verified | +| `react` | 18 → 19 | 8 | 100% | 95% | 130 | excalidraw/excalidraw, tldraw/tldraw | verified | | `react-router` | 6 → 7 | 13 | 100% | 88% | 18 | Arize-ai/phoenix, getlago/lago-front | verified | | `redis` | 4.x/5.x → 5.x/6.x | 14 | 100% | 83% | 20 | deanmoses/tacocat-gallery-sam, swoofer/mcp-coordinator, UNN-Devotek/FCM-Fallout-Chat-Mod, tearingItUp786/blog | verified | | `@sentry/browser` | 10 → 11 | 16 | 100% | 50% | 22 | Automancer-Ltd/automancer-site, sbomify/sbomify, MihaelaAghirculesei/Pokedex, Simple-With-Us/ContactLogo, getsentry/sentry-godot | verified | diff --git a/docs/compatibility.md b/docs/compatibility.md index 9a6e4a2..b7cf456 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -31,7 +31,7 @@ already run as assisted ([verification gate](packs.md)). | `lucide-react` | 0 → 1 | 5 | 100% | 100% | 11 | tradeblocks-org/tradeblocks, spizeck/SFPCA, cubicsteak/keep | verified | | `next` | 15 → 16 | 9 | 100% | 88% | 34 | sambecker/exif-photo-blog, stoczec/wortkarte, benshabbat/GamesForMyKids, HiroshiIgarash/koremina | verified | | `openai` | 4 → 7 | 15 | 100% | 15% | 20 | zeikar/charivo, openai/openai-guardrails-js, wardby/wardby, csnyder256/ux-struggle-detector, grantsingleton/batch-ai, digitalchokro/askchokro, microsoft/prompty | candidate | -| `react` | 18 → 19 | 8 | 100% | 69% | 130 | excalidraw/excalidraw, tldraw/tldraw | verified | +| `react` | 18 → 19 | 8 | 100% | 95% | 130 | excalidraw/excalidraw, tldraw/tldraw | verified | | `react-router` | 6 → 7 | 13 | 100% | 88% | 18 | Arize-ai/phoenix, getlago/lago-front | verified | | `redis` | 4.x/5.x → 5.x/6.x | 14 | 100% | 83% | 20 | deanmoses/tacocat-gallery-sam, swoofer/mcp-coordinator, UNN-Devotek/FCM-Fallout-Chat-Mod, tearingItUp786/blog | verified | | `@sentry/browser` | 10 → 11 | 16 | 100% | 50% | 22 | Automancer-Ltd/automancer-site, sbomify/sbomify, MihaelaAghirculesei/Pokedex, Simple-With-Us/ContactLogo, getsentry/sentry-godot | verified | diff --git a/packages/core/src/packs/react/detect.ts b/packages/core/src/packs/react/detect.ts index 55bb02d..6678325 100644 --- a/packages/core/src/packs/react/detect.ts +++ b/packages/core/src/packs/react/detect.ts @@ -1,4 +1,12 @@ -import { type ArrowFunction, Node, Project, type SourceFile, SyntaxKind } from 'ts-morph'; +import { + type ArrowFunction, + Node, + Project, + type SourceFile, + SyntaxKind, + type TypeNode, + type TypeReferenceNode, +} from 'ts-morph'; import type { Finding } from '../../domain/report.js'; import { onReset } from '../../shared-state.js'; import type { SourceSite } from '../contract.js'; @@ -82,3 +90,68 @@ export function dropRefCallbackReturn( } return { text, applied: false, reason: 'the reported site is not a ref callback that assigns' }; } + +/** + * Where a ref is typed as holding an element and nothing else: `ref: React.RefObject`. + * In React 19 types `useRef(null)` is `RefObject` and `RefObject` holds exactly `T`, + * so the annotation has to say `| null`. The compiler reports it where a ref is passed in (a use + * site, with the one cause spread over many), and the fix is at the declaration, which this finds. + * Only an element type (or a type parameter bounded by one) counts: `RefObject` for + * `useRef(0)` stays valid. A cast (`ref as RefObject`) compiles, so it is not a site. + */ +function isElement(type: TypeNode): boolean { + if (Node.isUnionTypeNode(type)) return type.getTypeNodes().every(isElement); + if (!Node.isTypeReference(type)) return false; + const name = type.getTypeName().getText(); + if (/Element$/.test(name)) return true; + return type + .getAncestors() + .some( + (a) => + Node.isTypeParametered(a) && + a + .getTypeParameters() + .some((p) => p.getName() === name && /Element$/.test(p.getConstraint()?.getText() ?? '')), + ); +} + +function isCast(type: Node): boolean { + let node = type; + for (let up = node.getParent(); up && Node.isTypeNode(up); up = node.getParent()) node = up; + const parent = node.getParent(); + return ( + !!parent && + (Node.isAsExpression(parent) || + Node.isTypeAssertion(parent) || + Node.isSatisfiesExpression(parent)) + ); +} + +function refObjectAnnotations(source: SourceFile): TypeReferenceNode[] { + const imported = source + .getImportDeclarations() + .some( + (d) => + d.getModuleSpecifierValue() === 'react' && + d.getNamedImports().some((n) => n.getName() === 'RefObject' && !n.getAliasNode()), + ); + return source.getDescendantsOfKind(SyntaxKind.TypeReference).filter((ref) => { + const name = ref.getTypeName(); + const qualified = Node.isQualifiedName(name); + // `React.RefObject` needs no import (the UMD namespace); a bare `RefObject` must come from "react". + if (qualified ? name.getLeft().getText() !== 'React' : !imported) return false; + if ((qualified ? name.getRight().getText() : name.getText()) !== 'RefObject') return false; + const [arg, ...rest] = ref.getTypeArguments(); + if (!arg || rest.length > 0 || !isElement(arg)) return false; + return !isCast(ref); + }); +} + +export function refObjectSites(text: string): SourceSite[] { + if (!text.includes('RefObject<')) return []; + const source = parse(text); + return refObjectAnnotations(source).map((ref) => { + const { line, column } = source.getLineAndColumnAtPos(ref.getStart()); + return { line, column, snippet: (text.split('\n')[line - 1] ?? '').trim(), name: 'RefObject' }; + }); +} diff --git a/packages/core/src/packs/react/fixtures/ref-object-nullable/before.ts b/packages/core/src/packs/react/fixtures/ref-object-nullable/before.ts new file mode 100644 index 0000000..00abe97 --- /dev/null +++ b/packages/core/src/packs/react/fixtures/ref-object-nullable/before.ts @@ -0,0 +1,24 @@ +import type { RefObject } from 'react'; +import { useRef } from 'react'; + +interface Props { + box: React.RefObject; // @uptide ref-object-nullable at:React.RefObject kind:type path:TS2322 message:"Type 'RefObject' is not assignable to type 'RefObject'." + input: RefObject; // @uptide ref-object-nullable at:RefObject kind:type path:TS2322 message:"Type 'RefObject' is not assignable to type 'RefObject'." + ready: React.RefObject; // @uptide ref-object-nullable keep at:React.RefObject + count: React.RefObject; // @uptide ref-object-nullable keep at:React.RefObject + data: React.MutableRefObject; +} + +export function useBounds(ref: React.RefObject) { // @uptide ref-object-nullable at:React.RefObject kind:type path:TS2322 message:"Type 'RefObject' is not assignable to type 'RefObject'." + return ref; +} + +export function useAny(ref: React.RefObject) { + return ref; +} + +export function Panel({ toolbar }: { toolbar: unknown }) { + const own = useRef(null); + // A cast compiles; there is nothing to edit. + return [toolbar as RefObject, own]; // @uptide ref-object-nullable keep at:RefObject +} diff --git a/packages/core/src/packs/react/index.ts b/packages/core/src/packs/react/index.ts index bf0802a..2820eca 100644 --- a/packages/core/src/packs/react/index.ts +++ b/packages/core/src/packs/react/index.ts @@ -1,6 +1,8 @@ -import { definePack, type PackRule } from '../contract.js'; +import { join, resolve } from 'node:path'; +import { projectFiles } from '../../check/importers.js'; +import { definePack, type Pack, type PackRule } from '../contract.js'; import type { TransformResult } from '../types.js'; -import { dropRefCallbackReturn, refCallbackReturnSites } from './detect.js'; +import { dropRefCallbackReturn, refCallbackReturnSites, refObjectSites } from './detect.js'; /** * react >=18 <19 → >=19 <20, with react-dom and the @types packages that move with it. Taken @@ -136,6 +138,8 @@ const rules: PackRule[] = [ kinds: ['type', 'signature'], symbols: /^TS(?:2322|2345)$/, message: /RefObject refObjectSites(text), guide: '`useRef(null)` is `RefObject` in React 19 types, and `RefObject` has a `current` of exactly `T`. A parameter, prop or field typed `RefObject` that receives such a ref becomes `RefObject`: edit the declaration the error points through, not the call site, and handle `current` being null where it is read. `types-react-codemod` has `refobject-defaults` for annotations; it skips an aliased `RefObject` import.', }, @@ -184,7 +188,7 @@ const rules: PackRule[] = [ }, ]; -export const reactPack = definePack({ +const base = definePack({ meta: { package: 'react', from: '>=18 <19', @@ -266,3 +270,19 @@ export const reactPack = definePack({ instructions: 'Migrate only the reported site to React 19, from the compiler error and the guide above. Keep every value, handler, effect and render result as it is. Never cast to `any` or suppress a diagnostic to make it compile. If the site needs a decision the guide leaves open (which props type an element is read as, whether a root can be unmounted from where the code stands), say so instead of choosing.', }); + +/** + * `React.RefObject` needs no import (the UMD `React` namespace), so a file that only types a + * prop with it never imports react and `detect`, which reads the files that do, would skip it. + * Every project file is a candidate; the text test is the cheap part. + */ +export const reactPack: Pack = { + ...base, + runtimeFindings(input) { + const dir = resolve(input.root, input.workspace); + const typing = projectFiles(dir).files.filter((file) => + input.read(join(input.workspace, file))?.includes('RefObject<'), + ); + return base.runtimeFindings?.({ ...input, files: [...(input.files ?? []), ...typing] }) ?? []; + }, +}; diff --git a/packages/core/src/packs/react/verification.json b/packages/core/src/packs/react/verification.json index b23b98b..1c60121 100644 --- a/packages/core/src/packs/react/verification.json +++ b/packages/core/src/packs/react/verification.json @@ -4,10 +4,10 @@ "repos": 2, "breaking": { "precision": 1, - "recall": 0.6899, + "recall": 0.9457, "falsePositives": 0, - "truePositives": 89, - "falseNegatives": 40 + "truePositives": 122, + "falseNegatives": 7 }, "behavior": { "precision": 1,