Skip to content

Commit e33071c

Browse files
authored
[compiler] Improved ref validation for non-mutating functions (facebook#35893)
If a function is known to freeze its inputs, and captures refs, then we can safely assume those refs are not mutated during render. An example is React Native's PanResponder, which is designed for use in interaction handling. Calling `PanResponder.create()` creates an object that shouldn't be interacted with at render time, so we can treat it as freezing its arguments, returning a frozen value, and not accessing any refs in the callbacks passed to it. ValidateNoRefAccessInRender is updated accordingly - if we see a Freeze <place> and ImmutableCapture <place> for the same place in the same instruction, we know that it's not being mutated. Note that this is a pretty targeted fix. One weakness is that we may not always emit a Freeze effect if a value is already frozen, which could cause this optimization not to kick in. The worst case there is that you'd just get a ref access in render error though, not miscompilation. And we could always choose to always emit Freeze effects, even for frozen values, just to retain the information for validations like this.
1 parent c0060cf commit e33071c

8 files changed

Lines changed: 276 additions & 23 deletions

File tree

compiler/CLAUDE.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,20 @@ yarn snap -p <file-basename> -d
3535
yarn snap -u
3636
```
3737

38+
## Linting
39+
40+
```bash
41+
# Run lint on the compiler source
42+
yarn workspace babel-plugin-react-compiler lint
43+
```
44+
45+
## Formatting
46+
47+
```bash
48+
# Run prettier on all files (from the react root directory, not compiler/)
49+
yarn prettier-all
50+
```
51+
3852
## Compiling Arbitrary Files
3953

4054
Use `yarn snap compile` to compile any file (not just fixtures) with the React Compiler:

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts

Lines changed: 106 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -487,24 +487,26 @@ function validateNoRefAccessInRenderImpl(
487487
*/
488488
if (!didError) {
489489
const isRefLValue = isUseRefType(instr.lvalue.identifier);
490-
for (const operand of eachInstructionValueOperand(instr.value)) {
491-
/**
492-
* By default we check that function call operands are not refs,
493-
* ref values, or functions that can access refs.
494-
*/
495-
if (
496-
isRefLValue ||
497-
(hookKind != null &&
498-
hookKind !== 'useState' &&
499-
hookKind !== 'useReducer')
500-
) {
490+
if (
491+
isRefLValue ||
492+
(hookKind != null &&
493+
hookKind !== 'useState' &&
494+
hookKind !== 'useReducer')
495+
) {
496+
for (const operand of eachInstructionValueOperand(
497+
instr.value,
498+
)) {
501499
/**
502500
* Allow passing refs or ref-accessing functions when:
503501
* 1. lvalue is a ref (mergeRefs pattern: `mergeRefs(ref1, ref2)`)
504502
* 2. calling hooks (independently validated for ref safety)
505503
*/
506504
validateNoDirectRefValueAccess(errors, operand, env);
507-
} else if (interpolatedAsJsx.has(instr.lvalue.identifier.id)) {
505+
}
506+
} else if (interpolatedAsJsx.has(instr.lvalue.identifier.id)) {
507+
for (const operand of eachInstructionValueOperand(
508+
instr.value,
509+
)) {
508510
/**
509511
* Special case: the lvalue is passed as a jsx child
510512
*
@@ -513,7 +515,98 @@ function validateNoRefAccessInRenderImpl(
513515
* render function which attempts to obey the rules.
514516
*/
515517
validateNoRefValueAccess(errors, env, operand);
516-
} else {
518+
}
519+
} else if (hookKind == null && instr.effects != null) {
520+
/**
521+
* For non-hook functions with known aliasing effects, use the
522+
* effects to determine what validation to apply for each place.
523+
* Track visited id:kind pairs to avoid duplicate errors.
524+
*/
525+
const visitedEffects: Set<string> = new Set();
526+
for (const effect of instr.effects) {
527+
let place: Place | null = null;
528+
let validation: 'ref-passed' | 'direct-ref' | 'none' = 'none';
529+
switch (effect.kind) {
530+
case 'Freeze': {
531+
place = effect.value;
532+
validation = 'direct-ref';
533+
break;
534+
}
535+
case 'Mutate':
536+
case 'MutateTransitive':
537+
case 'MutateConditionally':
538+
case 'MutateTransitiveConditionally': {
539+
place = effect.value;
540+
validation = 'ref-passed';
541+
break;
542+
}
543+
case 'Render': {
544+
place = effect.place;
545+
validation = 'ref-passed';
546+
break;
547+
}
548+
case 'Capture':
549+
case 'Alias':
550+
case 'MaybeAlias':
551+
case 'Assign':
552+
case 'CreateFrom': {
553+
place = effect.from;
554+
validation = 'ref-passed';
555+
break;
556+
}
557+
case 'ImmutableCapture': {
558+
/**
559+
* ImmutableCapture can come from two sources:
560+
* 1. A known signature that explicitly freezes the operand
561+
* (e.g. PanResponder.create) — safe, the function doesn't
562+
* call callbacks during render.
563+
* 2. Downgraded defaults when the operand is already frozen
564+
* (e.g. foo(propRef)) — the function is unknown and may
565+
* access the ref.
566+
*
567+
* We distinguish these by checking whether the same operand
568+
* also has a Freeze effect on this instruction, which only
569+
* comes from known signatures.
570+
*/
571+
place = effect.from;
572+
const isFrozen = instr.effects.some(
573+
e =>
574+
e.kind === 'Freeze' &&
575+
e.value.identifier.id === effect.from.identifier.id,
576+
);
577+
validation = isFrozen ? 'direct-ref' : 'ref-passed';
578+
break;
579+
}
580+
case 'Create':
581+
case 'CreateFunction':
582+
case 'Apply':
583+
case 'Impure':
584+
case 'MutateFrozen':
585+
case 'MutateGlobal': {
586+
break;
587+
}
588+
}
589+
if (place !== null && validation !== 'none') {
590+
const key = `${place.identifier.id}:${validation}`;
591+
if (!visitedEffects.has(key)) {
592+
visitedEffects.add(key);
593+
if (validation === 'direct-ref') {
594+
validateNoDirectRefValueAccess(errors, place, env);
595+
} else {
596+
validateNoRefPassedToFunction(
597+
errors,
598+
env,
599+
place,
600+
place.loc,
601+
);
602+
}
603+
}
604+
}
605+
}
606+
} else {
607+
for (const operand of eachInstructionValueOperand(
608+
instr.value,
609+
)) {
517610
validateNoRefPassedToFunction(
518611
errors,
519612
env,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
```javascript
55
// @validateRefAccessDuringRender:true
6+
import {mutate} from 'shared-runtime';
7+
68
function Foo(props, ref) {
7-
console.log(ref.current);
9+
mutate(ref.current);
810
return <div>{props.bar}</div>;
911
}
1012

@@ -26,14 +28,14 @@ Error: Cannot access refs during render
2628
2729
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).
2830
29-
error.validate-mutate-ref-arg-in-render.ts:3:14
30-
1 | // @validateRefAccessDuringRender:true
31-
2 | function Foo(props, ref) {
32-
> 3 | console.log(ref.current);
33-
| ^^^^^^^^^^^ Passing a ref to a function may read its value during render
34-
4 | return <div>{props.bar}</div>;
35-
5 | }
36-
6 |
31+
error.validate-mutate-ref-arg-in-render.ts:5:9
32+
3 |
33+
4 | function Foo(props, ref) {
34+
> 5 | mutate(ref.current);
35+
| ^^^^^^^^^^^ Passing a ref to a function may read its value during render
36+
6 | return <div>{props.bar}</div>;
37+
7 | }
38+
8 |
3739
```
3840
3941

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// @validateRefAccessDuringRender:true
2+
import {mutate} from 'shared-runtime';
3+
24
function Foo(props, ref) {
3-
console.log(ref.current);
5+
mutate(ref.current);
46
return <div>{props.bar}</div>;
57
}
68

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @flow
6+
import {PanResponder, Stringify} from 'shared-runtime';
7+
8+
export default component Playground() {
9+
const onDragEndRef = useRef(() => {});
10+
useEffect(() => {
11+
onDragEndRef.current = () => {
12+
console.log('drag ended');
13+
};
14+
});
15+
const panResponder = useMemo(
16+
() =>
17+
PanResponder.create({
18+
onPanResponderTerminate: () => {
19+
onDragEndRef.current();
20+
},
21+
}),
22+
[]
23+
);
24+
return <Stringify responder={panResponder} />;
25+
}
26+
27+
```
28+
29+
## Code
30+
31+
```javascript
32+
import { c as _c } from "react/compiler-runtime";
33+
import { PanResponder, Stringify } from "shared-runtime";
34+
35+
export default function Playground() {
36+
const $ = _c(3);
37+
const onDragEndRef = useRef(_temp);
38+
let t0;
39+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40+
t0 = () => {
41+
onDragEndRef.current = _temp2;
42+
};
43+
$[0] = t0;
44+
} else {
45+
t0 = $[0];
46+
}
47+
useEffect(t0);
48+
let t1;
49+
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
50+
t1 = PanResponder.create({
51+
onPanResponderTerminate: () => {
52+
onDragEndRef.current();
53+
},
54+
});
55+
$[1] = t1;
56+
} else {
57+
t1 = $[1];
58+
}
59+
const panResponder = t1;
60+
let t2;
61+
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
62+
t2 = <Stringify responder={panResponder} />;
63+
$[2] = t2;
64+
} else {
65+
t2 = $[2];
66+
}
67+
return t2;
68+
}
69+
function _temp2() {
70+
console.log("drag ended");
71+
}
72+
function _temp() {}
73+
74+
```
75+
76+
### Eval output
77+
(kind: exception) Fixture not implemented
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// @flow
2+
import {PanResponder, Stringify} from 'shared-runtime';
3+
4+
export default component Playground() {
5+
const onDragEndRef = useRef(() => {});
6+
useEffect(() => {
7+
onDragEndRef.current = () => {
8+
console.log('drag ended');
9+
};
10+
});
11+
const panResponder = useMemo(
12+
() =>
13+
PanResponder.create({
14+
onPanResponderTerminate: () => {
15+
onDragEndRef.current();
16+
},
17+
}),
18+
[]
19+
);
20+
return <Stringify responder={panResponder} />;
21+
}

compiler/packages/snap/src/sprout/shared-runtime-type-provider.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,44 @@ export function makeSharedRuntimeTypeProvider({
196196
],
197197
},
198198
},
199+
PanResponder: {
200+
kind: 'object',
201+
properties: {
202+
create: {
203+
kind: 'function',
204+
positionalParams: [EffectEnum.Freeze],
205+
restParam: null,
206+
calleeEffect: EffectEnum.Read,
207+
returnType: {kind: 'type', name: 'Any'},
208+
returnValueKind: ValueKindEnum.Frozen,
209+
aliasing: {
210+
receiver: '@receiver',
211+
params: ['@config'],
212+
rest: null,
213+
returns: '@returns',
214+
temporaries: [],
215+
effects: [
216+
{
217+
kind: 'Freeze',
218+
value: '@config',
219+
reason: ValueReasonEnum.KnownReturnSignature,
220+
},
221+
{
222+
kind: 'Create',
223+
into: '@returns',
224+
value: ValueKindEnum.Frozen,
225+
reason: ValueReasonEnum.KnownReturnSignature,
226+
},
227+
{
228+
kind: 'ImmutableCapture',
229+
from: '@config',
230+
into: '@returns',
231+
},
232+
],
233+
},
234+
},
235+
},
236+
},
199237
},
200238
};
201239
} else if (moduleName === 'ReactCompilerKnownIncompatibleTest') {

compiler/packages/snap/src/sprout/shared-runtime.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,4 +421,10 @@ export function typedMutate(x: any, v: any = null): void {
421421
x.property = v;
422422
}
423423

424+
export const PanResponder = {
425+
create(obj: any): any {
426+
return obj;
427+
},
428+
};
429+
424430
export default typedLog;

0 commit comments

Comments
 (0)