diff --git a/docs/react-compiler.md b/docs/react-compiler.md index e19872b8..a529f45f 100644 --- a/docs/react-compiler.md +++ b/docs/react-compiler.md @@ -4,6 +4,71 @@ We are incrementally adopting the compiler across all of our React codebases, where it is enabled but not all code may be compliant yet. As there are real performance risks if compiler violations are re-introduced, especially in cases where manual optimizations have been removed, we've put safeguards in place to prevent them from happening. +## Workflow: Identifying and fixing violations + +When working on React components or hooks in this codebase, follow this workflow: + +> **Important:** Do NOT use ESLint to check for compiler violations. The `eslint-plugin-react-hooks` rules must still pass, but they are not an indicator of whether compiler violations exist. The project uses `@doist/react-compiler-tracker` which tracks violations at the file level via `.react-compiler.rec.json`. Always use the CLI commands shown below. + +### 1. Check if the file needs attention + +Look up the file in `.react-compiler.rec.json`: + +- **Not listed** β†’ Compiler is optimizing it. Do NOT add `useMemo`, `useCallback`, or `React.memo`. +- **Listed with errors** β†’ Continue to step 2. + +### 2. Identify violations + +Run the tracker with `--show-errors` to see exact errors: + +```bash +npx @doist/react-compiler-tracker --check-files --show-errors src/path/to/file.tsx +``` + +Example output: + +``` +πŸ” Checking 1 file for React Compiler errors… +⚠️ Found 4 React Compiler issues across 1 file + +Detailed errors: + - src/path/to/file.tsx: Line 15: Cannot access refs during render (x2) + - src/path/to/file.tsx: Line 28: Existing memoization could not be preserved (x2) +``` + +Parse the output to extract error reasons and line numbers to plan the fix. + +### 3. Fix violations + +Use the patterns in [Fix patterns](#fix-patterns) to fix each violation. Focus on making the code compiler-compatible rather than adding more manual memoization. + +### 4. Verify the fix + +```bash +npx @doist/react-compiler-tracker --check-files src/path/to/file.tsx +``` + +The tool compares current errors against `.react-compiler.rec.json`: + +- **Errors increased** (exit code 1): + + ``` + React Compiler errors have increased in: + β€’ src/path/to/file.tsx: +2 + Please fix the errors and run the command again. + ``` + +- **Errors decreased** (exit code 0): + + ``` + πŸŽ‰ React Compiler errors have decreased in: + β€’ src/path/to/file.tsx: -2 + ``` + +- **No changes** (exit code 0): No output about changes, just the check summary. + +A quick way to identify violations visually is to install the [React Compiler Marker VSCode extension](https://marketplace.visualstudio.com/items?itemName=blazejkustra.react-compiler-marker), which highlights your components and hooks with ✨ or 🚫 emojis in real time. + ## Violation tracking We use [`@doist/react-compiler-tracker`](https://github.com/Doist/react-compiler-tracker) to track modules that the compiler cannot optimize. @@ -24,17 +89,30 @@ Violations are recorded in a [`.react-compiler.rec.json`](https://github.com/Doi We leverage [lint-staged](https://github.com/lint-staged/lint-staged) to automatically update the records file on commit. If the numbers of errors are increased, the commit is blocked until the errors either go back to their previous levels, or if the records file is explicitly re-created. The same check is also run on CI. -## Identifying and fixing violations - -When modifying a file with violations, consider fixing them so we can take advantage of the compiler's optimizations. - -A quick way to identify them is to install the [React Compiler Marker VSCode extension](https://marketplace.visualstudio.com/items?itemName=blazejkustra.react-compiler-marker), which highlights your components and hooks with ✨ or 🚫 emojis in real time. - -Once fixed, the file's entry will be removed from `.react-compiler.rec.json`. - -> **For LLM agents:** If a file is not in `.react-compiler.rec.json`, do not add `useMemo`, `useCallback`, or `React.memo` β€” the compiler handles memoization automatically. See [For LLM agents](#for-llm-agents) for the full workflow. - -## Common errors +Once all violations in a file are fixed, the file's entry will be removed from `.react-compiler.rec.json`. + +## Error reference + +| Compiler error message | Pattern | Section | +| ----------------------------------------------------------------------------------- | ---------------------------- | ------------------------------------------------ | +| Cannot access refs during render | Ref access during render | [Link](#ref-access-during-render) | +| Existing memoization could not be preserved | Mismatched useMemo deps | [Link](#mismatched-usememo-dependencies) | +| Support destructuring of context variables | Mutating props | [Link](#mutating-props) | +| Expression type `X` cannot be safely reordered | Default parameters for props | [Link](#default-parameters-for-props) | +| Expression type `BinaryExpression` / `LogicalExpression` cannot be safely reordered | switch(true) pattern | [Link](#switchtrue-pattern) | +| Expected Identifier, got `X` key in ObjectExpression | Computed property keys | [Link](#computed-property-keys) | +| Destructure should never be Reassign | Loop variable reassignment | [Link](#loop-variable-reassignment) | +| Hooks must always be called in a consistent order | Conditional hook calls | [Link](#conditional-hook-calls) | +| Cannot access variable before it is declared | Function declaration order | [Link](#function-declaration-order) | +| Handle TryStatement with a finalizer / without a catch clause | Try/catch blocks | [Link](#trycatch-blocks) | +| ThrowStatement inside try/catch not yet supported | Try/catch blocks | [Link](#trycatch-blocks) | +| Support value blocks … within a try/catch statement | Try/catch blocks | [Link](#trycatch-blocks) | +| This value cannot be modified (hook argument) | Mutable objects with useMemo | [Link](#mutable-objects-created-with-usememo) | +| This value cannot be modified (function return) | Mutating return values | [Link](#mutating-function-or-hook-return-values) | +| This value cannot be modified (DOM) | DOM mutations | [Link](#dom-mutations) | +| This value cannot be modified (test code) | Render-time test mutations | [Link](#render-time-mutations-in-test-code) | + +## Fix patterns ### Ref access during render @@ -208,7 +286,7 @@ function ProjectBoardView({ showCompleted: showCompletedProp }) { > > Reason: (BuildHIR::node.lowerReorderableExpression) Expression type `OptionalMemberExpression` cannot be safely reordered -The compiler can't safely reorder default parameter values that reference other parameters. +The compiler can't safely reorder default parameter values that contain expressions. The general fix is to remove the default parameter and use `??` (nullish coalescing) in the function body instead. **Before:** @@ -230,88 +308,50 @@ function DndTaskWrapper({ task, stableTaskId: stableTaskIdProp }: Props) { } ``` -The same violation applies to other expression types in default parameters: - -#### ArrowFunctionExpression - -> Reason: (BuildHIR::node.lowerReorderableExpression) Expression type `ArrowFunctionExpression` cannot be safely reordered - -**Before:** - -```typescript -function useField( - deserialize: (val: unknown) => T = (val) => val as T, // Violation -) -``` - -**After:** - -```typescript -// Extract to module-level function declaration -function defaultDeserialize(val: unknown): T { - return val as T -} - -function useField(deserialize: (val: unknown) => T = defaultDeserialize) -``` - -#### JSXElement - -> Reason: (BuildHIR::node.lowerReorderableExpression) Expression type `JSXElement` cannot be safely reordered - -**Before:** - -```typescript -function Layout({ - illustration = , // Violation -}: Props) -``` - -**After:** - -```typescript -function Layout({ illustration: illustrationProp }: Props) { - const illustration = illustrationProp ?? -} -``` - -#### CallExpression +The same violation applies to other expression types in default parameters. The fix follows the same principle β€” move the default out of the parameter list: -> Reason: (BuildHIR::node.lowerReorderableExpression) Expression type `CallExpression` cannot be safely reordered +- **`ArrowFunctionExpression`** β€” extract to a module-level function declaration: -**Before:** + ```typescript + // Before + function useField( + deserialize: (val: unknown) => T = (val) => val as T, // Violation + ) -```typescript -function TimeInput({ - referenceDate = startOfDay(new Date()), // Violation -}: Props) -``` - -**After:** + // After + function defaultDeserialize(val: unknown): T { + return val as T + } + function useField(deserialize: (val: unknown) => T = defaultDeserialize) + ``` -```typescript -function TimeInput({ referenceDate: referenceDateProp }: Props) { - const referenceDate = referenceDateProp ?? startOfDay(new Date()) -} -``` +- **`JSXElement`** β€” move the default to the function body with `??`: -For static values that don't change, extract to a module-level constant: + ```typescript + // Before + function Layout({ illustration = }: Props) // Violation -**Before:** + // After + function Layout({ illustration: illustrationProp }: Props) { + const illustration = illustrationProp ?? + } + ``` -```typescript -function Popover({ - timezone = Intl.DateTimeFormat().resolvedOptions().timeZone, // Violation -}: Props) -``` +- **`CallExpression`** β€” move the default to the function body with `??`, or extract to a module-level constant for static values: -**After:** + ```typescript + // Before + function TimeInput({ referenceDate = startOfDay(new Date()) }: Props) // Violation -```typescript -const DEFAULT_TIMEZONE = Intl.DateTimeFormat().resolvedOptions().timeZone + // After + function TimeInput({ referenceDate: referenceDateProp }: Props) { + const referenceDate = referenceDateProp ?? startOfDay(new Date()) + } -function Popover({ timezone = DEFAULT_TIMEZONE }: Props) -``` + // Alternative for static values: extract to module-level constant + const DEFAULT_TIMEZONE = Intl.DateTimeFormat().resolvedOptions().timeZone + function Popover({ timezone = DEFAULT_TIMEZONE }: Props) + ``` ### `switch(true)` pattern @@ -427,6 +467,23 @@ const renderedItems = useStoreState(store, 'renderedItems') The `store.useState()` pattern is from older versions of AriaKit. Since [version 0.4.9](https://ariakit.org/changelog#new-usestorestate-hook), AriaKit provides [`useStoreState`](https://ariakit.org/reference/use-store-state) which accepts stores that are null or undefined, returning undefined in those cases. The same principle applies to any conditional hook call - use an API that handles the conditional case internally. +#### Selector callbacks on store hooks + +When migrating from `store.useState(selectorFn)`, fetch the raw state value with `useStoreState` and derive the result in a separate expression. + +**Before:** + +```typescript +const hovercardPlacement = hovercardStore.useState((state) => state.currentPlacement.split('-')[0]) +``` + +**After:** + +```typescript +const currentPlacement = useStoreState(hovercardStore, 'currentPlacement') +const hovercardPlacement = currentPlacement?.split('-')[0] +``` + ### Function declaration order > Reason: Cannot access variable before it is declared @@ -535,7 +592,11 @@ React Compiler has limited support for try/catch statements. Several patterns ca > **Note:** A fix for some of these limitations has been merged and may be available in a future compiler release. See [facebook/react#35606](https://github.com/facebook/react/pull/35606). -#### Try-catch-finally +#### Structural violations + +These violations occur when the compiler can't parse the shape of the try/catch statement itself. + +**Try-catch-finally** > Reason: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause @@ -586,7 +647,7 @@ async function handleSubmit(event: React.FormEvent) { } ``` -#### Try without catch +**Try without catch** > Reason: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause @@ -624,7 +685,11 @@ useEffect(function loadData() { }, []) ``` -#### ThrowStatement inside try/catch +#### Content violations + +These violations occur because of what's inside the try/catch block. + +**ThrowStatement inside try/catch** > Reason: ThrowStatement inside try/catch not yet supported @@ -663,44 +728,13 @@ try { } ``` -#### Redundant try/catch - -When calling a function that already handles errors internally and returns a safe fallback, wrapping it in another try/catch is redundant and may cause violations. - -**Before:** - -```typescript -// getLocalStorageValue already has internal try/catch, returns undefined on error -function getLocalStorageValue(key: string): T | undefined { - try { - const item = localStorage.getItem(key) - return item ? JSON.parse(item) : undefined - } catch { - return undefined - } -} - -const [value, setValue] = useState(() => { - try { - return getLocalStorageValue(key) ?? defaultValue - } catch { - return defaultValue - } -}) -``` - -**After:** - -```typescript -// getLocalStorageValue already handles errors, no outer try/catch needed -const [value, setValue] = useState(() => getLocalStorageValue(key) ?? defaultValue) -``` - -#### Value blocks in try/catch (general pattern) +**Value blocks in try/catch** > Reason: Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement -Conditional expressions (`? :`), logical operators (`&&`, `||`), nullish coalescing (`??`), and other "value blocks" inside try/catch cause violations. The general fix is to extract the try/catch logic into a helper function outside the component. +Conditional expressions (`? :`), logical operators (`&&`, `||`), nullish coalescing (`??`), and optional chaining (`?.()`) inside try/catch cause violations. The general fix is to extract the try/catch logic into a helper function, or replace optional chaining with explicit `if` checks. + +_Extract to helper function:_ **Before:** @@ -728,11 +762,7 @@ function safeRiskyOperation(deps: Deps) { const value = useMemo(() => safeRiskyOperation(deps), [deps]) ``` -#### Optional chaining in try/catch blocks - -> Reason: Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement - -The compiler can't handle optional chaining (`?.()`) inside try/catch blocks. +_Replace optional chaining with explicit checks:_ **Before:** @@ -760,6 +790,39 @@ try { } ``` +**Redundant try/catch** + +When calling a function that already handles errors internally and returns a safe fallback, wrapping it in another try/catch is redundant and may cause violations. + +**Before:** + +```typescript +// getLocalStorageValue already has internal try/catch, returns undefined on error +function getLocalStorageValue(key: string): T | undefined { + try { + const item = localStorage.getItem(key) + return item ? JSON.parse(item) : undefined + } catch { + return undefined + } +} + +const [value, setValue] = useState(() => { + try { + return getLocalStorageValue(key) ?? defaultValue + } catch { + return defaultValue + } +}) +``` + +**After:** + +```typescript +// getLocalStorageValue already handles errors, no outer try/catch needed +const [value, setValue] = useState(() => getLocalStorageValue(key) ?? defaultValue) +``` + ### Mutable objects created with useMemo > Reason: This value cannot be modified. Modifying a value previously passed as an argument to a hook is not allowed. @@ -918,70 +981,67 @@ useLayoutEffect( ) ``` -## For LLM agents +### Render-time mutations in test code -When working on React components or hooks in this codebase, follow this workflow: - -### 1. Check if the file needs attention - -Look up the file in `.react-compiler.rec.json`: +> Reason: This value cannot be modified -- **Not listed** β†’ Compiler is optimizing it. Do NOT add `useMemo`, `useCallback`, or `React.memo`. -- **Listed with errors** β†’ Continue to step 2. +Test files are compiled too, so patterns like mutating an external `let` variable or object property from inside a rendered component trigger this violation. -### 2. Identify violations +#### Render counting with `React.Profiler` -Run the tracker with `--show-errors` to see exact errors: +**Before:** -```bash -npx @doist/react-compiler-tracker --check-files --show-errors src/path/to/file.tsx +```typescript +let renderCount = 0 +function TestComponent() { + renderCount += 1 // Violation: mutation during render + return
+} +render() +expect(renderCount).toBe(1) ``` -Example output: - -``` -πŸ” Checking 1 file for React Compiler errors… -⚠️ Found 4 React Compiler issues across 1 file +**After:** -Detailed errors: - - src/path/to/file.tsx: Line 15: Cannot access refs during render (x2) - - src/path/to/file.tsx: Line 28: Existing memoization could not be preserved (x2) +```typescript +const onRender = jest.fn() +function TestComponent() { + return
+} +render( + + + , +) +expect(onRender).toHaveBeenCalledTimes(1) ``` -Parse the output to extract error reasons and line numbers to plan the fix. - -### 3. Fix violations - -Use the patterns in [Common errors](#common-errors) to fix each violation. Focus on making the code compiler-compatible rather than adding more manual memoization. - -### 4. Verify the fix +#### Hook testing with `renderHook` -**CLI Tool (recommended)** +**Before:** -```bash -npx @doist/react-compiler-tracker --check-files src/path/to/file.tsx +```typescript +let state: ReturnType +function TestComponent(options: Options) { + state = useMyHook(options) // Violation: mutation during render + return
+} +render() +expect(state.someValue).toBe(expected) ``` -The tool compares current errors against `.react-compiler.rec.json`: - -- **Errors increased** (exit code 1): - - ``` - React Compiler errors have increased in: - β€’ src/path/to/file.tsx: +2 - Please fix the errors and run the command again. - ``` +**After:** -- **Errors decreased** (exit code 0): +```typescript +const { result } = renderHook(() => useMyHook(options)) +expect(result.current.someValue).toBe(expected) +``` - ``` - πŸŽ‰ React Compiler errors have decreased in: - β€’ src/path/to/file.tsx: -2 - ``` +## Appendix: Alternative verification methods -- **No changes** (exit code 0): No output about changes, just the check summary. +These methods are alternatives to the CLI tool for deeper debugging. The CLI tool (shown in [Verify the fix](#4-verify-the-fix)) is the recommended approach for day-to-day use. -**Babel with Inline Logger (alternative)** +**Babel with Inline Logger** Run a Node script that uses Babel's API with a custom logger to see exact errors: