diff --git a/compiler/.claude/settings.local.json b/compiler/.claude/settings.local.json index 9bbd18802e4..cedea420a7d 100644 --- a/compiler/.claude/settings.local.json +++ b/compiler/.claude/settings.local.json @@ -1,7 +1,15 @@ { "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:*)", + "Bash(yarn workspace snap run build:*)" ], "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..65640ea2369 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), }), ], [ @@ -693,6 +822,22 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useRef', returnValueKind: ValueKind.Mutable, + aliasing: { + receiver: '@receiver', + params: [], + rest: '@rest', + returns: '@return', + temporaries: [], + effects: [ + { + kind: 'Create', + into: '@return', + value: ValueKind.Mutable, + reason: ValueReason.KnownReturnSignature, + }, + {kind: 'Capture', from: '@rest', into: '@return'}, + ], + }, }), ], [ @@ -715,6 +860,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useMemo', returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -722,10 +868,16 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ addHook(DEFAULT_SHAPES, { positionalParams: [], restParam: Effect.Freeze, - returnType: {kind: 'Poly'}, + returnType: { + kind: 'Function', + isConstructor: false, + return: {kind: 'Poly'}, + shapeId: null, + }, calleeEffect: Effect.Read, hookKind: 'useCallback', returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -739,41 +891,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 +907,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useLayoutEffect', returnValueKind: ValueKind.Frozen, + aliasing: EffectHookAliasing, }, BuiltInUseLayoutEffectHookId, ), @@ -804,6 +923,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useInsertionEffect', returnValueKind: ValueKind.Frozen, + aliasing: EffectHookAliasing, }, BuiltInUseInsertionEffectHookId, ), @@ -817,6 +937,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: 'useTransition', returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -829,6 +950,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ hookKind: 'useOptimistic', returnValueKind: ValueKind.Frozen, returnValueReason: ValueReason.State, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }), ], [ @@ -842,6 +964,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ returnType: {kind: 'Poly'}, calleeEffect: Effect.Read, returnValueKind: ValueKind.Frozen, + aliasing: RenderHookAliasing(ValueReason.HookCaptured), }, BuiltInUseOperatorId, ), @@ -866,27 +989,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..90b259e941a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -983,15 +983,15 @@ export function printAliasingEffect(effect: AliasingEffect): string { return `...${printPlaceForAliasEffect(arg.place)}`; }) .join(', '); - let signature = ''; - if (effect.signature != null) { - if (effect.signature.aliasing != null) { - signature = printAliasingSignature(effect.signature.aliasing); - } else { - signature = JSON.stringify(effect.signature, null, 2); - } - } - return `Apply ${printPlaceForAliasEffect(effect.into)} = ${receiverCallee}(${args})${signature != '' ? '\n ' : ''}${signature}`; + // let signature = ''; + // if (effect.signature != null) { + // if (effect.signature.aliasing != null) { + // signature = printAliasingSignature(effect.signature.aliasing); + // } else { + // signature = JSON.stringify(effect.signature, null, 2); + // } + // } + return `Apply ${printPlaceForAliasEffect(effect.into)} = ${receiverCallee}(${args})`; } case 'Freeze': { return `Freeze ${printPlaceForAliasEffect(effect.value)} ${effect.reason}`; @@ -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..b9c033a491e 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: { @@ -1072,6 +1094,8 @@ function applyEffect( break; } case 'Apply': { + effects.push(effect); + const functionValues = state.values(effect.function); if ( functionValues.length === 1 && @@ -1966,6 +1990,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 +2002,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 +2180,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 +2197,17 @@ 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') { + // Functions are checked independently effects.push({ kind: 'Render', - place: prop.place, + place, }); } } @@ -2203,6 +2243,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 +2429,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 +2499,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 +2512,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 +2822,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..9425c9a8bfd 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,8 @@ import { ValueReason, Place, isPrimitiveType, + isUseRefType, + isJsxOrJsxUnionType, } from '../HIR/HIR'; import { eachInstructionLValue, @@ -28,6 +30,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 +109,6 @@ export function inferMutationAliasingRanges( reason: MutationReason | null; }> = []; const renders: Array<{index: number; place: Place}> = []; - let index = 0; const errors = new CompilerError(); @@ -197,14 +201,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 +216,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 +225,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 +261,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; @@ -383,17 +397,7 @@ export function inferMutationAliasingRanges( break; } case 'Apply': { - CompilerError.invariant(false, { - reason: `[AnalyzeFunctions] Expected Apply effects to be replaced with more precise effects`, - description: null, - details: [ - { - kind: 'error', - loc: effect.function.loc, - message: null, - }, - ], - }); + break; } case 'MutateTransitive': case 'MutateConditionally': @@ -515,6 +519,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) { @@ -568,7 +579,12 @@ export function inferMutationAliasingRanges( } } - if (errors.hasAnyErrors() && !isFunctionExpression) { + if ( + errors.hasAnyErrors() && + (fn.fnType === 'Component' || + isJsxOrJsxUnionType(fn.returns.identifier.type) || + !isFunctionExpression) + ) { return Err(errors); } return Ok(functionEffects); @@ -577,7 +593,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 +627,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 +708,7 @@ class AliasingState { lastMutated: 0, mutationReason: null, value, + render: null, }); } @@ -681,9 +761,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 +771,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 +812,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..c53b6cb167a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureValuesInRender.ts @@ -0,0 +1,387 @@ +/** + * 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, Effect} from '..'; +import { + areEqualSourceLocations, + HIRFunction, + IdentifierId, + InstructionId, + isJsxType, + isUseRefType, +} from '../HIR'; +import {AliasingEffect, hashEffect} from '../Inference/AliasingEffects'; +import {createControlDominators} from '../Inference/ControlDominators'; +import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; +import {Err, Ok, Result} from '../Utils/Result'; +import {getOrInsertWith} from '../Utils/utils'; +import {printFunction} from '../HIR/PrintHIR'; + +type ImpureEffect = Extract; +type RenderEffect = Extract; +type FunctionCache = Map>; +type ImpuritySignature = { + effects: Map; + error: CompilerError; + returns: IdentifierId; +}; + +export function validateNoImpureValuesInRender( + fn: HIRFunction, +): Result { + const impure = new Map(); + const impureFunctions = new Map(); + const result = inferImpureValues(fn, impure, impureFunctions, new Map()); + + if (result.error.hasAnyErrors()) { + return Err(result.error); + } + return Ok(undefined); +} + +function inferFunctionExpressionMemo( + fn: HIRFunction, + impure: Map, + impureFunctions: Map, + cache: FunctionCache, +): ImpuritySignature { + const key = fn.context + .map( + place => + `${place.identifier.id}:${impure.has(place.identifier.id)}:${Array.from( + impureFunctions.get(place.identifier.id)?.effects ?? new Map(), + ) + .map(([id, effect]) => `${id}=>${effect.into.identifier.id}`) + .join(',')}`, + ) + .join(','); + return getOrInsertWith( + getOrInsertWith(cache, fn, () => new Map()), + key, + () => inferImpureValues(fn, impure, impureFunctions, cache), + ); +} + +function processEffects( + id: InstructionId, + effects: Array, + impure: Map, + impureFunctions: 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) + ) { + // console.log( + // `${effect.kind} $${effect.into.identifier.id} <= $${effect.from.identifier.id} ($${sourceEffect.into.identifier.id} forward)`, + // ); + 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) { + // console.log( + // `${effect.kind} $${effect.into.identifier.id} => $${effect.from.identifier.id} ($${destinationEffect.into.identifier.id} backward)`, + // ); + impure.set(effect.from.identifier.id, destinationEffect); + hasChanges = true; + } + } + if ( + (effect.kind === 'Alias' || + effect.kind === 'Assign' || + effect.kind === 'ImmutableCapture') && + !rendered.has(effect.into.identifier.id) && + !isJsxType(effect.into.identifier.type) + ) { + const functionEffect = impureFunctions.get(effect.from.identifier.id); + if ( + functionEffect != null && + !impureFunctions.has(effect.into.identifier.id) + // || + // !areEqualFunctionSignatures( + // impureFunctions.get(effect.into.identifier.id)!.effects, + // functionEffect.effects, + // ) + ) { + // console.log( + // `${effect.kind} $${effect.into.identifier.id} <= $${effect.from.identifier.id} (function)`, + // ); + impureFunctions.set(effect.into.identifier.id, functionEffect); + hasChanges = true; + } + } + break; + } + case 'Impure': { + if (!impure.has(effect.into.identifier.id)) { + // console.log(`Impure $${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, + impureFunctions, + cache, + ); + if (result.error.hasAnyErrors()) { + break; + } + const previousResult = impureFunctions.get(effect.into.identifier.id); + if ( + previousResult == null || + !areEqualFunctionSignatures(result.effects, previousResult.effects) + ) { + // console.log(`Function $${effect.into.identifier.id}`); + impureFunctions.set(effect.into.identifier.id, result); + hasChanges = true; + } + break; + } + case 'Apply': { + const functionSignature = impureFunctions.get( + effect.function.identifier.id, + ); + if (functionSignature != null) { + for (const [id, functionEffect] of functionSignature.effects) { + if (!impure.has(id)) { + impure.set(id, functionEffect); + hasChanges = true; + } + if ( + id === functionSignature.returns && + !impure.has(effect.into.identifier.id) + ) { + impure.set(effect.into.identifier.id, functionEffect); + hasChanges = true; + } + } + } + break; + } + case 'MaybeAlias': + 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, + impureFunctions: Map, + cache: FunctionCache, +): ImpuritySignature { + const getBlockControl = createControlDominators(fn, place => { + return impure.has(place.identifier.id); + }); + + let hasChanges = false; + let iterations = 0; + do { + hasChanges = false; + + if (iterations++ > 100) { + throw new Error('too many iterations'); + } + + 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, + impureFunctions, + cache, + ) || hasChanges; + } + if (block.terminal.kind === 'return' && block.terminal.effects != null) { + hasChanges = + processEffects( + block.terminal.id, + block.terminal.effects, + impure, + impureFunctions, + cache, + ) || hasChanges; + } + } + } while (hasChanges); + + fn.env.logger?.debugLogIRs?.({ + kind: 'debug', + name: 'ValidateNoImpureValuesInRender', + value: JSON.stringify(Array.from(impure.keys()).sort(), null, 2), + }); + fn.env.logger?.debugLogIRs?.({ + kind: 'debug', + name: 'ValidateNoImpureValuesInRender (function)', + value: JSON.stringify(Array.from(impureFunctions.keys()).sort(), null, 2), + }); + + const error = new CompilerError(); + function validateRenderEffect(effect: RenderEffect): void { + let impureEffect = impure.get(effect.place.identifier.id); + if (impureEffect == null) { + const functionSignature = impureFunctions.get(effect.place.identifier.id); + impureEffect = functionSignature?.effects.get(functionSignature.returns); + } + 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, + impureFunctions, + 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: Map = new Map(); + 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.set(place.identifier.id, impureEffect); + } + } + return {effects: impureEffects, error, returns: fn.returns.identifier.id}; +} + +function areEqualFunctionSignatures( + sig1: Map, + sig2: Map, +): boolean { + return ( + sig1.size === sig2.size && + Array.from(sig1).every( + ([id, effect]) => + sig2.has(id) && hashEffect(effect) === hashEffect(sig2.get(id)!), + ) + ); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.md b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.md new file mode 100644 index 00000000000..653a438d653 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.md @@ -0,0 +1,137 @@ +# ValidateNoRefAccessInRender + +This document summarizes the design and key learnings for the ref mutation validation pass. + +## Purpose + +Validates that a function does not mutate a ref value during render. This ensures React components follow the rules of React by not writing to `ref.current` during the render phase. + +## Key Concepts + +### Ref vs RefValue + +- **Ref**: The ref object itself (e.g., `useRef()` return value). Has type `React.RefObject`. +- **RefValue**: The `.current` property of a ref. This is the mutable value that should not be accessed during render. + +The validation tracks both using a `RefInfo` type with a `refId` that correlates refs with their `.current` values. + +### What Constitutes a Mutation + +A mutation is any `PropertyStore` or `ComputedStore` instruction where: +1. The target object is a known ref (tracked in the `refs` map) +2. OR the target object has a ref type (`isUseRefType`) + +### Allowed Patterns + +1. **Event handlers and effect callbacks**: Functions that are not called at the top level during render can mutate refs freely. + +2. **Null-guard initialization**: The pattern `if (ref.current == null) { ref.current = value; }` is allowed because it's a common lazy initialization pattern that only runs once. + +## Algorithm: Single Forward Data-Flow Pass + +The validation uses a single forward pass over all blocks: + +### Phase 1: Track Refs +- Initialize refs from function params and context (captured variables) +- Process phi nodes to propagate ref info through control flow joins +- Track refs through LoadLocal, StoreLocal, PropertyLoad operations + +### Phase 2: Detect Null Guards +- Track nullable values (null literals, undefined) +- Track binary comparisons of `ref.current` to null (`==`, `===`, `!=`, `!==`) +- Mark blocks as "safe" for specific refs when inside null-guard branches +- Propagate safety through control flow until fallthrough + +### Phase 3: Validate Mutations +- For PropertyStore/ComputedStore on refs: + - If inside a null-guard for this ref: allow (but track for duplicate detection) + - If at top level: error immediately + - If in nested function: track for later (error if function is called) + +### Phase 4: Track Ref-Mutating Functions +- When a FunctionExpression mutates a ref, track it in `refMutatingFunctions` +- When such a function is called at top level, report the error at the mutation site + +## Key Data Structures + +```typescript +// Correlates refs with their .current values +type RefInfo = { + kind: 'Ref' | 'RefValue'; + refId: number; +}; + +// Tracks null-guard conditions +type GuardInfo = { + refId: number; + isEquality: boolean; // true for ==, ===; false for !=, !== +}; + +// Information about a mutation (for error reporting) +type MutationInfo = { + loc: SourceLocation; + isCurrentProperty: boolean; +}; +``` + +## Error Reporting + +### Error Location + +Errors highlight the **entire instruction** (e.g., `ref.current = value`), not just the ref identifier. This is achieved by using `instr.loc` instead of `value.object.loc`. + +### Duplicate Initialization + +When a ref is initialized more than once inside a null-guard: +1. Primary error: Points to the second initialization +2. Secondary error: Points to the first initialization with "Ref was first initialized here" + +### Transitive Mutations + +When a function that mutates refs is called during render: +- The error points to the mutation site inside the function +- Not the call site (the call site is what triggers the check) + +## Edge Cases and Patterns + +### Unary NOT on Guards + +The `!` operator inverts guard polarity: +```javascript +if (!ref.current) { ... } // Same as: if (ref.current == null) +``` + +### Nested Functions + +Functions defined during render but not called are allowed to mutate refs: +```javascript +// OK - onClick is not called during render +const onClick = () => { ref.current = value; }; +return - - - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{}], -}; - -``` - - -## Error - -``` -Found 1 error: - -Error: Cannot access refs during render - -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). - -error.ref-value-in-custom-component-event-handler-wrapper.ts:31:41 - 29 | <> - 30 | -> 31 | - | ^^^^^^^^ Passing a ref to a function may read its value during render - 32 | - 33 | - 34 | -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md deleted file mode 100644 index 718e2c81419..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md +++ /dev/null @@ -1,55 +0,0 @@ - -## Input - -```javascript -// @enableInferEventHandlers -import {useRef} from 'react'; - -// Simulates a handler wrapper -function handleClick(value: any) { - return () => { - console.log(value); - }; -} - -function Component() { - const ref = useRef(null); - - // This should still error: passing ref.current directly to a wrapper - // The ref value is accessed during render, not in the event handler - return ( - <> - - - - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{}], -}; - -``` - - -## Error - -``` -Found 1 error: - -Error: Cannot access refs during render - -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). - -error.ref-value-in-event-handler-wrapper.ts:19:35 - 17 | <> - 18 | -> 19 | - | ^^^^^^^^^^^ Cannot access ref value during render - 20 | - 21 | ); - 22 | } -``` - - \ No newline at end of file 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..e063342e800 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 @@ -41,7 +41,7 @@ error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoi 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 update 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..9c7fec3b428 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 @@ -40,14 +40,14 @@ Error: Cannot access refs during render 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). -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 update 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..3a9f61b35ae 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 @@ -40,7 +40,7 @@ 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 update ref during render 14 | 15 | return ; 16 | } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md deleted file mode 100644 index 1d5a4a2284c..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md +++ /dev/null @@ -1,39 +0,0 @@ - -## Input - -```javascript -// @validateRefAccessDuringRender:true -function Foo(props, ref) { - console.log(ref.current); - return
{props.bar}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{bar: 'foo'}, {ref: {cuurrent: 1}}], - isComponent: true, -}; - -``` - - -## Error - -``` -Found 1 error: - -Error: Cannot access refs during render - -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). - -error.validate-mutate-ref-arg-in-render.ts:3:14 - 1 | // @validateRefAccessDuringRender:true - 2 | function Foo(props, ref) { -> 3 | console.log(ref.current); - | ^^^^^^^^^^^ Passing a ref to a function may read its value during render - 4 | return
{props.bar}
; - 5 | } - 6 | -``` - - \ No newline at end of file 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/impure-functions-in-render-indirect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-indirect.expect.md new file mode 100644 index 00000000000..4718ee7f203 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-indirect.expect.md @@ -0,0 +1,56 @@ + +## Input + +```javascript +// @validateNoImpureFunctionsInRender + +import {identity, makeArray} from 'shared-runtime'; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. The impurity is lost + * when passed through external function calls. + */ +function Component() { + const getDate = () => Date.now(); + const array = makeArray(getDate()); + const hasDate = identity(array); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoImpureFunctionsInRender + +import { identity, makeArray } from "shared-runtime"; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. The impurity is lost + * when passed through external function calls. + */ +function Component() { + const $ = _c(1); + const getDate = _temp; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const array = makeArray(getDate()); + const hasDate = identity(array); + t0 = ; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} +function _temp() { + return Date.now(); +} + +``` + +### 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/impure-functions-in-render-indirect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-indirect.js new file mode 100644 index 00000000000..285838c7981 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-indirect.js @@ -0,0 +1,15 @@ +// @validateNoImpureFunctionsInRender + +import {identity, makeArray} from 'shared-runtime'; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. The impurity is lost + * when passed through external function calls. + */ +function Component() { + const getDate = () => Date.now(); + const array = makeArray(getDate()); + const hasDate = identity(array); + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-via-function-call-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-via-function-call-2.expect.md new file mode 100644 index 00000000000..b7ebee784d1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-via-function-call-2.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +// @validateNoImpureFunctionsInRender + +import {identity, makeArray} from 'shared-runtime'; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. The impurity is lost + * when passed through external function calls. + */ +function Component() { + const now = () => Date.now(); + const f = () => { + const array = makeArray(now()); + const hasDate = identity(array); + return hasDate; + }; + const hasDate = f(); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoImpureFunctionsInRender + +import { identity, makeArray } from "shared-runtime"; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. The impurity is lost + * when passed through external function calls. + */ +function Component() { + const $ = _c(1); + const now = _temp; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const f = () => { + const array = makeArray(now()); + const hasDate = identity(array); + return hasDate; + }; + const hasDate_0 = f(); + t0 = ; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} +function _temp() { + return Date.now(); +} + +``` + +### 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/impure-functions-in-render-via-function-call-2.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-via-function-call-2.js new file mode 100644 index 00000000000..c34d65bd537 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/impure-functions-in-render-via-function-call-2.js @@ -0,0 +1,19 @@ +// @validateNoImpureFunctionsInRender + +import {identity, makeArray} from 'shared-runtime'; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. The impurity is lost + * when passed through external function calls. + */ +function Component() { + const now = () => Date.now(); + const f = () => { + const array = makeArray(now()); + const hasDate = identity(array); + return hasDate; + }; + const hasDate = f(); + return ; +} 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..b585792f2cd 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":"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":20,"index":307},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot update 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..d79ac677067 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":"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":20,"index":339},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot update 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/pass-ref-to-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pass-ref-to-function.expect.md new file mode 100644 index 00000000000..df4aa13ddb8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pass-ref-to-function.expect.md @@ -0,0 +1,45 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses a ref during render. The return type of foo() is unknown. + */ +function Component(props) { + const ref = useRef(null); + const x = foo(ref); + return x.current; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses a ref during render. The return type of foo() is unknown. + */ +function Component(props) { + const $ = _c(1); + const ref = useRef(null); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = foo(ref); + $[0] = t0; + } else { + t0 = $[0]; + } + const x = t0; + return x.current; +} + +``` + +### 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/pass-ref-to-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pass-ref-to-function.js new file mode 100644 index 00000000000..4bf588acbc3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pass-ref-to-function.js @@ -0,0 +1,11 @@ +// @validateRefAccessDuringRender + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses a ref during render. The return type of foo() is unknown. + */ +function Component(props) { + const ref = useRef(null); + const x = foo(ref); + return x.current; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.expect.md deleted file mode 100644 index 77f104cab0c..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.expect.md +++ /dev/null @@ -1,42 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees:true - -import {useRef, useMemo} from 'react'; -import {makeArray} from 'shared-runtime'; - -function useFoo() { - const r = useRef(); - return useMemo(() => makeArray(r), []); -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [], -}; - -``` - - -## Error - -``` -Found 1 error: - -Error: Cannot access refs during render - -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). - -error.maybe-mutable-ref-not-preserved.ts:8:33 - 6 | function useFoo() { - 7 | const r = useRef(); -> 8 | return useMemo(() => makeArray(r), []); - | ^ Passing a ref to a function may read its value during render - 9 | } - 10 | - 11 | export const FIXTURE_ENTRYPOINT = { -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-with-refs.flow.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-with-refs.flow.expect.md deleted file mode 100644 index 0269b22a1f2..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-with-refs.flow.expect.md +++ /dev/null @@ -1,37 +0,0 @@ - -## Input - -```javascript -// @flow @validatePreserveExistingMemoizationGuarantees -import {identity} from 'shared-runtime'; - -component Component(disableLocalRef, ref) { - const localRef = useFooRef(); - const mergedRef = useMemo(() => { - return disableLocalRef ? ref : identity(ref, localRef); - }, [disableLocalRef, ref, localRef]); - return
; -} - -``` - - -## Error - -``` -Found 1 error: - -Error: Cannot access refs during render - -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). - - 5 | const localRef = useFooRef(); - 6 | const mergedRef = useMemo(() => { -> 7 | return disableLocalRef ? ref : identity(ref, localRef); - | ^^^ Passing a ref to a function may read its value during render - 8 | }, [disableLocalRef, ref, localRef]); - 9 | return
; - 10 | } -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.expect.md new file mode 100644 index 00000000000..f2201cdf288 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.expect.md @@ -0,0 +1,59 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees:true + +import {useRef, useMemo} from 'react'; +import {makeArray} from 'shared-runtime'; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. + */ +function useFoo() { + const r = useRef(); + return useMemo(() => makeArray(r), []); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees:true + +import { useRef, useMemo } from "react"; +import { makeArray } from "shared-runtime"; + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. + */ +function useFoo() { + const $ = _c(1); + const r = useRef(); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeArray(r); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], +}; + +``` + +### Eval output +(kind: ok) [{}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.ts similarity index 69% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.ts rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.ts index 13b8b445827..82751750504 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.ts @@ -3,6 +3,10 @@ import {useRef, useMemo} from 'react'; import {makeArray} from 'shared-runtime'; +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses an impure value during render. + */ function useFoo() { const r = useRef(); return useMemo(() => makeArray(r), []); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-with-refs.flow.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-with-refs.flow.expect.md new file mode 100644 index 00000000000..26551994a3e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-with-refs.flow.expect.md @@ -0,0 +1,52 @@ + +## Input + +```javascript +// @flow @validatePreserveExistingMemoizationGuarantees +import {identity} from 'shared-runtime'; + +component Component(disableLocalRef, ref) { + const localRef = useFooRef(); + const mergedRef = useMemo(() => { + return disableLocalRef ? ref : identity(ref, localRef); + }, [disableLocalRef, ref, localRef]); + return
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity } from "shared-runtime"; + +const Component = React.forwardRef(Component_withRef); +function Component_withRef(t0, ref) { + const $ = _c(6); + const { disableLocalRef } = t0; + const localRef = useFooRef(); + let t1; + if ($[0] !== disableLocalRef || $[1] !== localRef || $[2] !== ref) { + t1 = disableLocalRef ? ref : identity(ref, localRef); + $[0] = disableLocalRef; + $[1] = localRef; + $[2] = ref; + $[3] = t1; + } else { + t1 = $[3]; + } + const mergedRef = t1; + let t2; + if ($[4] !== mergedRef) { + t2 =
; + $[4] = mergedRef; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-with-refs.flow.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-with-refs.flow.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-with-refs.flow.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-with-refs.flow.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call-2.expect.md new file mode 100644 index 00000000000..6573a4b1cb5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call-2.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +//@flow +import {useRef} from 'react'; + +/** + * Allowed: we aren't sure that the ref.current value flows into the render + * output, so we optimistically assume it's safe + */ +component C() { + const r = useRef(null); + if (r.current == null) { + f(r); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +## Code + +```javascript +import { useRef } from "react"; + +function C() { + const r = useRef(null); + if (r.current == null) { + f(r); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +### Eval output +(kind: exception) f is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-call-2.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call-2.js similarity index 58% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-call-2.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call-2.js index 4e5a53cd3f6..8bfd4cd0e3c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-call-2.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call-2.js @@ -1,6 +1,10 @@ //@flow import {useRef} from 'react'; +/** + * Allowed: we aren't sure that the ref.current value flows into the render + * output, so we optimistically assume it's safe + */ component C() { const r = useRef(null); if (r.current == null) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call.expect.md new file mode 100644 index 00000000000..3502bff68d1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +//@flow +import {useRef} from 'react'; + +/** + * Allowed: we aren't sure that the ref.current value flows into the render + * output, so we optimistically assume it's safe + */ +component C() { + const r = useRef(null); + if (r.current == null) { + f(r.current); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +## Code + +```javascript +import { useRef } from "react"; + +function C() { + const r = useRef(null); + if (r.current == null) { + f(r.current); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +### Eval output +(kind: exception) f is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call.js similarity index 59% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-call.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call.js index 50288fafc4a..82192c0681a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-call.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-call.js @@ -1,6 +1,10 @@ //@flow import {useRef} from 'react'; +/** + * Allowed: we aren't sure that the ref.current value flows into the render + * output, so we optimistically assume it's safe + */ component C() { const r = useRef(null); if (r.current == null) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-post-access-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-post-access-2.expect.md new file mode 100644 index 00000000000..eb30bb34445 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-post-access-2.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +//@flow +import {useRef} from 'react'; + +/** + * Allowed: we aren't sure that the ref.current value flows into the render + * output, so we optimistically assume it's safe + */ +component C() { + const r = useRef(null); + if (r.current == null) { + r.current = 1; + } + f(r.current); +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +## Code + +```javascript +import { useRef } from "react"; + +function C() { + const r = useRef(null); + if (r.current == null) { + r.current = 1; + } + + f(r.current); +} + +export const FIXTURE_ENTRYPOINT = { + fn: C, + params: [{}], +}; + +``` + +### Eval output +(kind: exception) f is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-post-access-2.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-post-access-2.js similarity index 61% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-post-access-2.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-post-access-2.js index a8e3b124bfe..2a3e801b190 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-post-access-2.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-initialization-post-access-2.js @@ -1,6 +1,10 @@ //@flow import {useRef} from 'react'; +/** + * Allowed: we aren't sure that the ref.current value flows into the render + * output, so we optimistically assume it's safe + */ component C() { const r = useRef(null); if (r.current == null) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-custom-component-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-custom-component-event-handler-wrapper.expect.md new file mode 100644 index 00000000000..d67a02c57ec --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-custom-component-event-handler-wrapper.expect.md @@ -0,0 +1,121 @@ + +## Input + +```javascript +// @enableInferEventHandlers +import {useRef} from 'react'; + +// Simulates a custom component wrapper +function CustomForm({onSubmit, children}: any) { + return {children}; +} + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback: (data: T) => void) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = (data: any) => { + // Allowed: we aren't sure that the ref.current value flows into the render + // output, so we optimistically assume it's safe + if (ref.current !== null) { + console.log(ref.current.value); + } + }; + + return ( + <> + + + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableInferEventHandlers +import { useRef } from "react"; + +// Simulates a custom component wrapper +function CustomForm(t0) { + const $ = _c(3); + const { onSubmit, children } = t0; + let t1; + if ($[0] !== children || $[1] !== onSubmit) { + t1 =
{children}
; + $[0] = children; + $[1] = onSubmit; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback) { + const $ = _c(2); + let t0; + if ($[0] !== callback) { + t0 = (event) => { + event.preventDefault(); + callback({} as T); + }; + $[0] = callback; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +function Component() { + const $ = _c(1); + const ref = useRef(null); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const onSubmit = (data) => { + if (ref.current !== null) { + console.log(ref.current.value); + } + }; + t0 = ( + <> + + + + + + ); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-custom-component-event-handler-wrapper.tsx similarity index 84% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-custom-component-event-handler-wrapper.tsx index b90a1217165..5874c164558 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-custom-component-event-handler-wrapper.tsx @@ -18,8 +18,8 @@ function Component() { const ref = useRef(null); const onSubmit = (data: any) => { - // This should error: passing function with ref access to custom component - // event handler, even though it would be safe on a native
+ // Allowed: we aren't sure that the ref.current value flows into the render + // output, so we optimistically assume it's safe if (ref.current !== null) { console.log(ref.current.value); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-event-handler-wrapper.expect.md new file mode 100644 index 00000000000..1289ade402e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-event-handler-wrapper.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +// @enableInferEventHandlers +import {useRef} from 'react'; + +// Simulates a handler wrapper +function handleClick(value: any) { + return () => { + console.log(value); + }; +} + +function Component() { + const ref = useRef(null); + + // Allowed: we aren't sure that the ref.current value flows into the render + // output, so we optimistically assume it's safe + return ( + <> + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableInferEventHandlers +import { useRef } from "react"; + +// Simulates a handler wrapper +function handleClick(value) { + const $ = _c(2); + let t0; + if ($[0] !== value) { + t0 = () => { + console.log(value); + }; + $[0] = value; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +function Component() { + const $ = _c(1); + const ref = useRef(null); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = ( + <> + + + + ); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok) \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-event-handler-wrapper.tsx similarity index 74% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-event-handler-wrapper.tsx index 58313e560ce..64410473591 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-value-in-event-handler-wrapper.tsx @@ -11,8 +11,8 @@ function handleClick(value: any) { function Component() { const ref = useRef(null); - // This should still error: passing ref.current directly to a wrapper - // The ref value is accessed during render, not in the event handler + // Allowed: we aren't sure that the ref.current value flows into the render + // output, so we optimistically assume it's safe return ( <> 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/use-ref-added-to-dep-without-type-info.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-ref-added-to-dep-without-type-info.expect.md new file mode 100644 index 00000000000..f96ef187dda --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-ref-added-to-dep-without-type-info.expect.md @@ -0,0 +1,59 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses a ref during render. Type info is lost when ref is + * stored in an object field. + */ +function Foo({a}) { + const ref = useRef(); + const val = {ref}; + const x = {a, val: val.ref.current}; + + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses a ref during render. Type info is lost when ref is + * stored in an object field. + */ +function Foo(t0) { + const $ = _c(3); + const { a } = t0; + const ref = useRef(); + let t1; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t1 = { ref }; + $[0] = t1; + } else { + t1 = $[0]; + } + const val = t1; + let t2; + if ($[1] !== a) { + const x = { a, val: val.ref.current }; + t2 = ; + $[1] = a; + $[2] = t2; + } else { + t2 = $[2]; + } + return t2; +} + +``` + +### 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/use-ref-added-to-dep-without-type-info.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-ref-added-to-dep-without-type-info.js new file mode 100644 index 00000000000..2d29c6f15fa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-ref-added-to-dep-without-type-info.js @@ -0,0 +1,14 @@ +// @validateRefAccessDuringRender + +/** + * Allowed: we don't have sufficient type information to be sure that + * this accesses a ref during render. Type info is lost when ref is + * stored in an object field. + */ +function Foo({a}) { + const ref = useRef(); + const val = {ref}; + const x = {a, val: val.ref.current}; + + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-uncertain-impure-functions-in-render-via-render-helper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-uncertain-impure-functions-in-render-via-render-helper.expect.md new file mode 100644 index 00000000000..e694e2c542d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-uncertain-impure-functions-in-render-via-render-helper.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @validateNoImpureFunctionsInRender + +import {identity, makeArray} from 'shared-runtime'; + +function Component() { + const now = Date.now(); + const renderItem = () => { + const array = makeArray(now); + // we don't have an alias signature for identity(), so we optimistically + // assume this doesn't propagate the impurity + const hasDate = identity(array); + return ; + }; + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoImpureFunctionsInRender + +import { identity, makeArray } from "shared-runtime"; + +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const now = Date.now(); + const renderItem = () => { + const array = makeArray(now); + const hasDate = identity(array); + return ; + }; + t0 = ; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + +### 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-uncertain-impure-functions-in-render-via-render-helper.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-uncertain-impure-functions-in-render-via-render-helper.js new file mode 100644 index 00000000000..19028b87ddb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-uncertain-impure-functions-in-render-via-render-helper.js @@ -0,0 +1,15 @@ +// @validateNoImpureFunctionsInRender + +import {identity, makeArray} from 'shared-runtime'; + +function Component() { + const now = Date.now(); + const renderItem = () => { + const array = makeArray(now); + // we don't have an alias signature for identity(), so we optimistically + // assume this doesn't propagate the impurity + const hasDate = identity(array); + return ; + }; + return ; +} 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/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/validate-mutate-ref-arg-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/validate-mutate-ref-arg-in-render.expect.md new file mode 100644 index 00000000000..4c2979ca3a7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/validate-mutate-ref-arg-in-render.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender:true + +function Foo(props, ref) { + // Allowed: the value is not guaranteed to flow into something that's rendered + console.log(ref.current); + return
{props.bar}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{bar: 'foo'}, {ref: {cuurrent: 1}}], + isComponent: true, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender:true + +function Foo(props, ref) { + const $ = _c(2); + + console.log(ref.current); + let t0; + if ($[0] !== props.bar) { + t0 =
{props.bar}
; + $[0] = props.bar; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ bar: "foo" }, { ref: { cuurrent: 1 } }], + isComponent: true, +}; + +``` + +### Eval output +(kind: ok)
foo
+logs: [undefined] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/validate-mutate-ref-arg-in-render.js similarity index 75% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/validate-mutate-ref-arg-in-render.js index 10218fc6163..55986513a19 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/validate-mutate-ref-arg-in-render.js @@ -1,5 +1,7 @@ // @validateRefAccessDuringRender:true + function Foo(props, ref) { + // Allowed: the value is not guaranteed to flow into something that's rendered console.log(ref.current); return
{props.bar}
; } 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'), ], }, ], diff --git a/compiler/packages/snap/src/constants.ts b/compiler/packages/snap/src/constants.ts index d1ede2a2f2a..066b6b950a2 100644 --- a/compiler/packages/snap/src/constants.ts +++ b/compiler/packages/snap/src/constants.ts @@ -26,5 +26,3 @@ export const FIXTURES_PATH = path.join( 'compiler', ); export const SNAPSHOT_EXTENSION = '.expect.md'; -export const FILTER_FILENAME = 'testfilter.txt'; -export const FILTER_PATH = path.join(PROJECT_ROOT, FILTER_FILENAME); diff --git a/compiler/packages/snap/src/fixture-utils.ts b/compiler/packages/snap/src/fixture-utils.ts index fae6afef151..29f7d4ddfbb 100644 --- a/compiler/packages/snap/src/fixture-utils.ts +++ b/compiler/packages/snap/src/fixture-utils.ts @@ -8,7 +8,7 @@ import fs from 'fs/promises'; import * as glob from 'glob'; import path from 'path'; -import {FILTER_PATH, FIXTURES_PATH, SNAPSHOT_EXTENSION} from './constants'; +import {FIXTURES_PATH, SNAPSHOT_EXTENSION} from './constants'; const INPUT_EXTENSIONS = [ '.js', @@ -22,19 +22,9 @@ const INPUT_EXTENSIONS = [ ]; export type TestFilter = { - debug: boolean; paths: Array; }; -async function exists(file: string): Promise { - try { - await fs.access(file); - return true; - } catch { - return false; - } -} - function stripExtension(filename: string, extensions: Array): string { for (const ext of extensions) { if (filename.endsWith(ext)) { @@ -44,37 +34,6 @@ function stripExtension(filename: string, extensions: Array): string { return filename; } -export async function readTestFilter(): Promise { - if (!(await exists(FILTER_PATH))) { - throw new Error(`testfilter file not found at \`${FILTER_PATH}\``); - } - - const input = await fs.readFile(FILTER_PATH, 'utf8'); - const lines = input.trim().split('\n'); - - let debug: boolean = false; - const line0 = lines[0]; - if (line0 != null) { - // Try to parse pragmas - let consumedLine0 = false; - if (line0.indexOf('@only') !== -1) { - consumedLine0 = true; - } - if (line0.indexOf('@debug') !== -1) { - debug = true; - consumedLine0 = true; - } - - if (consumedLine0) { - lines.shift(); - } - } - return { - debug, - paths: lines.filter(line => !line.trimStart().startsWith('//')), - }; -} - export function getBasename(fixture: TestFixture): string { return stripExtension(path.basename(fixture.inputPath), INPUT_EXTENSIONS); } diff --git a/compiler/packages/snap/src/runner-watch.ts b/compiler/packages/snap/src/runner-watch.ts index 6073fd30f92..06ae9984668 100644 --- a/compiler/packages/snap/src/runner-watch.ts +++ b/compiler/packages/snap/src/runner-watch.ts @@ -8,8 +8,8 @@ import watcher from '@parcel/watcher'; import path from 'path'; import ts from 'typescript'; -import {FILTER_FILENAME, FIXTURES_PATH, PROJECT_ROOT} from './constants'; -import {TestFilter, readTestFilter} from './fixture-utils'; +import {FIXTURES_PATH, PROJECT_ROOT} from './constants'; +import {TestFilter} from './fixture-utils'; import {execSync} from 'child_process'; export function watchSrc( @@ -117,6 +117,10 @@ export type RunnerState = { lastUpdate: number; mode: RunnerMode; filter: TestFilter | null; + debug: boolean; + // Input mode for interactive pattern entry + inputMode: 'none' | 'pattern'; + inputBuffer: string; }; function subscribeFixtures( @@ -142,26 +146,6 @@ function subscribeFixtures( }); } -function subscribeFilterFile( - state: RunnerState, - onChange: (state: RunnerState) => void, -) { - watcher.subscribe(PROJECT_ROOT, async (err, events) => { - if (err) { - console.error(err); - process.exit(1); - } else if ( - events.findIndex(event => event.path.includes(FILTER_FILENAME)) !== -1 - ) { - if (state.mode.filter) { - state.filter = await readTestFilter(); - state.mode.action = RunnerAction.Test; - onChange(state); - } - } - }); -} - function subscribeTsc( state: RunnerState, onChange: (state: RunnerState) => void, @@ -200,15 +184,67 @@ function subscribeKeyEvents( onChange: (state: RunnerState) => void, ) { process.stdin.on('keypress', async (str, key) => { + // Handle input mode (pattern entry) + if (state.inputMode !== 'none') { + if (key.name === 'return') { + // Enter pressed - process input + const pattern = state.inputBuffer.trim(); + state.inputMode = 'none'; + state.inputBuffer = ''; + process.stdout.write('\n'); + + if (pattern !== '') { + // Set the pattern as filter + state.filter = {paths: [pattern]}; + state.mode.filter = true; + state.mode.action = RunnerAction.Test; + onChange(state); + } + // If empty, just exit input mode without changes + return; + } else if (key.name === 'escape') { + // Cancel input mode + state.inputMode = 'none'; + state.inputBuffer = ''; + process.stdout.write(' (cancelled)\n'); + return; + } else if (key.name === 'backspace') { + if (state.inputBuffer.length > 0) { + state.inputBuffer = state.inputBuffer.slice(0, -1); + // Erase character: backspace, space, backspace + process.stdout.write('\b \b'); + } + return; + } else if (str && !key.ctrl && !key.meta) { + // Regular character - accumulate and echo + state.inputBuffer += str; + process.stdout.write(str); + return; + } + return; // Ignore other keys in input mode + } + + // Normal mode keypress handling if (key.name === 'u') { // u => update fixtures state.mode.action = RunnerAction.Update; } else if (key.name === 'q') { process.exit(0); - } else if (key.name === 'f') { - state.mode.filter = !state.mode.filter; - state.filter = state.mode.filter ? await readTestFilter() : null; + } else if (key.name === 'a') { + // a => exit filter mode and run all tests + state.mode.filter = false; + state.filter = null; + state.mode.action = RunnerAction.Test; + } else if (key.name === 'd') { + // d => toggle debug logging + state.debug = !state.debug; state.mode.action = RunnerAction.Test; + } else if (key.name === 'p') { + // p => enter pattern input mode + state.inputMode = 'pattern'; + state.inputBuffer = ''; + process.stdout.write('Pattern: '); + return; // Don't trigger onChange yet } else { // any other key re-runs tests state.mode.action = RunnerAction.Test; @@ -219,21 +255,33 @@ function subscribeKeyEvents( export async function makeWatchRunner( onChange: (state: RunnerState) => void, - filterMode: boolean, + debugMode: boolean, + initialPattern?: string, ): Promise { - const state = { + // Determine initial filter state + let filter: TestFilter | null = null; + let filterEnabled = false; + + if (initialPattern) { + filter = {paths: [initialPattern]}; + filterEnabled = true; + } + + const state: RunnerState = { compilerVersion: 0, isCompilerBuildValid: false, lastUpdate: -1, mode: { action: RunnerAction.Test, - filter: filterMode, + filter: filterEnabled, }, - filter: filterMode ? await readTestFilter() : null, + filter, + debug: debugMode, + inputMode: 'none', + inputBuffer: '', }; subscribeTsc(state, onChange); subscribeFixtures(state, onChange); subscribeKeyEvents(state, onChange); - subscribeFilterFile(state, onChange); } diff --git a/compiler/packages/snap/src/runner.ts b/compiler/packages/snap/src/runner.ts index 478a32d426c..53679d85451 100644 --- a/compiler/packages/snap/src/runner.ts +++ b/compiler/packages/snap/src/runner.ts @@ -12,8 +12,8 @@ import * as readline from 'readline'; import ts from 'typescript'; import yargs from 'yargs'; import {hideBin} from 'yargs/helpers'; -import {FILTER_PATH, PROJECT_ROOT} from './constants'; -import {TestFilter, getFixtures, readTestFilter} from './fixture-utils'; +import {PROJECT_ROOT} from './constants'; +import {TestFilter, getFixtures} from './fixture-utils'; import {TestResult, TestResults, report, update} from './reporter'; import { RunnerAction, @@ -33,9 +33,9 @@ type RunnerOptions = { sync: boolean; workerThreads: boolean; watch: boolean; - filter: boolean; update: boolean; pattern?: string; + debug: boolean; }; const opts: RunnerOptions = yargs @@ -59,18 +59,16 @@ const opts: RunnerOptions = yargs .alias('u', 'update') .describe('update', 'Update fixtures') .default('update', false) - .boolean('filter') - .describe( - 'filter', - 'Only run fixtures which match the contents of testfilter.txt', - ) - .default('filter', false) .string('pattern') .alias('p', 'pattern') .describe( 'pattern', 'Optional glob pattern to filter fixtures (e.g., "error.*", "use-memo")', ) + .boolean('debug') + .alias('d', 'debug') + .describe('debug', 'Enable debug logging to print HIR for each pass') + .default('debug', false) .help('help') .strict() .parseSync(hideBin(process.argv)) as RunnerOptions; @@ -82,12 +80,15 @@ async function runFixtures( worker: Worker & typeof runnerWorker, filter: TestFilter | null, compilerVersion: number, + debug: boolean, + requireSingleFixture: boolean, ): Promise { // We could in theory be fancy about tracking the contents of the fixtures // directory via our file subscription, but it's simpler to just re-read // the directory each time. const fixtures = await getFixtures(filter); const isOnlyFixture = filter !== null && fixtures.size === 1; + const shouldLog = debug && (!requireSingleFixture || isOnlyFixture); let entries: Array<[string, TestResult]>; if (!opts.sync) { @@ -96,12 +97,7 @@ async function runFixtures( for (const [fixtureName, fixture] of fixtures) { work.push( worker - .transformFixture( - fixture, - compilerVersion, - (filter?.debug ?? false) && isOnlyFixture, - true, - ) + .transformFixture(fixture, compilerVersion, shouldLog, true) .then(result => [fixtureName, result]), ); } @@ -113,7 +109,7 @@ async function runFixtures( let output = await runnerWorker.transformFixture( fixture, compilerVersion, - (filter?.debug ?? false) && isOnlyFixture, + shouldLog, true, ); entries.push([fixtureName, output]); @@ -128,7 +124,7 @@ async function onChange( worker: Worker & typeof runnerWorker, state: RunnerState, ) { - const {compilerVersion, isCompilerBuildValid, mode, filter} = state; + const {compilerVersion, isCompilerBuildValid, mode, filter, debug} = state; if (isCompilerBuildValid) { const start = performance.now(); @@ -142,6 +138,8 @@ async function onChange( worker, mode.filter ? filter : null, compilerVersion, + debug, + true, // requireSingleFixture in watch mode ); const end = performance.now(); if (mode.action === RunnerAction.Update) { @@ -159,11 +157,13 @@ async function onChange( console.log( '\n' + (mode.filter - ? `Current mode = FILTER, filter test fixtures by "${FILTER_PATH}".` + ? `Current mode = FILTER, pattern = "${filter?.paths[0] ?? ''}".` : 'Current mode = NORMAL, run all test fixtures.') + '\nWaiting for input or file changes...\n' + 'u - update all fixtures\n' + - `f - toggle (turn ${mode.filter ? 'off' : 'on'}) filter mode\n` + + `d - toggle (turn ${debug ? 'off' : 'on'}) debug logging\n` + + 'p - enter pattern to filter fixtures\n' + + (mode.filter ? 'a - run all tests (exit filter mode)\n' : '') + 'q - quit\n' + '[any] - rerun tests\n', ); @@ -180,15 +180,16 @@ export async function main(opts: RunnerOptions): Promise { worker.getStderr().pipe(process.stderr); worker.getStdout().pipe(process.stdout); - // If pattern is provided, force watch mode off and use pattern filter - const shouldWatch = opts.watch && opts.pattern == null; - if (opts.watch && opts.pattern != null) { - console.warn('NOTE: --watch is ignored when a --pattern is supplied'); - } + // Check if watch mode should be enabled + const shouldWatch = opts.watch; if (shouldWatch) { - makeWatchRunner(state => onChange(worker, state), opts.filter); - if (opts.filter) { + makeWatchRunner( + state => onChange(worker, state), + opts.debug, + opts.pattern, + ); + if (opts.pattern) { /** * Warm up wormers when in watch mode. Loading the Forget babel plugin * and all of its transitive dependencies takes 1-3s (per worker) on a M1. @@ -236,14 +237,17 @@ export async function main(opts: RunnerOptions): Promise { let testFilter: TestFilter | null = null; if (opts.pattern) { testFilter = { - debug: true, paths: [opts.pattern], }; - } else if (opts.filter) { - testFilter = await readTestFilter(); } - const results = await runFixtures(worker, testFilter, 0); + const results = await runFixtures( + worker, + testFilter, + 0, + opts.debug, + false, // no requireSingleFixture in non-watch mode + ); if (opts.update) { update(results); isSuccess = true;