Skip to content

[compiler] Improve impurity/ref validation #35298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
josephsavona wants to merge 1 commit into
base: main
Choose a base branch
from
+3,168 −1,853
Open

[compiler] Improve impurity/ref validation #35298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Pending

* Improve impurity and ref validation, reducing false positives [#35298](https://github.com/facebook/react/pull/35298) by [@josephsavona](https://github.com/josephsavona)

## 19.1.0-rc.2 (May 14, 2025)

## babel-plugin-react-compiler
Expand Down
17 changes: 10 additions & 7 deletions compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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}
Expand Down Expand Up @@ -271,10 +271,6 @@ function runWithEnvironment(
assertValidMutableRanges(hir);
}

if (env.config.validateRefAccessDuringRender) {
validateNoRefAccessInRender(hir).unwrap();
}

if (env.config.validateNoSetStateInRender) {
validateNoSetStateInRender(hir).unwrap();
}
Expand All @@ -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();
Expand Down
220 changes: 162 additions & 58 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 '..';
Expand Down Expand Up @@ -626,11 +626,136 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
// TODO: rest of Global objects
];

const createRenderHookAliasing: (
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: 'Freeze',
value: '@value',
reason: ValueReason.HookCaptured,
},
{kind: 'Assign', from: '@value', into: '@return'},
],
},
},
BuiltInUseEffectEventId,
);
const REACT_APIS: Array<[string, BuiltInType]> = [
[
'useContext',
Expand All @@ -644,6 +769,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
hookKind: 'useContext',
returnValueKind: ValueKind.Frozen,
returnValueReason: ValueReason.Context,
aliasing: createRenderHookAliasing(ValueReason.Context),
},
BuiltInUseContextHookId,
),
Expand All @@ -658,6 +784,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
hookKind: 'useState',
returnValueKind: ValueKind.Frozen,
returnValueReason: ValueReason.State,
aliasing: createRenderHookAliasing(ValueReason.State),
}),
],
[
Expand All @@ -670,6 +797,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
hookKind: 'useActionState',
returnValueKind: ValueKind.Frozen,
returnValueReason: ValueReason.State,
aliasing: createRenderHookAliasing(ValueReason.HookCaptured),
}),
],
[
Expand All @@ -682,6 +810,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
hookKind: 'useReducer',
returnValueKind: ValueKind.Frozen,
returnValueReason: ValueReason.ReducerState,
aliasing: createRenderHookAliasing(ValueReason.ReducerState),
}),
],
[
Expand All @@ -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'},
],
},
}),
],
[
Expand All @@ -715,17 +860,24 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
calleeEffect: Effect.Read,
hookKind: 'useMemo',
returnValueKind: ValueKind.Frozen,
aliasing: createRenderHookAliasing(ValueReason.HookCaptured),
}),
],
[
'useCallback',
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: createRenderHookAliasing(ValueReason.HookCaptured),
}),
],
[
Expand All @@ -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,
),
Expand All @@ -789,6 +907,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
calleeEffect: Effect.Read,
hookKind: 'useLayoutEffect',
returnValueKind: ValueKind.Frozen,
aliasing: EffectHookAliasing,
},
BuiltInUseLayoutEffectHookId,
),
Expand All @@ -804,6 +923,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
calleeEffect: Effect.Read,
hookKind: 'useInsertionEffect',
returnValueKind: ValueKind.Frozen,
aliasing: EffectHookAliasing,
},
BuiltInUseInsertionEffectHookId,
),
Expand All @@ -817,6 +937,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
calleeEffect: Effect.Read,
hookKind: 'useTransition',
returnValueKind: ValueKind.Frozen,
aliasing: createRenderHookAliasing(ValueReason.HookCaptured),
}),
],
[
Expand All @@ -829,6 +950,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
hookKind: 'useOptimistic',
returnValueKind: ValueKind.Frozen,
returnValueReason: ValueReason.State,
aliasing: createRenderHookAliasing(ValueReason.HookCaptured),
}),
],
[
Expand All @@ -842,6 +964,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
returnType: {kind: 'Poly'},
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Frozen,
aliasing: createRenderHookAliasing(ValueReason.HookCaptured),
},
BuiltInUseOperatorId,
),
Expand All @@ -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, [])],
];

Expand Down
Loading
Loading