diff --git a/compiler/.claude/settings.local.json b/compiler/.claude/settings.local.json index 9bbd18802e4..cb3a55acfac 100644 --- a/compiler/.claude/settings.local.json +++ b/compiler/.claude/settings.local.json @@ -1,7 +1,14 @@ { "permissions": { "allow": [ - "Bash(node scripts/enable-feature-flag.js:*)" + "Bash(node scripts/enable-feature-flag.js:*)", + "Bash(yarn snap:*)", + "Bash(for test in \"error.invalid-access-ref-during-render\" \"error.invalid-ref-in-callback-invoked-during-render\" \"error.invalid-impure-functions-in-render-via-render-helper\")", + "Bash(do)", + "Bash(echo:*)", + "Bash(done)", + "Bash(cat:*)", + "Bash(sl revert:*)" ], "deny": [], "ask": [] diff --git a/compiler/CLAUDE.md b/compiler/CLAUDE.md new file mode 100644 index 00000000000..7a25725c659 --- /dev/null +++ b/compiler/CLAUDE.md @@ -0,0 +1,245 @@ +# React Compiler Knowledge Base + +This document contains knowledge about the React Compiler gathered during development sessions. It serves as a reference for understanding the codebase architecture and key concepts. + +## Project Structure + +- `packages/babel-plugin-react-compiler/` - Main compiler package + - `src/HIR/` - High-level Intermediate Representation types and utilities + - `src/Inference/` - Effect inference passes (aliasing, mutation, etc.) + - `src/Validation/` - Validation passes that check for errors + - `src/Entrypoint/Pipeline.ts` - Main compilation pipeline with pass ordering + - `src/__tests__/fixtures/compiler/` - Test fixtures + - `error.*.js` - Fixtures that should produce compilation errors + - `*.expect.md` - Expected output for each fixture + +## Running Tests + +```bash +# Run all tests +yarn snap + +# Run specific test by pattern +yarn snap -p + +# Update fixture outputs +yarn snap -u +``` + +## Version Control + +This repository uses Sapling (`sl`) for version control. Unlike git, Sapling does not require explicitly adding files to the staging area. + +```bash +# Check status +sl status + +# Commit all changes +sl commit -m "Your commit message" + +# Commit with multi-line message using heredoc +sl commit -m "$(cat <<'EOF' +Summary line + +Detailed description here +EOF +)" +``` + +## Key Concepts + +### HIR (High-level Intermediate Representation) + +The compiler converts source code to HIR for analysis. Key types in `src/HIR/HIR.ts`: + +- **HIRFunction** - A function being compiled + - `body.blocks` - Map of BasicBlocks + - `context` - Captured variables from outer scope + - `params` - Function parameters + - `returns` - The function's return place + - `aliasingEffects` - Effects that describe the function's behavior when called + +- **Instruction** - A single operation + - `lvalue` - The place being assigned to + - `value` - The instruction kind (CallExpression, FunctionExpression, LoadLocal, etc.) + - `effects` - Array of AliasingEffects for this instruction + +- **Terminal** - Block terminators (return, branch, etc.) + - `effects` - Array of AliasingEffects + +- **Place** - A reference to a value + - `identifier.id` - Unique IdentifierId + +- **Phi nodes** - Join points for values from different control flow paths + - Located at `block.phis` + - `phi.place` - The result place + - `phi.operands` - Map of predecessor block to source place + +### AliasingEffects System + +Effects describe data flow and operations. Defined in `src/Inference/AliasingEffects.ts`: + +**Data Flow Effects:** +- `Impure` - Marks a place as containing an impure value (e.g., Date.now() result, ref.current) +- `Capture a -> b` - Value from `a` is captured into `b` (mutable capture) +- `Alias a -> b` - `b` aliases `a` +- `ImmutableCapture a -> b` - Immutable capture (like Capture but read-only) +- `Assign a -> b` - Direct assignment +- `MaybeAlias a -> b` - Possible aliasing +- `CreateFrom a -> b` - Created from source + +**Mutation Effects:** +- `Mutate value` - Value is mutated +- `MutateTransitive value` - Value and transitive captures are mutated +- `MutateConditionally value` - May mutate +- `MutateTransitiveConditionally value` - May mutate transitively + +**Other Effects:** +- `Render place` - Place is used in render context (JSX props, component return) +- `Freeze place` - Place is frozen (made immutable) +- `Create place` - New value created +- `CreateFunction` - Function expression created, includes `captures` array +- `Apply` - Function application with receiver, function, args, and result + +### Hook Aliasing Signatures + +Located in `src/HIR/Globals.ts`, hooks can define custom aliasing signatures to control how data flows through them. + +**Structure:** +```typescript +aliasing: { + receiver: '@receiver', // The hook function itself + params: ['@param0'], // Named positional parameters + rest: '@rest', // Rest parameters (or null) + returns: '@returns', // Return value + temporaries: [], // Temporary values during execution + effects: [ // Array of effects to apply when hook is called + {kind: 'Freeze', value: '@param0', reason: ValueReason.HookCaptured}, + {kind: 'Assign', from: '@param0', into: '@returns'}, + ], +} +``` + +**Common patterns:** + +1. **RenderHookAliasing** (useState, useContext, useMemo, useCallback): + - Freezes arguments (`Freeze @rest`) + - Marks arguments as render-time (`Render @rest`) + - Creates frozen return value + - Aliases arguments to return + +2. **EffectHookAliasing** (useEffect, useLayoutEffect, useInsertionEffect): + - Freezes function and deps + - Creates internal effect object + - Captures function and deps into effect + - Returns undefined + +3. **Event handler hooks** (useEffectEvent): + - Freezes callback (`Freeze @fn`) + - Aliases input to return (`Assign @fn -> @returns`) + - NO Render effect (callback not called during render) + +**Example: useEffectEvent** +```typescript +const UseEffectEventHook = addHook( + DEFAULT_SHAPES, + { + positionalParams: [Effect.Freeze], // Takes one positional param + restParam: null, + returnType: {kind: 'Function', ...}, + calleeEffect: Effect.Read, + hookKind: 'useEffectEvent', + returnValueKind: ValueKind.Frozen, + aliasing: { + receiver: '@receiver', + params: ['@fn'], // Name for the callback parameter + rest: null, + returns: '@returns', + temporaries: [], + effects: [ + {kind: 'Freeze', value: '@fn', reason: ValueReason.HookCaptured}, + {kind: 'Assign', from: '@fn', into: '@returns'}, + // Note: NO Render effect - callback is not called during render + ], + }, + }, + BuiltInUseEffectEventId, +); + +// Add as both names for compatibility +['useEffectEvent', UseEffectEventHook], +['experimental_useEffectEvent', UseEffectEventHook], +``` + +**Key insight:** If a hook is missing an `aliasing` config, it falls back to `DefaultNonmutatingHook` which includes a `Render` effect on all arguments. This can cause false positives for hooks like `useEffectEvent` whose callbacks are not called during render. + +### Effect Inference Pipeline + +Effects are populated by `InferMutationAliasingEffects` (runs before validation): + +1. For `Date.now()`, `Math.random()` etc. - adds `Impure` effect (controlled by `validateNoImpureFunctionsInRender` config) +2. For `ref.current` access - adds `Impure` effect (controlled by `validateRefAccessDuringRender` config) +3. For return terminals - adds `Alias` from return value to `fn.returns` +4. For component/JSX returns - adds `Render` effect +5. For function expressions - adds `CreateFunction` effect with captures + +### Validation: validateNoImpureValuesInRender + +Located at `src/Validation/ValidateNoImpureValuesInRender.ts` + +**Purpose:** Detect when impure values (refs, Date.now results, etc.) flow into render context. + +**Algorithm:** +1. Track impure values in a Map +2. Track functions with impure returns separately (they're not impure values themselves) +3. Fixed-point iteration over all blocks: + - Process phi nodes (any impure operand makes result impure) + - Process instruction effects + - Process terminal effects + - Backwards propagation for mutated LoadLocal values +4. Validate: check all Render effects against impure values + +**Key patterns:** +- `Impure` effect marks the target as impure +- `Capture/Alias/etc` propagates impurity from source to target +- `Apply` propagates impurity from args/receiver to result +- `CreateFunction` propagates impurity from captured values (but NOT from body effects) +- If a value has both `Render` and `Capture` in same instruction, only error on Render (don't cascade) + +**Tracking functions with impure returns:** +- Separate from the `impure` map (function values aren't impure, just their returns) +- Populated when analyzing FunctionExpression bodies +- Used when: + 1. Calling the function - mark call result as impure + 2. Capturing the function - mark target as impure (for object.foo = impureFunc cases) + +**Backwards propagation:** +- When `$x = LoadLocal y` and `$x` is mutated with impure content, mark `y` as impure +- This handles: `const arr = []; arr.push(impure); render(arr)` + +## Known Issues / Edge Cases + +### Function Outlining +After `OutlineFunctions` pass, inner functions are replaced with `LoadGlobal(_temp)`. The validation runs BEFORE outlining, so it sees the original FunctionExpression. But be aware that test output shows post-outlining HIR. + +### SSA and LoadLocal +In SSA form, each `LoadLocal` creates a new identifier. When a loaded value is mutated: +- `$x = LoadLocal y` +- `mutate($x, impure)` +- `$z = LoadLocal y` (different from $x!) +- `render($z)` + +The impurity in $x must propagate back to y, then forward to $z. This requires backwards propagation in the fixed-point loop. + +## Configuration Flags + +In `Environment.ts` / test directives: +- `validateNoImpureFunctionsInRender` - Enable impure function validation (Date.now, Math.random, etc.) +- `validateRefAccessDuringRender` - Enable ref access validation + +## Debugging Tips + +1. Run `yarn snap -p ` to see full HIR output with effects +2. Look for `@aliasingEffects=` on FunctionExpressions +3. Look for `Impure`, `Render`, `Capture` effects on instructions +4. Check the pass ordering in Pipeline.ts to understand when effects are populated vs validated diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index a61967ef4a0..a98667c6954 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -132,6 +132,12 @@ export class CompilerDiagnostic { return new CompilerDiagnostic({...options, details: []}); } + clone(): CompilerDiagnostic { + const cloned = CompilerDiagnostic.create({...this.options}); + cloned.options.details = [...this.options.details]; + return cloned; + } + get reason(): CompilerDiagnosticOptions['reason'] { return this.options.reason; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 30d66522715..d579968e178 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -96,7 +96,6 @@ import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHI import {outlineJSX} from '../Optimization/OutlineJsx'; import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls'; import {transformFire} from '../Transform'; -import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender'; import {validateStaticComponents} from '../Validation/ValidateStaticComponents'; import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions'; import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects'; @@ -107,6 +106,7 @@ import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions'; import {optimizeForSSR} from '../Optimization/OptimizeForSSR'; import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies'; import {validateSourceLocations} from '../Validation/ValidateSourceLocations'; +import {validateNoImpureValuesInRender} from '../Validation/ValidateNoImpureValuesInRender'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -271,10 +271,6 @@ function runWithEnvironment( assertValidMutableRanges(hir); } - if (env.config.validateRefAccessDuringRender) { - validateNoRefAccessInRender(hir).unwrap(); - } - if (env.config.validateNoSetStateInRender) { validateNoSetStateInRender(hir).unwrap(); } @@ -296,8 +292,15 @@ function runWithEnvironment( env.logErrors(validateNoJSXInTryStatement(hir)); } - if (env.config.validateNoImpureFunctionsInRender) { - validateNoImpureFunctionsInRender(hir).unwrap(); + if ( + env.config.validateNoImpureFunctionsInRender || + env.config.validateRefAccessDuringRender + ) { + validateNoImpureValuesInRender(hir).unwrap(); + } + + if (env.config.validateRefAccessDuringRender) { + validateNoRefAccessInRender(hir).unwrap(); } validateNoFreezingKnownMutableFunctions(hir).unwrap(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 441b5d5452a..b0b074a18ff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -38,7 +38,7 @@ import { addObject, } from './ObjectShape'; import {BuiltInType, ObjectType, PolyType} from './Types'; -import {TypeConfig} from './TypeSchema'; +import {AliasingSignatureConfig, TypeConfig} from './TypeSchema'; import {assertExhaustive} from '../Utils/utils'; import {isHookName} from './Environment'; import {CompilerError, SourceLocation} from '..'; @@ -626,11 +626,136 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ // TODO: rest of Global objects ]; +const RenderHookAliasing: ( + reason: ValueReason, +) => AliasingSignatureConfig = reason => ({ + receiver: '@receiver', + params: [], + rest: '@rest', + returns: '@returns', + temporaries: [], + effects: [ + // Freeze the arguments + { + kind: 'Freeze', + value: '@rest', + reason: ValueReason.HookCaptured, + }, + // Render the arguments + { + kind: 'Render', + place: '@rest', + }, + // Returns a frozen value + { + kind: 'Create', + into: '@returns', + value: ValueKind.Frozen, + reason, + }, + // May alias any arguments into the return + { + kind: 'Alias', + from: '@rest', + into: '@returns', + }, + ], +}); + +const EffectHookAliasing: AliasingSignatureConfig = { + receiver: '@receiver', + params: ['@fn', '@deps'], + rest: '@rest', + returns: '@returns', + temporaries: ['@effect'], + effects: [ + // Freezes the function and deps + { + kind: 'Freeze', + value: '@rest', + reason: ValueReason.Effect, + }, + { + kind: 'Freeze', + value: '@fn', + reason: ValueReason.Effect, + }, + { + kind: 'Freeze', + value: '@deps', + reason: ValueReason.Effect, + }, + // Deps are accessed during render + { + kind: 'Render', + place: '@deps', + }, + // Internally creates an effect object that captures the function and deps + { + kind: 'Create', + into: '@effect', + value: ValueKind.Frozen, + reason: ValueReason.KnownReturnSignature, + }, + // The effect stores the function and dependencies + { + kind: 'Capture', + from: '@rest', + into: '@effect', + }, + { + kind: 'Capture', + from: '@fn', + into: '@effect', + }, + // Returns undefined + { + kind: 'Create', + into: '@returns', + value: ValueKind.Primitive, + reason: ValueReason.KnownReturnSignature, + }, + ], +}; + /* * TODO(mofeiZ): We currently only store rest param effects for hooks. * now that FeatureFlag `enableTreatHooksAsFunctions` is removed we can * use positional params too (?) */ +const useEffectEvent = addHook( + DEFAULT_SHAPES, + { + positionalParams: [], + restParam: Effect.Freeze, + returnType: { + kind: 'Function', + return: {kind: 'Poly'}, + shapeId: BuiltInEffectEventId, + isConstructor: false, + }, + calleeEffect: Effect.Read, + hookKind: 'useEffectEvent', + // Frozen because it should not mutate any locally-bound values + returnValueKind: ValueKind.Frozen, + aliasing: { + receiver: '@receiver', + params: ['@value'], + rest: null, + returns: '@return', + temporaries: [], + effects: [ + {kind: 'Assign', from: '@value', into: '@return'}, + { + kind: 'Freeze', + value: '@value', + reason: ValueReason.HookCaptured, + }, + ], + }, + }, + BuiltInUseEffectEventId, +); const REACT_APIS: Array<[string, BuiltInType]> = [ [ 'useContext', @@ -644,6 +769,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ hookKind: 'useContext', returnValueKind: ValueKind.Frozen, returnValueReason: ValueReason.Context, + aliasing: RenderHookAliasing(ValueReason.Context), }, BuiltInUseContextHookId, ), @@ -658,6 +784,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ hookKind: 'useState', returnValueKind: ValueKind.Frozen, returnValueReason: ValueReason.State, + aliasing: RenderHookAliasing(ValueReason.State), }), ], [ @@ -670,6 +797,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ hookKind: 'useActionState', returnValueKind: ValueKind.Frozen, returnValueReason: ValueReason.State, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -682,6 +810,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ hookKind: 'useReducer', returnValueKind: ValueKind.Frozen, returnValueReason: ValueReason.ReducerState, + aliasing: RenderHookAliasing(ValueReason.ReducerState), }), ], [ @@ -715,6 +844,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useMemo', returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -726,6 +856,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useCallback', returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -739,41 +870,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useEffect', returnValueKind: ValueKind.Frozen, - aliasing: { - receiver: '@receiver', - params: [], - rest: '@rest', - returns: '@returns', - temporaries: ['@effect'], - effects: [ - // Freezes the function and deps - { - kind: 'Freeze', - value: '@rest', - reason: ValueReason.Effect, - }, - // Internally creates an effect object that captures the function and deps - { - kind: 'Create', - into: '@effect', - value: ValueKind.Frozen, - reason: ValueReason.KnownReturnSignature, - }, - // The effect stores the function and dependencies - { - kind: 'Capture', - from: '@rest', - into: '@effect', - }, - // Returns undefined - { - kind: 'Create', - into: '@returns', - value: ValueKind.Primitive, - reason: ValueReason.KnownReturnSignature, - }, - ], - }, + aliasing: EffectHookAliasing, }, BuiltInUseEffectHookId, ), @@ -789,6 +886,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useLayoutEffect', returnValueKind: ValueKind.Frozen, + aliasing: EffectHookAliasing, }, BuiltInUseLayoutEffectHookId, ), @@ -804,6 +902,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useInsertionEffect', returnValueKind: ValueKind.Frozen, + aliasing: EffectHookAliasing, }, BuiltInUseInsertionEffectHookId, ), @@ -817,6 +916,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useTransition', returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -829,6 +929,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ hookKind: 'useOptimistic', returnValueKind: ValueKind.Frozen, returnValueReason: ValueReason.State, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -842,6 +943,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ returnType: {kind: 'Poly'}, calleeEffect: Effect.Read, returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }, BuiltInUseOperatorId, ), @@ -866,27 +968,8 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ BuiltInFireId, ), ], - [ - 'useEffectEvent', - addHook( - DEFAULT_SHAPES, - { - positionalParams: [], - restParam: Effect.Freeze, - returnType: { - kind: 'Function', - return: {kind: 'Poly'}, - shapeId: BuiltInEffectEventId, - isConstructor: false, - }, - calleeEffect: Effect.Read, - hookKind: 'useEffectEvent', - // Frozen because it should not mutate any locally-bound values - returnValueKind: ValueKind.Frozen, - }, - BuiltInUseEffectEventId, - ), - ], + ['useEffectEvent', useEffectEvent], + ['experimental_useEffectEvent', useEffectEvent], ['AUTODEPS', addObject(DEFAULT_SHAPES, BuiltInAutodepsId, [])], ]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index c396f6b0881..9e8dae0abd0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1879,7 +1879,15 @@ export function isRefValueType(id: Identifier): boolean { } export function isUseRefType(id: Identifier): boolean { - return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInUseRefId'; + return isUseRefType_(id.type); +} + +export function isUseRefType_(type: Type): boolean { + return ( + (type.kind === 'Object' && type.shapeId === 'BuiltInUseRefId') || + (type.kind === 'Phi' && + type.operands.some(operand => isUseRefType_(operand))) + ); } export function isUseStateType(id: Identifier): boolean { @@ -1890,6 +1898,13 @@ export function isJsxType(type: Type): boolean { return type.kind === 'Object' && type.shapeId === 'BuiltInJsx'; } +export function isJsxOrJsxUnionType(type: Type): boolean { + return ( + (type.kind === 'Object' && type.shapeId === 'BuiltInJsx') || + (type.kind === 'Phi' && type.operands.some(op => isJsxOrJsxUnionType(op))) + ); +} + export function isRefOrRefValue(id: Identifier): boolean { return isUseRefType(id) || isRefValueType(id); } @@ -2058,4 +2073,23 @@ export function getHookKindForType( return null; } +export function areEqualSourceLocations( + loc1: SourceLocation, + loc2: SourceLocation, +): boolean { + if (typeof loc1 === 'symbol' || typeof loc2 === 'symbol') { + return false; + } + return ( + loc1.filename === loc2.filename && + loc1.identifierName === loc2.identifierName && + loc1.start.line === loc2.start.line && + loc1.start.column === loc2.start.column && + loc1.start.index === loc2.start.index && + loc1.end.line === loc2.end.line && + loc1.end.column === loc2.end.column && + loc1.end.index === loc2.end.index + ); +} + export * from './Types'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts index d3ecb2abdcd..1a2a56a7f79 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts @@ -988,7 +988,7 @@ export function createTemporaryPlace( identifier: makeTemporaryIdentifier(env.nextIdentifierId, loc), reactive: false, effect: Effect.Unknown, - loc: GeneratedSource, + loc, }; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index c92f9e55623..1ea88272d8c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError} from '../CompilerError'; +import {CompilerError, ErrorCategory} from '../CompilerError'; import {AliasingEffect, AliasingSignature} from '../Inference/AliasingEffects'; import {assertExhaustive} from '../Utils/utils'; import { @@ -190,14 +190,22 @@ function parseAliasingSignatureConfig( }; } case 'Impure': { - const place = lookup(effect.place); + const into = lookup(effect.into); return { kind: 'Impure', + into, + category: ErrorCategory.Purity, + description: effect.description, + reason: effect.reason, + sourceMessage: effect.sourceMessage, + usageMessage: effect.usageMessage, + }; + } + case 'Render': { + const place = lookup(effect.place); + return { + kind: 'Render', place, - error: CompilerError.throwTodo({ - reason: 'Support impure effect declarations', - loc: GeneratedSource, - }), }; } case 'Apply': { @@ -1513,6 +1521,11 @@ export const DefaultNonmutatingHook = addHook( value: '@rest', reason: ValueReason.HookCaptured, }, + // Render the arguments + { + kind: 'Render', + place: '@rest', + }, // Returns a frozen value { kind: 'Create', diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 71fb4c43b33..2d33f7c724a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -1009,7 +1009,7 @@ export function printAliasingEffect(effect: AliasingEffect): string { return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`; } case 'Impure': { - return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`; + return `Impure ${printPlaceForAliasEffect(effect.into)} reason=${effect.reason} description=${effect.description}`; } case 'Render': { return `Render ${printPlaceForAliasEffect(effect.place)}`; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts index eeaaebf7a39..a88d70aa7e4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts @@ -185,11 +185,29 @@ export const ApplyEffectSchema: z.ZodType = z.object({ export type ImpureEffectConfig = { kind: 'Impure'; - place: string; + into: string; + reason: string; + description: string; + sourceMessage: string; + usageMessage: string; }; export const ImpureEffectSchema: z.ZodType = z.object({ kind: z.literal('Impure'), + into: LifetimeIdSchema, + reason: z.string(), + description: z.string(), + sourceMessage: z.string(), + usageMessage: z.string(), +}); + +export type RenderEffectConfig = { + kind: 'Render'; + place: string; +}; + +export const RenderEffectSchema: z.ZodType = z.object({ + kind: z.literal('Render'), place: LifetimeIdSchema, }); @@ -204,7 +222,8 @@ export type AliasingEffectConfig = | ImpureEffectConfig | MutateEffectConfig | MutateTransitiveConditionallyConfig - | ApplyEffectConfig; + | ApplyEffectConfig + | RenderEffectConfig; export const AliasingEffectSchema: z.ZodType = z.union([ FreezeEffectSchema, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts index 7f30e25a5c0..ce13cf0c3e0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerDiagnostic} from '../CompilerError'; +import {CompilerDiagnostic, ErrorCategory} from '../CompilerError'; import { FunctionExpression, GeneratedSource, @@ -162,7 +162,15 @@ export type AliasingEffect = /** * Indicates a side-effect that is not safe during render */ - | {kind: 'Impure'; place: Place; error: CompilerDiagnostic} + | { + kind: 'Impure'; + into: Place; + category: ErrorCategory; + reason: string; + description: string; + usageMessage: string; + sourceMessage: string; + } /** * Indicates that a given place is accessed during render. Used to distingush * hook arguments that are known to be called immediately vs those used for @@ -222,6 +230,14 @@ export function hashEffect(effect: AliasingEffect): string { return [effect.kind, effect.value.identifier.id, effect.reason].join(':'); } case 'Impure': + return [ + effect.kind, + effect.into.identifier.id, + effect.reason, + effect.description, + effect.usageMessage, + effect.sourceMessage, + ].join(':'); case 'Render': { return [effect.kind, effect.place.identifier.id].join(':'); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/ControlDominators.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/ControlDominators.ts index 1fab651947a..b6fd85cd054 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/ControlDominators.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/ControlDominators.ts @@ -8,7 +8,7 @@ import {BlockId, computePostDominatorTree, HIRFunction, Place} from '../HIR'; import {PostDominator} from '../HIR/Dominator'; -export type ControlDominators = (id: BlockId) => boolean; +export type ControlDominators = (id: BlockId) => Place | null; /** * Returns an object that lazily calculates whether particular blocks are controlled @@ -23,7 +23,7 @@ export function createControlDominators( }); const postDominatorFrontierCache = new Map>(); - function isControlledBlock(id: BlockId): boolean { + function isControlledBlock(id: BlockId): Place | null { let controlBlocks = postDominatorFrontierCache.get(id); if (controlBlocks === undefined) { controlBlocks = postDominatorFrontier(fn, postDominators, id); @@ -35,24 +35,24 @@ export function createControlDominators( case 'if': case 'branch': { if (isControlVariable(controlBlock.terminal.test)) { - return true; + return controlBlock.terminal.test; } break; } case 'switch': { if (isControlVariable(controlBlock.terminal.test)) { - return true; + return controlBlock.terminal.test; } for (const case_ of controlBlock.terminal.cases) { if (case_.test !== null && isControlVariable(case_.test)) { - return true; + return case_.test; } } break; } } } - return false; + return null; } return isControlledBlock; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 4a027b87b6a..e6866a51a3a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -27,11 +27,13 @@ import { InstructionKind, InstructionValue, isArrayType, - isJsxType, + isJsxOrJsxUnionType, isMapType, + isMutableEffect, isPrimitiveType, isRefOrRefValue, isSetType, + isUseRefType, makeIdentifierId, Phi, Place, @@ -70,6 +72,7 @@ import { MutationReason, } from './AliasingEffects'; import {ErrorCategory} from '../CompilerError'; +import {REF_ERROR_DESCRIPTION} from '../Validation/ValidateNoRefAccessInRender'; const DEBUG = false; @@ -569,14 +572,32 @@ function inferBlock( terminal.effects = effects.length !== 0 ? effects : null; } } else if (terminal.kind === 'return') { + terminal.effects = [ + context.internEffect({ + kind: 'Alias', + from: terminal.value, + into: context.fn.returns, + }), + ]; if (!context.isFuctionExpression) { - terminal.effects = [ + terminal.effects.push( context.internEffect({ kind: 'Freeze', value: terminal.value, reason: ValueReason.JsxCaptured, }), - ]; + ); + } + if ( + context.fn.fnType === 'Component' || + isJsxOrJsxUnionType(context.fn.returns.identifier.type) + ) { + terminal.effects.push( + context.internEffect({ + kind: 'Render', + place: terminal.value, + }), + ); } } } @@ -749,17 +770,7 @@ function applyEffect( break; } case 'ImmutableCapture': { - const kind = state.kind(effect.from).kind; - switch (kind) { - case ValueKind.Global: - case ValueKind.Primitive: { - // no-op: we don't need to track data flow for copy types - break; - } - default: { - effects.push(effect); - } - } + effects.push(effect); break; } case 'CreateFrom': { @@ -1061,6 +1072,17 @@ function applyEffect( reason: new Set(fromValue.reason), }); state.define(effect.into, value); + applyEffect( + context, + state, + { + kind: 'ImmutableCapture', + from: effect.from, + into: effect.into, + }, + initialized, + effects, + ); break; } default: { @@ -1966,6 +1988,11 @@ function computeSignatureForInstruction( value: ValueKind.Primitive, reason: ValueReason.Other, }); + effects.push({ + kind: 'ImmutableCapture', + from: value.object, + into: lvalue, + }); } else { effects.push({ kind: 'CreateFrom', @@ -1973,6 +2000,20 @@ function computeSignatureForInstruction( into: lvalue, }); } + if ( + env.config.validateRefAccessDuringRender && + isUseRefType(value.object.identifier) + ) { + effects.push({ + kind: 'Impure', + into: lvalue, + category: ErrorCategory.Refs, + reason: `Cannot access ref value during render`, + description: REF_ERROR_DESCRIPTION, + sourceMessage: `Ref is initially accessed`, + usageMessage: `Ref value is used during render`, + }); + } break; } case 'PropertyStore': @@ -2137,6 +2178,15 @@ function computeSignatureForInstruction( into: lvalue, }); } + if (value.children != null) { + // Children are typically called during render, not used as an event/effect callback + for (const child of value.children) { + effects.push({ + kind: 'Render', + place: child, + }); + } + } if (value.kind === 'JsxExpression') { if (value.tag.kind === 'Identifier') { // Tags are render function, by definition they're called during render @@ -2145,29 +2195,23 @@ function computeSignatureForInstruction( place: value.tag, }); } - if (value.children != null) { - // Children are typically called during render, not used as an event/effect callback - for (const child of value.children) { - effects.push({ - kind: 'Render', - place: child, - }); - } - } for (const prop of value.props) { - if ( - prop.kind === 'JsxAttribute' && - prop.place.identifier.type.kind === 'Function' && - (isJsxType(prop.place.identifier.type.return) || - (prop.place.identifier.type.return.kind === 'Phi' && - prop.place.identifier.type.return.operands.some(operand => - isJsxType(operand), - ))) - ) { - // Any props which return jsx are assumed to be called during render + const place = + prop.kind === 'JsxAttribute' ? prop.place : prop.argument; + if (isUseRefType(place.identifier)) { + continue; + } + if (place.identifier.type.kind === 'Function') { + if (isJsxOrJsxUnionType(place.identifier.type.return)) { + effects.push({ + kind: 'Render', + place, + }); + } + } else { effects.push({ kind: 'Render', - place: prop.place, + place, }); } } @@ -2203,6 +2247,11 @@ function computeSignatureForInstruction( value: ValueKind.Primitive, reason: ValueReason.Other, }); + effects.push({ + kind: 'ImmutableCapture', + from: value.value, + into: place, + }); } else if (patternItem.kind === 'Identifier') { effects.push({ kind: 'CreateFrom', @@ -2384,15 +2433,46 @@ function computeSignatureForInstruction( }); break; } + case 'BinaryExpression': { + effects.push({ + kind: 'Create', + into: lvalue, + value: ValueKind.Primitive, + reason: ValueReason.Other, + }); + effects.push({ + kind: 'ImmutableCapture', + into: lvalue, + from: value.left, + }); + effects.push({ + kind: 'ImmutableCapture', + into: lvalue, + from: value.right, + }); + break; + } + case 'UnaryExpression': { + effects.push({ + kind: 'Create', + into: lvalue, + value: ValueKind.Primitive, + reason: ValueReason.Other, + }); + effects.push({ + kind: 'ImmutableCapture', + into: lvalue, + from: value.value, + }); + break; + } case 'TaggedTemplateExpression': - case 'BinaryExpression': case 'Debugger': case 'JSXText': case 'MetaProperty': case 'Primitive': case 'RegExpLiteral': case 'TemplateLiteral': - case 'UnaryExpression': case 'UnsupportedNode': { effects.push({ kind: 'Create', @@ -2423,7 +2503,7 @@ function computeEffectsForLegacySignature( lvalue: Place, receiver: Place, args: Array, - loc: SourceLocation, + _loc: SourceLocation, ): Array { const returnValueReason = signature.returnValueReason ?? ValueReason.Other; const effects: Array = []; @@ -2436,20 +2516,18 @@ function computeEffectsForLegacySignature( if (signature.impure && state.env.config.validateNoImpureFunctionsInRender) { effects.push({ kind: 'Impure', - place: receiver, - error: CompilerDiagnostic.create({ - category: ErrorCategory.Purity, - reason: 'Cannot call impure function during render', - description: - (signature.canonicalName != null - ? `\`${signature.canonicalName}\` is an impure function. ` - : '') + - 'Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)', - }).withDetails({ - kind: 'error', - loc, - message: 'Cannot call impure function', - }), + into: lvalue, + category: ErrorCategory.Purity, + reason: 'Cannot access impure value during render', + description: + 'Calling an impure function can produce unstable results that update ' + + 'unpredictably when the component happens to re-render. ' + + '(https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)', + sourceMessage: + signature.canonicalName != null + ? `\`${signature.canonicalName}\` is an impure function.` + : 'This function is impure', + usageMessage: 'Cannot access impure value during render', }); } if (signature.knownIncompatible != null && state.env.enableValidations) { @@ -2748,7 +2826,23 @@ function computeEffectsForSignature( } break; } - case 'Impure': + case 'Impure': { + if (env.config.validateNoImpureFunctionsInRender) { + const values = substitutions.get(effect.into.identifier.id) ?? []; + for (const value of values) { + effects.push({ + kind: effect.kind, + into: value, + category: effect.category, + reason: effect.reason, + description: effect.description, + sourceMessage: effect.sourceMessage, + usageMessage: effect.usageMessage, + }); + } + } + break; + } case 'MutateFrozen': case 'MutateGlobal': { const values = substitutions.get(effect.place.identifier.id) ?? []; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts index 43148dc4c67..f0cab1345db 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -19,6 +19,7 @@ import { ValueReason, Place, isPrimitiveType, + isUseRefType, } from '../HIR/HIR'; import { eachInstructionLValue, @@ -28,6 +29,9 @@ import { import {assertExhaustive, getOrInsertWith} from '../Utils/utils'; import {Err, Ok, Result} from '../Utils/Result'; import {AliasingEffect, MutationReason} from './AliasingEffects'; +import {printIdentifier, printType} from '../HIR/PrintHIR'; + +const DEBUG = false; /** * This pass builds an abstract model of the heap and interprets the effects of the @@ -104,7 +108,6 @@ export function inferMutationAliasingRanges( reason: MutationReason | null; }> = []; const renders: Array<{index: number; place: Place}> = []; - let index = 0; const errors = new CompilerError(); @@ -197,14 +200,12 @@ export function inferMutationAliasingRanges( }); } else if ( effect.kind === 'MutateFrozen' || - effect.kind === 'MutateGlobal' || - effect.kind === 'Impure' + effect.kind === 'MutateGlobal' ) { errors.pushDiagnostic(effect.error); functionEffects.push(effect); } else if (effect.kind === 'Render') { renders.push({index: index++, place: effect.place}); - functionEffects.push(effect); } } } @@ -214,10 +215,6 @@ export function inferMutationAliasingRanges( state.assign(index, from, into); } } - if (block.terminal.kind === 'return') { - state.assign(index++, block.terminal.value, fn.returns); - } - if ( (block.terminal.kind === 'maybe-throw' || block.terminal.kind === 'return') && @@ -227,23 +224,31 @@ export function inferMutationAliasingRanges( if (effect.kind === 'Alias') { state.assign(index++, effect.from, effect.into); } else { - CompilerError.invariant(effect.kind === 'Freeze', { - reason: `Unexpected '${effect.kind}' effect for MaybeThrow terminal`, - description: null, - details: [ - { - kind: 'error', - loc: block.terminal.loc, - message: null, - }, - ], - }); + CompilerError.invariant( + effect.kind === 'Freeze' || effect.kind === 'Render', + { + reason: `Unexpected '${effect.kind}' effect for MaybeThrow terminal`, + description: null, + details: [ + { + kind: 'error', + loc: block.terminal.loc, + message: null, + }, + ], + }, + ); } } } } for (const mutation of mutations) { + if (DEBUG) { + console.log( + `[${mutation.index}] mutate ${printIdentifier(mutation.place.identifier)}`, + ); + } state.mutate( mutation.index, mutation.place.identifier, @@ -255,8 +260,16 @@ export function inferMutationAliasingRanges( errors, ); } + if (DEBUG) { + console.log(state.debug()); + } for (const render of renders) { - state.render(render.index, render.place.identifier, errors); + if (DEBUG) { + console.log( + `[${render.index}] render ${printIdentifier(render.place.identifier)}`, + ); + } + state.render(render.index, render.place, errors); } for (const param of [...fn.context, ...fn.params]) { const place = param.kind === 'Identifier' ? param : param.place; @@ -515,6 +528,13 @@ export function inferMutationAliasingRanges( const ignoredErrors = new CompilerError(); for (const param of [...fn.params, ...fn.context, fn.returns]) { const place = param.kind === 'Identifier' ? param : param.place; + const node = state.nodes.get(place.identifier); + if (node != null && node.render != null) { + functionEffects.push({ + kind: 'Render', + place: place, + }); + } tracked.push(place); } for (const into of tracked) { @@ -577,7 +597,6 @@ export function inferMutationAliasingRanges( function appendFunctionErrors(errors: CompilerError, fn: HIRFunction): void { for (const effect of fn.aliasingEffects ?? []) { switch (effect.kind) { - case 'Impure': case 'MutateFrozen': case 'MutateGlobal': { errors.pushDiagnostic(effect.error); @@ -612,10 +631,74 @@ type Node = { | {kind: 'Object'} | {kind: 'Phi'} | {kind: 'Function'; function: HIRFunction}; + render: Place | null; }; + +function _printNode(node: Node): string { + const out: Array = []; + debugNode(out, node); + return out.join('\n'); +} +function debugNode(out: Array, node: Node): void { + out.push( + printIdentifier(node.id) + + printType(node.id.type) + + ` lastMutated=[${node.lastMutated}]`, + ); + if (node.transitive != null) { + out.push(` transitive=${node.transitive.kind}`); + } + if (node.local != null) { + out.push(` local=${node.local.kind}`); + } + if (node.mutationReason != null) { + out.push(` mutationReason=${node.mutationReason?.kind}`); + } + const edges: Array<{ + index: number; + direction: '<=' | '=>'; + kind: string; + id: Identifier; + }> = []; + for (const [alias, index] of node.createdFrom) { + edges.push({index, direction: '<=', kind: 'createFrom', id: alias}); + } + for (const [alias, index] of node.aliases) { + edges.push({index, direction: '<=', kind: 'alias', id: alias}); + } + for (const [alias, index] of node.maybeAliases) { + edges.push({index, direction: '<=', kind: 'alias?', id: alias}); + } + for (const [alias, index] of node.captures) { + edges.push({index, direction: '<=', kind: 'capture', id: alias}); + } + for (const edge of node.edges) { + edges.push({ + index: edge.index, + direction: '=>', + kind: edge.kind, + id: edge.node, + }); + } + edges.sort((a, b) => a.index - b.index); + for (const edge of edges) { + out.push( + ` [${edge.index}] ${edge.direction} ${edge.kind} ${printIdentifier(edge.id)}`, + ); + } +} + class AliasingState { nodes: Map = new Map(); + debug(): string { + const items: Array = []; + for (const [_id, node] of this.nodes) { + debugNode(items, node); + } + return items.join('\n'); + } + create(place: Place, value: Node['value']): void { this.nodes.set(place.identifier, { id: place.identifier, @@ -629,6 +712,7 @@ class AliasingState { lastMutated: 0, mutationReason: null, value, + render: null, }); } @@ -681,9 +765,9 @@ class AliasingState { } } - render(index: number, start: Identifier, errors: CompilerError): void { + render(index: number, start: Place, errors: CompilerError): void { const seen = new Set(); - const queue: Array = [start]; + const queue: Array = [start.identifier]; while (queue.length !== 0) { const current = queue.pop()!; if (seen.has(current)) { @@ -691,11 +775,34 @@ class AliasingState { } seen.add(current); const node = this.nodes.get(current); - if (node == null || node.transitive != null || node.local != null) { + if (node == null || isUseRefType(node.id)) { + if (DEBUG) { + console.log(` render ${printIdentifier(current)}: skip mutated/ref`); + } continue; } - if (node.value.kind === 'Function') { - appendFunctionErrors(errors, node.value.function); + if ( + node.local == null && + node.transitive == null && + node.value.kind === 'Function' + ) { + const returns = node.value.function.returns; + if ( + isJsxType(returns.identifier.type) || + (returns.identifier.type.kind === 'Phi' && + returns.identifier.type.operands.some(operand => + isJsxType(operand), + )) + ) { + appendFunctionErrors(errors, node.value.function); + } + if (DEBUG) { + console.log(` render ${printIdentifier(current)}: skip function`); + } + continue; + } + if (node.render == null) { + node.render = start; } for (const [alias, when] of node.createdFrom) { if (when >= index) { @@ -709,6 +816,12 @@ class AliasingState { } queue.push(alias); } + for (const [alias, when] of node.maybeAliases) { + if (when >= index) { + continue; + } + queue.push(alias); + } for (const [capture, when] of node.captures) { if (when >= index) { continue; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts index 897614015f5..32f5d6f40f2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts @@ -167,6 +167,14 @@ export function Set_filter( return result; } +export function Set_subtract( + source: ReadonlySet, + other: Iterable, +): Set { + const otherSet = other instanceof Set ? other : new Set(other); + return Set_filter(source, item => !otherSet.has(item)); +} + export function hasNode( input: NodePath, ): input is NodePath> { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts deleted file mode 100644 index ca0612d80ce..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts +++ /dev/null @@ -1,59 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {CompilerDiagnostic, CompilerError} from '..'; -import {ErrorCategory} from '../CompilerError'; -import {HIRFunction} from '../HIR'; -import {getFunctionCallSignature} from '../Inference/InferMutationAliasingEffects'; -import {Result} from '../Utils/Result'; - -/** - * Checks that known-impure functions are not called during render. Examples of invalid functions to - * call during render are `Math.random()` and `Date.now()`. Users may extend this set of - * impure functions via a module type provider and specifying functions with `impure: true`. - * - * TODO: add best-effort analysis of functions which are called during render. We have variations of - * this in several of our validation passes and should unify those analyses into a reusable helper - * and use it here. - */ -export function validateNoImpureFunctionsInRender( - fn: HIRFunction, -): Result { - const errors = new CompilerError(); - for (const [, block] of fn.body.blocks) { - for (const instr of block.instructions) { - const value = instr.value; - if (value.kind === 'MethodCall' || value.kind == 'CallExpression') { - const callee = - value.kind === 'MethodCall' ? value.property : value.callee; - const signature = getFunctionCallSignature( - fn.env, - callee.identifier.type, - ); - if (signature != null && signature.impure === true) { - errors.pushDiagnostic( - CompilerDiagnostic.create({ - category: ErrorCategory.Purity, - reason: 'Cannot call impure function during render', - description: - (signature.canonicalName != null - ? `\`${signature.canonicalName}\` is an impure function. ` - : '') + - 'Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)', - suggestions: null, - }).withDetails({ - kind: 'error', - loc: callee.loc, - message: 'Cannot call impure function', - }), - ); - } - } - } - } - return errors.asResult(); -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureValuesInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureValuesInRender.ts new file mode 100644 index 00000000000..3db4dd9ab4c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureValuesInRender.ts @@ -0,0 +1,307 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import prettyFormat from 'pretty-format'; +import {CompilerDiagnostic, CompilerError, Effect} from '..'; +import { + areEqualSourceLocations, + HIRFunction, + IdentifierId, + InstructionId, + isJsxType, + isRefValueType, + isUseRefType, +} from '../HIR'; +import { + eachInstructionLValue, + eachInstructionValueOperand, +} from '../HIR/visitors'; +import {AliasingEffect} from '../Inference/AliasingEffects'; +import {createControlDominators} from '../Inference/ControlDominators'; +import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; +import {Err, Ok, Result} from '../Utils/Result'; +import { + assertExhaustive, + getOrInsertWith, + Set_filter, + Set_subtract, +} from '../Utils/utils'; +import {printInstruction} from '../HIR/PrintHIR'; + +type ImpureEffect = Extract; +type RenderEffect = Extract; +type FunctionCache = Map>; +type ImpuritySignature = {effects: Array; error: CompilerError}; + +export function validateNoImpureValuesInRender( + fn: HIRFunction, +): Result { + const impure = new Map(); + const result = inferImpureValues(fn, impure, new Map()); + + if (result.error.hasAnyErrors()) { + return Err(result.error); + } + return Ok(undefined); +} + +function inferFunctionExpressionMemo( + fn: HIRFunction, + impure: Map, + cache: FunctionCache, +): ImpuritySignature { + const key = fn.context + .map(place => `${place.identifier.id}:${impure.has(place.identifier.id)}`) + .join(','); + return getOrInsertWith( + getOrInsertWith(cache, fn, () => new Map()), + key, + () => inferImpureValues(fn, impure, cache), + ); +} + +function processEffects( + id: InstructionId, + effects: Array, + impure: Map, + cache: FunctionCache, +): boolean { + let hasChanges = false; + const rendered: Set = new Set(); + for (const effect of effects) { + if (effect.kind === 'Render') { + rendered.add(effect.place.identifier.id); + } + } + for (const effect of effects) { + switch (effect.kind) { + case 'Alias': + case 'Assign': + case 'Capture': + case 'CreateFrom': + case 'ImmutableCapture': { + const sourceEffect = impure.get(effect.from.identifier.id); + if ( + sourceEffect != null && + !impure.has(effect.into.identifier.id) && + !rendered.has(effect.from.identifier.id) && + !isUseRefType(effect.into.identifier) && + !isJsxType(effect.into.identifier.type) + ) { + impure.set(effect.into.identifier.id, sourceEffect); + hasChanges = true; + } + if ( + sourceEffect == null && + (effect.kind === 'Assign' || effect.kind === 'Capture') && + !impure.has(effect.from.identifier.id) && + !rendered.has(effect.from.identifier.id) && + !isUseRefType(effect.from.identifier) && + isMutable({id}, effect.into) + ) { + const destinationEffect = impure.get(effect.into.identifier.id); + if (destinationEffect != null) { + impure.set(effect.from.identifier.id, destinationEffect); + hasChanges = true; + } + } + break; + } + case 'Impure': { + if (!impure.has(effect.into.identifier.id)) { + impure.set(effect.into.identifier.id, effect); + hasChanges = true; + } + break; + } + case 'Render': { + break; + } + case 'CreateFunction': { + const result = inferFunctionExpressionMemo( + effect.function.loweredFunc.func, + impure, + cache, + ); + if (result.error.hasAnyErrors()) { + break; + } + const impureEffect: ImpureEffect | null = + result.effects.find( + (functionEffect: AliasingEffect): functionEffect is ImpureEffect => + functionEffect.kind === 'Impure' && + functionEffect.into.identifier.id === + effect.function.loweredFunc.func.returns.identifier.id, + ) ?? null; + if (impureEffect != null) { + impure.set(effect.into.identifier.id, impureEffect); + hasChanges = true; + } + break; + } + case 'MaybeAlias': + case 'Apply': + case 'Create': + case 'Freeze': + case 'Mutate': + case 'MutateConditionally': + case 'MutateFrozen': + case 'MutateGlobal': + case 'MutateTransitive': + case 'MutateTransitiveConditionally': { + break; + } + } + } + return hasChanges; +} + +function inferImpureValues( + fn: HIRFunction, + impure: Map, + cache: FunctionCache, +): ImpuritySignature { + const getBlockControl = createControlDominators(fn, place => { + return impure.has(place.identifier.id); + }); + + let hasChanges = false; + do { + hasChanges = false; + + for (const block of fn.body.blocks.values()) { + const controlPlace = getBlockControl(block.id); + const controlImpureEffect = + controlPlace != null ? impure.get(controlPlace.identifier.id) : null; + + for (const phi of block.phis) { + if (impure.has(phi.place.identifier.id)) { + // Already marked impure on a previous pass + continue; + } + let impureEffect = null; + for (const [, operand] of phi.operands) { + const operandEffect = impure.get(operand.identifier.id); + if (operandEffect != null) { + impureEffect = operandEffect; + break; + } + } + if (impureEffect != null) { + impure.set(phi.place.identifier.id, impureEffect); + hasChanges = true; + } else { + for (const [pred] of phi.operands) { + const predControl = getBlockControl(pred); + if (predControl != null) { + const predEffect = impure.get(predControl.identifier.id); + if (predEffect != null) { + impure.set(phi.place.identifier.id, predEffect); + hasChanges = true; + break; + } + } + } + } + } + + for (const instr of block.instructions) { + const _impure = new Set(impure.keys()); + hasChanges = + processEffects(instr.id, instr.effects ?? [], impure, cache) || + hasChanges; + } + if (block.terminal.kind === 'return' && block.terminal.effects != null) { + hasChanges = + processEffects( + block.terminal.id, + block.terminal.effects, + impure, + cache, + ) || hasChanges; + } + } + } while (hasChanges); + + fn.env.logger?.debugLogIRs?.({ + kind: 'debug', + name: 'ValidateNoImpureValuesInRender', + value: JSON.stringify(Array.from(impure.keys()).sort(), null, 2), + }); + + const error = new CompilerError(); + function validateRenderEffect(effect: RenderEffect): void { + const impureEffect = impure.get(effect.place.identifier.id); + if (impureEffect == null) { + return; + } + const diagnostic = CompilerDiagnostic.create({ + category: impureEffect.category, + reason: impureEffect.reason, + description: impureEffect.description, + }).withDetails({ + kind: 'error', + loc: effect.place.loc, + message: impureEffect.usageMessage, + }); + if (!areEqualSourceLocations(effect.place.loc, impureEffect.into.loc)) { + diagnostic.withDetails({ + kind: 'error', + loc: impureEffect.into.loc, + message: impureEffect.sourceMessage, + }); + } + error.pushDiagnostic(diagnostic); + } + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const value = instr.value; + if ( + value.kind === 'FunctionExpression' || + value.kind === 'ObjectMethod' + ) { + const result = inferFunctionExpressionMemo( + value.loweredFunc.func, + impure, + cache, + ); + if (result.error.hasAnyErrors()) { + error.merge(result.error); + } + } + for (const effect of instr.effects ?? []) { + if (effect.kind === 'Render') { + validateRenderEffect(effect); + } + } + } + if (block.terminal.kind === 'return' && block.terminal.effects != null) { + for (const effect of block.terminal.effects) { + if (effect.kind === 'Render') { + validateRenderEffect(effect); + } + } + } + } + const impureEffects: Array = []; + for (const param of [...fn.context, ...fn.params, fn.returns]) { + const place = param.kind === 'Identifier' ? param : param.place; + const impureEffect = impure.get(place.identifier.id); + if (impureEffect != null) { + impureEffects.push({ + kind: 'Impure', + into: impureEffect.into, + category: impureEffect.category, + reason: impureEffect.reason, + description: impureEffect.description, + sourceMessage: impureEffect.sourceMessage, + usageMessage: impureEffect.usageMessage, + }); + } + } + return {effects: impureEffects, error}; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts index 232e9f55bbc..f49955c4793 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts @@ -5,920 +5,425 @@ * LICENSE file in the root directory of this source tree. */ -import { - CompilerDiagnostic, - CompilerError, - ErrorCategory, -} from '../CompilerError'; +import {CompilerDiagnostic, CompilerError, ErrorCategory} from '../CompilerError'; import { BlockId, HIRFunction, IdentifierId, - Identifier, + Instruction, Place, - SourceLocation, - getHookKindForType, isRefValueType, isUseRefType, } from '../HIR'; -import {BuiltInEventHandlerId} from '../HIR/ObjectShape'; -import { - eachInstructionOperand, - eachInstructionValueOperand, - eachPatternOperand, - eachTerminalOperand, -} from '../HIR/visitors'; +import {eachTerminalSuccessor} from '../HIR/visitors'; import {Err, Ok, Result} from '../Utils/Result'; -import {retainWhere} from '../Utils/utils'; /** - * Validates that a function does not access a ref value during render. This includes a partial check - * for ref values which are accessed indirectly via function expressions. + * Validates that a function does not mutate a ref value during render. + * Reading refs is handled by a separate validation pass. + * + * Mutation is allowed in: + * - Event handlers and effect callbacks (functions not called at top level) + * - Inside `if (ref.current == null)` blocks (null-guard initialization pattern) * * ```javascript - * // ERROR - * const ref = useRef(); - * ref.current; + * // ERROR - direct mutation in render + * const ref = useRef(null); + * ref.current = value; * - * const ref = useRef(); - * foo(ref); // may access .current + * // ERROR - mutation in function called during render + * const fn = () => { ref.current = value; }; + * fn(); // fn is called, so mutation errors * - * // ALLOWED - * const ref = useHookThatReturnsRef(); - * ref.current; - * ``` + * // ALLOWED - mutation in event handler + * const onClick = () => { ref.current = value; }; + * return + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ref value is used during render + 20 | + 21 | ); + 22 | } + error.ref-value-in-event-handler-wrapper.ts:19:35 17 | <> 18 | > 19 | - | ^^^^^^^^^^^ Cannot access ref value during render + | ^^^^^^^^^^^ Ref is initially accessed 20 | 21 | ); 22 | } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md index a0c492120a3..020383ff06f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md @@ -33,15 +33,15 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Cannot access refs during render +Error: Mutating refs during render is not allowed -React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). +React refs are mutable containers that should only be mutated outside of render, such as in event handlers or effects. Mutating a ref during render can cause bugs because the mutation may not be associated with a particular render. See https://react.dev/reference/react/useRef. error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.ts:14:2 12 | 13 | // The ref is modified later, extending its range and preventing memoization of onChange > 14 | ref.current.inner = null; - | ^^^^^^^^^^^ Cannot update ref during render + | ^^^^^^^^^^^ Cannot mutate ref during render 15 | 16 | return ; 17 | } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md index ba5a7407773..16a4fcf4f5b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md @@ -36,18 +36,18 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Cannot access refs during render +Error: Mutating refs during render is not allowed -React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). +React refs are mutable containers that should only be mutated outside of render, such as in event handlers or effects. Mutating a ref during render can cause bugs because the mutation may not be associated with a particular render. See https://react.dev/reference/react/useRef. -error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.ts:17:2 - 15 | ref.current.inner = null; +error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.ts:15:4 + 13 | // The ref is modified later, extending its range and preventing memoization of onChange + 14 | const reset = () => { +> 15 | ref.current.inner = null; + | ^^^^^^^^^^^ Cannot mutate ref during render 16 | }; -> 17 | reset(); - | ^^^^^ This function accesses a ref value + 17 | reset(); 18 | - 19 | return ; - 20 | } ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-set-ref-nested-property-dont-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-set-ref-nested-property-dont-preserve-memoization.expect.md index b40b0bbf226..93bd4c27d34 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-set-ref-nested-property-dont-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-set-ref-nested-property-dont-preserve-memoization.expect.md @@ -32,15 +32,15 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Cannot access refs during render +Error: Mutating refs during render is not allowed -React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). +React refs are mutable containers that should only be mutated outside of render, such as in event handlers or effects. Mutating a ref during render can cause bugs because the mutation may not be associated with a particular render. See https://react.dev/reference/react/useRef. error.useCallback-set-ref-nested-property-dont-preserve-memoization.ts:13:2 11 | }); 12 | > 13 | ref.current.inner = null; - | ^^^^^^^^^^^ Cannot update ref during render + | ^^^^^^^^^^^ Cannot mutate ref during render 14 | 15 | return ; 16 | } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md index 2c864f56aff..d5674691bb9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateExhaustiveMemoizationDependencies +// @validateExhaustiveMemoizationDependencies @validateRefAccessDuringRender:false import {useMemo} from 'react'; import {Stringify} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.js index c0f8d28837a..feba85da7d2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.js @@ -1,4 +1,4 @@ -// @validateExhaustiveMemoizationDependencies +// @validateExhaustiveMemoizationDependencies @validateRefAccessDuringRender:false import {useMemo} from 'react'; import {Stringify} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md index ea5a887b8bf..5e754fc8f5e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md @@ -47,7 +47,7 @@ export const FIXTURE_ENTRYPOINT = { ## Logs ``` -{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"category":"Refs","reason":"Cannot access refs during render","description":"React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":289},"end":{"line":9,"column":16,"index":303},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot update ref during render"}]}}} +{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"category":"Refs","reason":"Mutating refs during render is not allowed","description":"React refs are mutable containers that should only be mutated outside of render, such as in event handlers or effects. Mutating a ref during render can cause bugs because the mutation may not be associated with a particular render. See https://react.dev/reference/react/useRef","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":289},"end":{"line":9,"column":16,"index":303},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot mutate ref during render"}]}}} {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":237},"end":{"line":8,"column":50,"index":285},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":259},"end":{"line":8,"column":30,"index":265},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]} {"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-impure-functions-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-impure-functions-in-render.expect.md index 1241971d827..aabc8d2baea 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-impure-functions-in-render.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-impure-functions-in-render.expect.md @@ -19,41 +19,65 @@ function Component() { ``` Found 3 errors: -Error: Cannot call impure function during render +Error: Cannot access impure value during render -`Date.now` is an impure function. Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). +Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). + +error.invalid-impure-functions-in-render.ts:7:20 + 5 | const now = performance.now(); + 6 | const rand = Math.random(); +> 7 | return ; + | ^^^^ Cannot access impure value during render + 8 | } + 9 | error.invalid-impure-functions-in-render.ts:4:15 2 | 3 | function Component() { > 4 | const date = Date.now(); - | ^^^^^^^^^^ Cannot call impure function + | ^^^^^^^^^^ `Date.now` is an impure function. 5 | const now = performance.now(); 6 | const rand = Math.random(); 7 | return ; -Error: Cannot call impure function during render +Error: Cannot access impure value during render -`performance.now` is an impure function. Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). +Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). + +error.invalid-impure-functions-in-render.ts:7:31 + 5 | const now = performance.now(); + 6 | const rand = Math.random(); +> 7 | return ; + | ^^^ Cannot access impure value during render + 8 | } + 9 | error.invalid-impure-functions-in-render.ts:5:14 3 | function Component() { 4 | const date = Date.now(); > 5 | const now = performance.now(); - | ^^^^^^^^^^^^^^^^^ Cannot call impure function + | ^^^^^^^^^^^^^^^^^ `performance.now` is an impure function. 6 | const rand = Math.random(); 7 | return ; 8 | } -Error: Cannot call impure function during render +Error: Cannot access impure value during render -`Math.random` is an impure function. Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). +Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). + +error.invalid-impure-functions-in-render.ts:7:42 + 5 | const now = performance.now(); + 6 | const rand = Math.random(); +> 7 | return ; + | ^^^^ Cannot access impure value during render + 8 | } + 9 | error.invalid-impure-functions-in-render.ts:6:15 4 | const date = Date.now(); 5 | const now = performance.now(); > 6 | const rand = Math.random(); - | ^^^^^^^^^^^^^ Cannot call impure function + | ^^^^^^^^^^^^^ `Math.random` is an impure function. 7 | return ; 8 | } 9 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/mutate-after-useeffect-ref-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/mutate-after-useeffect-ref-access.expect.md index acf9c28cabd..a96c859c660 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/mutate-after-useeffect-ref-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/mutate-after-useeffect-ref-access.expect.md @@ -47,7 +47,7 @@ export const FIXTURE_ENTRYPOINT = { ## Logs ``` -{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":190},"end":{"line":11,"column":1,"index":363},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"category":"Refs","reason":"Cannot access refs during render","description":"React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":321},"end":{"line":9,"column":16,"index":335},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot update ref during render"}]}}} +{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":190},"end":{"line":11,"column":1,"index":363},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"category":"Refs","reason":"Mutating refs during render is not allowed","description":"React refs are mutable containers that should only be mutated outside of render, such as in event handlers or effects. Mutating a ref during render can cause bugs because the mutation may not be associated with a particular render. See https://react.dev/reference/react/useRef","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":321},"end":{"line":9,"column":16,"index":335},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot mutate ref during render"}]}}} {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":269},"end":{"line":8,"column":50,"index":317},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":291},"end":{"line":8,"column":30,"index":297},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]} {"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":190},"end":{"line":11,"column":1,"index":363},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.expect.md index ed1dfa39ea5..b5b58ab04c6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.expect.md @@ -6,7 +6,7 @@ import {useRef} from 'react'; -component Foo() { +hook useFoo() { const ref = useRef(); const s = () => { @@ -16,6 +16,10 @@ component Foo() { return s; } +component Foo() { + useFoo(); +} + export const FIXTURE_ENTRYPOINT = { fn: Foo, params: [], @@ -30,7 +34,7 @@ import { c as _c } from "react/compiler-runtime"; import { useRef } from "react"; -function Foo() { +function useFoo() { const $ = _c(1); const ref = useRef(); let t0; @@ -44,6 +48,10 @@ function Foo() { return s; } +function Foo() { + useFoo(); +} + export const FIXTURE_ENTRYPOINT = { fn: Foo, params: [], @@ -52,4 +60,4 @@ export const FIXTURE_ENTRYPOINT = { ``` ### Eval output -(kind: ok) "[[ function params=0 ]]" \ No newline at end of file +(kind: ok) \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.js index f1a45ebc4ff..502e7f42fcb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.js @@ -2,7 +2,7 @@ import {useRef} from 'react'; -component Foo() { +hook useFoo() { const ref = useRef(); const s = () => { @@ -12,6 +12,10 @@ component Foo() { return s; } +component Foo() { + useFoo(); +} + export const FIXTURE_ENTRYPOINT = { fn: Foo, params: [], diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md index b55526e9211..5e0efa3aa45 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md @@ -25,22 +25,40 @@ component Component(prop1, ref) { ## Code ```javascript -import { useFire } from "react/compiler-runtime"; +import { c as _c, useFire } from "react/compiler-runtime"; import { fire } from "react"; import { print } from "shared-runtime"; const Component = React.forwardRef(Component_withRef); function Component_withRef(t0, ref) { + const $ = _c(5); const { prop1 } = t0; - const foo = () => { - console.log(prop1); - }; - const t1 = useFire(foo); - useEffect(() => { - t1(prop1); - bar(); - t1(); - }); + let t1; + if ($[0] !== prop1) { + t1 = () => { + console.log(prop1); + }; + $[0] = prop1; + $[1] = t1; + } else { + t1 = $[1]; + } + const foo = t1; + const t2 = useFire(foo); + let t3; + if ($[2] !== prop1 || $[3] !== t2) { + t3 = () => { + t2(prop1); + bar(); + t2(); + }; + $[2] = prop1; + $[3] = t2; + $[4] = t3; + } else { + t3 = $[4]; + } + useEffect(t3); print(ref.current); return null; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-use-impure-value-in-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-use-impure-value-in-ref.expect.md new file mode 100644 index 00000000000..6e0bf7d018f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-use-impure-value-in-ref.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @validateNoImpureFunctionsInRender +import {useIdentity} from 'shared-runtime'; + +function Component() { + const f = () => Math.random(); + const ref = useRef(f()); + return
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoImpureFunctionsInRender +import { useIdentity } from "shared-runtime"; + +function Component() { + const $ = _c(2); + const f = _temp; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = f(); + $[0] = t0; + } else { + t0 = $[0]; + } + const ref = useRef(t0); + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 =
; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} +function _temp() { + return Math.random(); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-use-impure-value-in-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-use-impure-value-in-ref.js new file mode 100644 index 00000000000..4002865548a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-use-impure-value-in-ref.js @@ -0,0 +1,8 @@ +// @validateNoImpureFunctionsInRender +import {useIdentity} from 'shared-runtime'; + +function Component() { + const f = () => Math.random(); + const ref = useRef(f()); + return
; +} diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts index f89b049d100..aa27b28822b 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts @@ -29,9 +29,9 @@ testRule( } `, errors: [ - makeTestCaseError('Cannot call impure function during render'), - makeTestCaseError('Cannot call impure function during render'), - makeTestCaseError('Cannot call impure function during render'), + makeTestCaseError('Cannot access impure value during render'), + makeTestCaseError('Cannot access impure value during render'), + makeTestCaseError('Cannot access impure value during render'), ], }, ],