@samitouri / QOS-React / commits / 04a7a61918

[compiler] Allow assigning ref-accessing functions to objects if not mutated (#34026)

Allows assigning a ref-accessing function to an object so long as that object is not subsequently transitively mutated. We should likely rewrite the ref validation to use the new mutation/aliasing effects, which would provide a more consistent behavior across instruction types and require fewer special cases like this. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34026). * #34027 * __->__ #34026

Joseph Savona committed Jul 29, 2025 at 10:57 UTC 04a7a61918522734a083bff37843865d7815d466
6 files changed +196 -19
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts
+79 -13
@@ -80,8 +80,18 @@ type RefAccessRefType =
80
81 type RefFnType = {readRefEffect: boolean; returnType: RefAccessType};
82
83 -class Env extends Map<IdentifierId, RefAccessType> {
83 +class Env {
84 #changed = false;
85 + #data: Map<IdentifierId, RefAccessType> = new Map();
86 + #temporaries: Map<IdentifierId, Place> = new Map();
87 +
88 + lookup(place: Place): Place {
89 + return this.#temporaries.get(place.identifier.id) ?? place;
90 + }
91 +
92 + define(place: Place, value: Place): void {
93 + this.#temporaries.set(place.identifier.id, value);
94 + }
95
96 resetChanged(): void {
97 this.#changed = false;
@@ -91,8 +101,14 @@ class Env extends Map<IdentifierId, RefAccessType> {
101 return this.#changed;
102 }
103
94 - override set(key: IdentifierId, value: RefAccessType): this {
95 - const cur = this.get(key);
104 + get(key: IdentifierId): RefAccessType | undefined {
105 + const operandId = this.#temporaries.get(key)?.identifier.id ?? key;
106 + return this.#data.get(operandId);
107 + }
108 +
109 + set(key: IdentifierId, value: RefAccessType): this {
110 + const operandId = this.#temporaries.get(key)?.identifier.id ?? key;
111 + const cur = this.#data.get(operandId);
112 const widenedValue = joinRefAccessTypes(value, cur ?? {kind: 'None'});
113 if (
114 !(cur == null && widenedValue.kind === 'None') &&
@@ -100,7 +116,8 @@ class Env extends Map<IdentifierId, RefAccessType> {
116 ) {
117 this.#changed = true;
118 }
103 - return super.set(key, widenedValue);
119 + this.#data.set(operandId, widenedValue);
120 + return this;
121 }
122 }
123
@@ -108,9 +125,48 @@ export function validateNoRefAccessInRender(
125 fn: HIRFunction,
126 ): Result<void, CompilerError> {
127 const env = new Env();
128 + collectTemporariesSidemap(fn, env);
129 return validateNoRefAccessInRenderImpl(fn, env).map(_ => undefined);
130 }
131
132 +function collectTemporariesSidemap(fn: HIRFunction, env: Env): void {
133 + for (const block of fn.body.blocks.values()) {
134 + for (const instr of block.instructions) {
135 + const {lvalue, value} = instr;
136 + switch (value.kind) {
137 + case 'LoadLocal': {
138 + const temp = env.lookup(value.place);
139 + if (temp != null) {
140 + env.define(lvalue, temp);
141 + }
142 + break;
143 + }
144 + case 'StoreLocal': {
145 + const temp = env.lookup(value.value);
146 + if (temp != null) {
147 + env.define(lvalue, temp);
148 + env.define(value.lvalue.place, temp);
149 + }
150 + break;
151 + }
152 + case 'PropertyLoad': {
153 + if (
154 + isUseRefType(value.object.identifier) &&
155 + value.property === 'current'
156 + ) {
157 + continue;
158 + }
159 + const temp = env.lookup(value.object);
160 + if (temp != null) {
161 + env.define(lvalue, temp);
162 + }
163 + break;
164 + }
165 + }
166 + }
167 + }
168 +}
169 +
170 function refTypeOfType(place: Place): RefAccessType {
171 if (isRefValueType(place.identifier)) {
172 return {kind: 'RefValue'};
@@ -524,11 +580,25 @@ function validateNoRefAccessInRenderImpl(
580 } else {
581 validateNoRefUpdate(errors, env, instr.value.object, instr.loc);
582 }
527 - for (const operand of eachInstructionValueOperand(instr.value)) {
528 - if (operand === instr.value.object) {
529 - continue;
583 + if (
584 + instr.value.kind === 'ComputedDelete' ||
585 + instr.value.kind === 'ComputedStore'
586 + ) {
587 + validateNoRefValueAccess(errors, env, instr.value.property);
588 + }
589 + if (
590 + instr.value.kind === 'ComputedStore' ||
591 + instr.value.kind === 'PropertyStore'
592 + ) {
593 + validateNoDirectRefValueAccess(errors, instr.value.value, env);
594 + const type = env.get(instr.value.value.identifier.id);
595 + if (type != null && type.kind === 'Structure') {
596 + let objectType: RefAccessType = type;
597 + if (target != null) {
598 + objectType = joinRefAccessTypes(objectType, target);
599 + }
600 + env.set(instr.value.object.identifier.id, objectType);
601 }
531 - validateNoRefValueAccess(errors, env, operand);
602 }
603 break;
604 }
@@ -730,11 +800,7 @@ function validateNoRefUpdate(
800 loc: SourceLocation,
801 ): void {
802 const type = destructure(env.get(operand.identifier.id));
733 - if (
734 - type?.kind === 'Ref' ||
735 - type?.kind === 'RefValue' ||
736 - (type?.kind === 'Structure' && type.fn?.readRefEffect)
737 - ) {
803 + if (type?.kind === 'Ref' || type?.kind === 'RefValue') {
804 errors.pushDiagnostic(
805 CompilerDiagnostic.create({
806 severity: ErrorSeverity.InvalidReact,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-ref-accessing-function-to-object-property-if-not-mutated.expect.md new
+52
@@ -0,0 +1,52 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {useRef} from 'react';
6 +import {Stringify} from 'shared-runtime';
7 +
8 +function Component(props) {
9 + const ref = useRef(props.value);
10 + const object = {};
11 + object.foo = () => ref.current;
12 + return <Stringify object={object} shouldInvokeFns={true} />;
13 +}
14 +
15 +export const FIXTURE_ENTRYPOINT = {
16 + fn: Component,
17 + params: [{value: 42}],
18 +};
19 +
20 +```
21 +
22 +## Code
23 +
24 +```javascript
25 +import { c as _c } from "react/compiler-runtime";
26 +import { useRef } from "react";
27 +import { Stringify } from "shared-runtime";
28 +
29 +function Component(props) {
30 + const $ = _c(1);
31 + const ref = useRef(props.value);
32 + let t0;
33 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
34 + const object = {};
35 + object.foo = () => ref.current;
36 + t0 = <Stringify object={object} shouldInvokeFns={true} />;
37 + $[0] = t0;
38 + } else {
39 + t0 = $[0];
40 + }
41 + return t0;
42 +}
43 +
44 +export const FIXTURE_ENTRYPOINT = {
45 + fn: Component,
46 + params: [{ value: 42 }],
47 +};
48 +
49 +```
50 +
51 +### Eval output
52 +(kind: ok) <div>{"object":{"foo":{"kind":"Function","result":42}},"shouldInvokeFns":true}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-ref-accessing-function-to-object-property-if-not-mutated.js new
+14
@@ -0,0 +1,14 @@
1 +import {useRef} from 'react';
2 +import {Stringify} from 'shared-runtime';
3 +
4 +function Component(props) {
5 + const ref = useRef(props.value);
6 + const object = {};
7 + object.foo = () => ref.current;
8 + return <Stringify object={object} shouldInvokeFns={true} />;
9 +}
10 +
11 +export const FIXTURE_ENTRYPOINT = {
12 + fn: Component,
13 + params: [{value: 42}],
14 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-render-mutate-object-with-ref-function.expect.md new
+37
@@ -0,0 +1,37 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {useRef} from 'react';
6 +
7 +function Component() {
8 + const ref = useRef(null);
9 + const object = {};
10 + object.foo = () => ref.current;
11 + const refValue = object.foo();
12 + return <div>{refValue}</div>;
13 +}
14 +
15 +```
16 +
17 +
18 +## Error
19 +
20 +```
21 +Found 1 error:
22 +
23 +Error: Cannot access refs during render
24 +
25 +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)
26 +
27 +error.invalid-access-ref-in-render-mutate-object-with-ref-function.ts:7:19
28 + 5 | const object = {};
29 + 6 | object.foo = () => ref.current;
30 +> 7 | const refValue = object.foo();
31 + | ^^^^^^^^^^ This function accesses a ref value
32 + 8 | return <div>{refValue}</div>;
33 + 9 | }
34 + 10 |
35 +```
36 +
37 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-in-render-mutate-object-with-ref-function.js new
+9
@@ -0,0 +1,9 @@
1 +import {useRef} from 'react';
2 +
3 +function Component() {
4 + const ref = useRef(null);
5 + const object = {};
6 + object.foo = () => ref.current;
7 + const refValue = object.foo();
8 + return <div>{refValue}</div>;
9 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md
+5 -6
@@ -41,14 +41,13 @@ Error: Cannot access refs during render
41
42 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)
43
44 -error.invalid-use-ref-added-to-dep-without-type-info.ts:10:21
45 - 8 | // however, this is an instance of accessing a ref during render and is disallowed
46 - 9 | // under React's rules, so we reject this input
47 -> 10 | const x = {a, val: val.ref.current};
48 - | ^^^^^^^^^^^^^^^ Cannot access ref value during render
44 +error.invalid-use-ref-added-to-dep-without-type-info.ts:12:28
45 + 10 | const x = {a, val: val.ref.current};
46 11 |
50 - 12 | return <VideoList videos={x} />;
47 +> 12 | return <VideoList videos={x} />;
48 + | ^ Cannot access ref value during render
49 13 | }
50 + 14 |
51 ```
52
53
\ No newline at end of file