Skip to content

Commit 4c35df6

Browse files
josephsavonajosephsavona
committed
[compiler] Improve impurity/ref tracking
note: This implements the idea discussed in reactwg/react#389 (comment) Currently we create `Impure` effects for impure functions like `Date.now()` or `Math.random()`, and then throw if the effect is reachable during render. However, impurity is a property of the resulting value: if the value isn't accessed during render then it's okay: maybe you're console-logging the time while debugging (fine), or storing the impure value into a ref and only accessing it in an effect or event handler (totally ok). This PR updates to validate that impure values are not transitively consumed during render, building on the `Render` effect. The validation currently uses an algorithm very similar to that of InferReactivePlaces - building a set of known impure values, starting with `Impure` effects as the sources and then flowing outward via data flow and mutations. If a transitively impure value reaches a `Render` effect, it's an error. We record both the source of the impure value and where it gets rendered to make it easier to understand and fix errors: ``` Error: Cannot access impure value during render 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-via-render-helper.ts:10:25 8 | const array = makeArray(now); 9 | const hasDate = identity(array); > 10 | return <Bar hasDate={hasDate} />; | ^^^^^^^ Cannot access impure value during render 11 | }; 12 | return <Foo renderItem={renderItem} />; 13 | } error.invalid-impure-functions-in-render-via-render-helper.ts:6:14 4 | 5 | function Component() { > 6 | const now = Date.now(); | ^^^^^^^^^^ `Date.now` is an impure function. 7 | const renderItem = () => { 8 | const array = makeArray(now); 9 | const hasDate = identity(array); ``` Impure values are allowed to flow into refs, meaning that we now allow `useRef(Date.now())` or `useRef(localFunctionThatReturnsMathDotRandom())` which would have errored previously. The next PR reuses this improved impurity tracking to validate ref access in render as well.
1 parent d963d42 commit 4c35df6

File tree

33 files changed

+1057
-199
lines changed

33 files changed

+1057
-199
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ export class CompilerDiagnostic {
132132
return new CompilerDiagnostic({...options, details: []});
133133
}
134134

135+
clone(): CompilerDiagnostic {
136+
const cloned = CompilerDiagnostic.create({...this.options});
137+
cloned.options.details = [...this.options.details];
138+
return cloned;
139+
}
140+
135141
get reason(): CompilerDiagnosticOptions['reason'] {
136142
return this.options.reason;
137143
}

‎compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHI
9696
import {outlineJSX} from '../Optimization/OutlineJsx';
9797
import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
9898
import {transformFire} from '../Transform';
99-
import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
10099
import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
101100
import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions';
102101
import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects';
@@ -107,6 +106,7 @@ import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions';
107106
import {optimizeForSSR} from '../Optimization/OptimizeForSSR';
108107
import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies';
109108
import {validateSourceLocations} from '../Validation/ValidateSourceLocations';
109+
import {validateNoImpureValuesInRender} from '../Validation/ValidateNoImpureValuesInRender';
110110

111111
export type CompilerPipelineValue =
112112
| {kind: 'ast'; name: string; value: CodegenFunction}
@@ -297,7 +297,7 @@ function runWithEnvironment(
297297
}
298298

299299
if (env.config.validateNoImpureFunctionsInRender) {
300-
validateNoImpureFunctionsInRender(hir).unwrap();
300+
validateNoImpureValuesInRender(hir).unwrap();
301301
}
302302

303303
validateNoFreezingKnownMutableFunctions(hir).unwrap();

‎compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts‎

Lines changed: 85 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import {
3838
addObject,
3939
} from './ObjectShape';
4040
import {BuiltInType, ObjectType, PolyType} from './Types';
41-
import {TypeConfig} from './TypeSchema';
41+
import {AliasingSignatureConfig, TypeConfig} from './TypeSchema';
4242
import {assertExhaustive} from '../Utils/utils';
4343
import {isHookName} from './Environment';
4444
import {CompilerError, SourceLocation} from '..';
@@ -626,6 +626,78 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
626626
// TODO: rest of Global objects
627627
];
628628

629+
const RenderHookAliasing: (
630+
reason: ValueReason,
631+
) => AliasingSignatureConfig = reason => ({
632+
receiver: '@receiver',
633+
params: [],
634+
rest: '@rest',
635+
returns: '@returns',
636+
temporaries: [],
637+
effects: [
638+
// Freeze the arguments
639+
{
640+
kind: 'Freeze',
641+
value: '@rest',
642+
reason: ValueReason.HookCaptured,
643+
},
644+
// Render the arguments
645+
{
646+
kind: 'Render',
647+
place: '@rest',
648+
},
649+
// Returns a frozen value
650+
{
651+
kind: 'Create',
652+
into: '@returns',
653+
value: ValueKind.Frozen,
654+
reason,
655+
},
656+
// May alias any arguments into the return
657+
{
658+
kind: 'Alias',
659+
from: '@rest',
660+
into: '@returns',
661+
},
662+
],
663+
});
664+
665+
const EffectHookAliasing: AliasingSignatureConfig = {
666+
receiver: '@receiver',
667+
params: [],
668+
rest: '@rest',
669+
returns: '@returns',
670+
temporaries: ['@effect'],
671+
effects: [
672+
// Freezes the function and deps
673+
{
674+
kind: 'Freeze',
675+
value: '@rest',
676+
reason: ValueReason.Effect,
677+
},
678+
// Internally creates an effect object that captures the function and deps
679+
{
680+
kind: 'Create',
681+
into: '@effect',
682+
value: ValueKind.Frozen,
683+
reason: ValueReason.KnownReturnSignature,
684+
},
685+
// The effect stores the function and dependencies
686+
{
687+
kind: 'Capture',
688+
from: '@rest',
689+
into: '@effect',
690+
},
691+
// Returns undefined
692+
{
693+
kind: 'Create',
694+
into: '@returns',
695+
value: ValueKind.Primitive,
696+
reason: ValueReason.KnownReturnSignature,
697+
},
698+
],
699+
};
700+
629701
/*
630702
* TODO(mofeiZ): We currently only store rest param effects for hooks.
631703
* now that FeatureFlag `enableTreatHooksAsFunctions` is removed we can
@@ -644,6 +716,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
644716
hookKind: 'useContext',
645717
returnValueKind: ValueKind.Frozen,
646718
returnValueReason: ValueReason.Context,
719+
aliasing: RenderHookAliasing(ValueReason.Context),
647720
},
648721
BuiltInUseContextHookId,
649722
),
@@ -658,6 +731,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
658731
hookKind: 'useState',
659732
returnValueKind: ValueKind.Frozen,
660733
returnValueReason: ValueReason.State,
734+
aliasing: RenderHookAliasing(ValueReason.State),
661735
}),
662736
],
663737
[
@@ -670,6 +744,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
670744
hookKind: 'useActionState',
671745
returnValueKind: ValueKind.Frozen,
672746
returnValueReason: ValueReason.State,
747+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
673748
}),
674749
],
675750
[
@@ -682,6 +757,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
682757
hookKind: 'useReducer',
683758
returnValueKind: ValueKind.Frozen,
684759
returnValueReason: ValueReason.ReducerState,
760+
aliasing: RenderHookAliasing(ValueReason.ReducerState),
685761
}),
686762
],
687763
[
@@ -715,6 +791,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
715791
calleeEffect: Effect.Read,
716792
hookKind: 'useMemo',
717793
returnValueKind: ValueKind.Frozen,
794+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
718795
}),
719796
],
720797
[
@@ -726,6 +803,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
726803
calleeEffect: Effect.Read,
727804
hookKind: 'useCallback',
728805
returnValueKind: ValueKind.Frozen,
806+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
729807
}),
730808
],
731809
[
@@ -739,41 +817,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
739817
calleeEffect: Effect.Read,
740818
hookKind: 'useEffect',
741819
returnValueKind: ValueKind.Frozen,
742-
aliasing: {
743-
receiver: '@receiver',
744-
params: [],
745-
rest: '@rest',
746-
returns: '@returns',
747-
temporaries: ['@effect'],
748-
effects: [
749-
// Freezes the function and deps
750-
{
751-
kind: 'Freeze',
752-
value: '@rest',
753-
reason: ValueReason.Effect,
754-
},
755-
// Internally creates an effect object that captures the function and deps
756-
{
757-
kind: 'Create',
758-
into: '@effect',
759-
value: ValueKind.Frozen,
760-
reason: ValueReason.KnownReturnSignature,
761-
},
762-
// The effect stores the function and dependencies
763-
{
764-
kind: 'Capture',
765-
from: '@rest',
766-
into: '@effect',
767-
},
768-
// Returns undefined
769-
{
770-
kind: 'Create',
771-
into: '@returns',
772-
value: ValueKind.Primitive,
773-
reason: ValueReason.KnownReturnSignature,
774-
},
775-
],
776-
},
820+
aliasing: EffectHookAliasing,
777821
},
778822
BuiltInUseEffectHookId,
779823
),
@@ -789,6 +833,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
789833
calleeEffect: Effect.Read,
790834
hookKind: 'useLayoutEffect',
791835
returnValueKind: ValueKind.Frozen,
836+
aliasing: EffectHookAliasing,
792837
},
793838
BuiltInUseLayoutEffectHookId,
794839
),
@@ -804,6 +849,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
804849
calleeEffect: Effect.Read,
805850
hookKind: 'useInsertionEffect',
806851
returnValueKind: ValueKind.Frozen,
852+
aliasing: EffectHookAliasing,
807853
},
808854
BuiltInUseInsertionEffectHookId,
809855
),
@@ -817,6 +863,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
817863
calleeEffect: Effect.Read,
818864
hookKind: 'useTransition',
819865
returnValueKind: ValueKind.Frozen,
866+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
820867
}),
821868
],
822869
[
@@ -829,6 +876,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
829876
hookKind: 'useOptimistic',
830877
returnValueKind: ValueKind.Frozen,
831878
returnValueReason: ValueReason.State,
879+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
832880
}),
833881
],
834882
[
@@ -842,6 +890,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
842890
returnType: {kind: 'Poly'},
843891
calleeEffect: Effect.Read,
844892
returnValueKind: ValueKind.Frozen,
893+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
845894
},
846895
BuiltInUseOperatorId,
847896
),

‎compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,14 +190,19 @@ function parseAliasingSignatureConfig(
190190
};
191191
}
192192
case 'Impure': {
193-
const place = lookup(effect.place);
193+
const into = lookup(effect.into);
194194
return {
195195
kind: 'Impure',
196+
into,
197+
description: effect.description,
198+
reason: effect.reason,
199+
};
200+
}
201+
case 'Render': {
202+
const place = lookup(effect.place);
203+
return {
204+
kind: 'Render',
196205
place,
197-
error: CompilerError.throwTodo({
198-
reason: 'Support impure effect declarations',
199-
loc: GeneratedSource,
200-
}),
201206
};
202207
}
203208
case 'Apply': {
@@ -1513,6 +1518,11 @@ export const DefaultNonmutatingHook = addHook(
15131518
value: '@rest',
15141519
reason: ValueReason.HookCaptured,
15151520
},
1521+
// Render the arguments
1522+
{
1523+
kind: 'Render',
1524+
place: '@rest',
1525+
},
15161526
// Returns a frozen value
15171527
{
15181528
kind: 'Create',

‎compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,7 @@ export function printAliasingEffect(effect: AliasingEffect): string {
10091009
return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
10101010
}
10111011
case 'Impure': {
1012-
return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
1012+
return `Impure ${printPlaceForAliasEffect(effect.into)} reason=${effect.reason} description=${effect.description}`;
10131013
}
10141014
case 'Render': {
10151015
return `Render ${printPlaceForAliasEffect(effect.place)}`;

‎compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,25 @@ export const ApplyEffectSchema: z.ZodType<ApplyEffectConfig> = z.object({
185185

186186
export type ImpureEffectConfig = {
187187
kind: 'Impure';
188-
place: string;
188+
into: string;
189+
reason: string;
190+
description: string;
189191
};
190192

191193
export const ImpureEffectSchema: z.ZodType<ImpureEffectConfig> = z.object({
192194
kind: z.literal('Impure'),
195+
into: LifetimeIdSchema,
196+
reason: z.string(),
197+
description: z.string(),
198+
});
199+
200+
export type RenderEffectConfig = {
201+
kind: 'Render';
202+
place: string;
203+
};
204+
205+
export const RenderEffectSchema: z.ZodType<RenderEffectConfig> = z.object({
206+
kind: z.literal('Render'),
193207
place: LifetimeIdSchema,
194208
});
195209

@@ -204,7 +218,8 @@ export type AliasingEffectConfig =
204218
| ImpureEffectConfig
205219
| MutateEffectConfig
206220
| MutateTransitiveConditionallyConfig
207-
| ApplyEffectConfig;
221+
| ApplyEffectConfig
222+
| RenderEffectConfig;
208223

209224
export const AliasingEffectSchema: z.ZodType<AliasingEffectConfig> = z.union([
210225
FreezeEffectSchema,

‎compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ export type AliasingEffect =
162162
/**
163163
* Indicates a side-effect that is not safe during render
164164
*/
165-
| {kind: 'Impure'; place: Place; error: CompilerDiagnostic}
165+
| {kind: 'Impure'; into: Place; reason: string; description: string}
166166
/**
167167
* Indicates that a given place is accessed during render. Used to distingush
168168
* hook arguments that are known to be called immediately vs those used for
@@ -222,6 +222,12 @@ export function hashEffect(effect: AliasingEffect): string {
222222
return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
223223
}
224224
case 'Impure':
225+
return [
226+
effect.kind,
227+
effect.into.identifier.id,
228+
effect.reason,
229+
effect.description,
230+
].join(':');
225231
case 'Render': {
226232
return [effect.kind, effect.place.identifier.id].join(':');
227233
}

0 commit comments

Comments
 (0)