Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/react-ref-object-declarations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"uptide": patch
---

The react pack now reports `RefObject<HTMLElement>` 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.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion docs/compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
75 changes: 74 additions & 1 deletion packages/core/src/packs/react/detect.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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<HTMLDivElement>`.
* In React 19 types `useRef<T>(null)` is `RefObject<T | null>` and `RefObject<T>` 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<number>` for
* `useRef(0)` stays valid. A cast (`ref as RefObject<HTMLDivElement>`) 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' };
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { RefObject } from 'react';
import { useRef } from 'react';

interface Props {
box: React.RefObject<HTMLDivElement>; // @uptide ref-object-nullable at:React.RefObject kind:type path:TS2322 message:"Type 'RefObject<HTMLDivElement | null>' is not assignable to type 'RefObject<HTMLDivElement>'."
input: RefObject<HTMLInputElement>; // @uptide ref-object-nullable at:RefObject kind:type path:TS2322 message:"Type 'RefObject<HTMLInputElement | null>' is not assignable to type 'RefObject<HTMLInputElement>'."
ready: React.RefObject<HTMLDivElement | null>; // @uptide ref-object-nullable keep at:React.RefObject
count: React.RefObject<number>; // @uptide ref-object-nullable keep at:React.RefObject
data: React.MutableRefObject<HTMLDivElement>;
}

export function useBounds<T extends HTMLElement>(ref: React.RefObject<T>) { // @uptide ref-object-nullable at:React.RefObject kind:type path:TS2322 message:"Type 'RefObject<T | null>' is not assignable to type 'RefObject<T>'."
return ref;
}

export function useAny<T>(ref: React.RefObject<T>) {
return ref;
}

export function Panel({ toolbar }: { toolbar: unknown }) {
const own = useRef<HTMLDivElement>(null);
// A cast compiles; there is nothing to edit.
return [toolbar as RefObject<HTMLDivElement>, own]; // @uptide ref-object-nullable keep at:RefObject
}
26 changes: 23 additions & 3 deletions packages/core/src/packs/react/index.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -136,6 +138,8 @@ const rules: PackRule[] = [
kinds: ['type', 'signature'],
symbols: /^TS(?:2322|2345)$/,
message: /RefObject</,
// The compiler reports a ref where it is passed; the declaration to edit is found here.
detect: (text) => refObjectSites(text),
guide:
'`useRef<T>(null)` is `RefObject<T | null>` in React 19 types, and `RefObject<T>` has a `current` of exactly `T`. A parameter, prop or field typed `RefObject<HTMLDivElement>` that receives such a ref becomes `RefObject<HTMLDivElement | null>`: 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.',
},
Expand Down Expand Up @@ -184,7 +188,7 @@ const rules: PackRule[] = [
},
];

export const reactPack = definePack({
const base = definePack({
meta: {
package: 'react',
from: '>=18 <19',
Expand Down Expand Up @@ -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<T>` 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] }) ?? [];
},
};
6 changes: 3 additions & 3 deletions packages/core/src/packs/react/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading