@samitouri / QOS-React-1 / commits / 1e68a0a3ae

[compiler] Improve handling of refs

Summary: This change expands our handling of refs to build an understanding of nested refs within objects and functions that may return refs. It builds a special-purpose type system within the ref analysis that gives a very lightweight structural type to objects and array expressions (merging the types of all their members), and then propagating those types throughout the analysis (e.g., if `ref` has type `Ref`, then `{ x: ref }` and `[ref]` have type `Structural(value=Ref)` and `{x: ref}.anything` and `[ref][anything]` have type `Ref`). This allows us to support structures that contain refs, and functions that operate over them, being created and passed around during rendering without at runtime accessing a ref value. The analysis here uses a fixpoint to allow types to be fully propagated through the system, and we defend against diverging by widening the type of a variable if it could grow infinitely: so, in something like ``` let x = ref; while (condition) { x = [x] } ``` we end up giving `x` the type `Structural(value=Ref)`. ghstack-source-id: afb0b0cb014ffcf21ef4d0ede6511330fd975ec3 Pull Request resolved: https://github.com/facebook/react/pull/30902

Mike Vitousek committed Sep 16, 2024 at 10:53 UTC 1e68a0a3aed9975d2e302ccf1dff0861bf2be706
7 files changed +579 -264
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
+388 -259
@@ -8,9 +8,11 @@
8 import {CompilerError, ErrorSeverity} from '../CompilerError';
9 import {
10 HIRFunction,
11 + Identifier,
12 IdentifierId,
13 Place,
14 SourceLocation,
15 + getHookKindForType,
16 isRefValueType,
17 isUseRefType,
18 } from '../HIR';
@@ -42,25 +44,154 @@ import {isEffectHook} from './ValidateMemoizedEffectDependencies';
44 * In the future we may reject more cases, based on either object names (`fooRef.current` is likely a ref)
45 * or based on property name alone (`foo.current` might be a ref).
46 */
45 -type State = {
46 - refs: Set<IdentifierId>;
47 - refValues: Map<IdentifierId, SourceLocation | null>;
48 - refAccessingFunctions: Set<IdentifierId>;
49 -};
47 +
48 +type RefAccessType = {kind: 'None'} | RefAccessRefType;
49 +
50 +type RefAccessRefType =
51 + | {kind: 'Ref'}
52 + | {kind: 'RefValue'; loc?: SourceLocation}
53 + | {kind: 'Structure'; value: null | RefAccessRefType; fn: null | RefFnType};
54 +
55 +type RefFnType = {readRefEffect: boolean; returnType: RefAccessType};
56 +
57 +class Env extends Map<IdentifierId, RefAccessType> {
58 + #changed = false;
59 +
60 + resetChanged(): void {
61 + this.#changed = false;
62 + }
63 +
64 + hasChanged(): boolean {
65 + return this.#changed;
66 + }
67 +
68 + override set(key: IdentifierId, value: RefAccessType): this {
69 + const cur = this.get(key);
70 + const widenedValue = joinRefAccessTypes(value, cur ?? {kind: 'None'});
71 + if (
72 + !(cur == null && widenedValue.kind === 'None') &&
73 + (cur == null || !tyEqual(cur, widenedValue))
74 + ) {
75 + this.#changed = true;
76 + }
77 + return super.set(key, widenedValue);
78 + }
79 +}
80
81 export function validateNoRefAccessInRender(fn: HIRFunction): void {
52 - const state = {
53 - refs: new Set<IdentifierId>(),
54 - refValues: new Map<IdentifierId, SourceLocation | null>(),
55 - refAccessingFunctions: new Set<IdentifierId>(),
56 - };
57 - validateNoRefAccessInRenderImpl(fn, state).unwrap();
82 + const env = new Env();
83 + validateNoRefAccessInRenderImpl(fn, env).unwrap();
84 +}
85 +
86 +function refTypeOfType(identifier: Identifier): RefAccessType {
87 + if (isRefValueType(identifier)) {
88 + return {kind: 'RefValue'};
89 + } else if (isUseRefType(identifier)) {
90 + return {kind: 'Ref'};
91 + } else {
92 + return {kind: 'None'};
93 + }
94 +}
95 +
96 +function tyEqual(a: RefAccessType, b: RefAccessType): boolean {
97 + if (a.kind !== b.kind) {
98 + return false;
99 + }
100 + switch (a.kind) {
101 + case 'None':
102 + return true;
103 + case 'Ref':
104 + return true;
105 + case 'RefValue':
106 + CompilerError.invariant(b.kind === 'RefValue', {
107 + reason: 'Expected ref value',
108 + loc: null,
109 + });
110 + return a.loc == b.loc;
111 + case 'Structure': {
112 + CompilerError.invariant(b.kind === 'Structure', {
113 + reason: 'Expected structure',
114 + loc: null,
115 + });
116 + const fnTypesEqual =
117 + (a.fn === null && b.fn === null) ||
118 + (a.fn !== null &&
119 + b.fn !== null &&
120 + a.fn.readRefEffect === b.fn.readRefEffect &&
121 + tyEqual(a.fn.returnType, b.fn.returnType));
122 + return (
123 + fnTypesEqual &&
124 + (a.value === b.value ||
125 + (a.value !== null && b.value !== null && tyEqual(a.value, b.value)))
126 + );
127 + }
128 + }
129 +}
130 +
131 +function joinRefAccessTypes(...types: Array<RefAccessType>): RefAccessType {
132 + function joinRefAccessRefTypes(
133 + a: RefAccessRefType,
134 + b: RefAccessRefType,
135 + ): RefAccessRefType {
136 + if (a.kind === 'RefValue') {
137 + return a;
138 + } else if (b.kind === 'RefValue') {
139 + return b;
140 + } else if (a.kind === 'Ref' || b.kind === 'Ref') {
141 + return {kind: 'Ref'};
142 + } else {
143 + CompilerError.invariant(
144 + a.kind === 'Structure' && b.kind === 'Structure',
145 + {
146 + reason: 'Expected structure',
147 + loc: null,
148 + },
149 + );
150 + const fn =
151 + a.fn === null
152 + ? b.fn
153 + : b.fn === null
154 + ? a.fn
155 + : {
156 + readRefEffect: a.fn.readRefEffect || b.fn.readRefEffect,
157 + returnType: joinRefAccessTypes(
158 + a.fn.returnType,
159 + b.fn.returnType,
160 + ),
161 + };
162 + const value =
163 + a.value === null
164 + ? b.value
165 + : b.value === null
166 + ? a.value
167 + : joinRefAccessRefTypes(a.value, b.value);
168 + return {
169 + kind: 'Structure',
170 + fn,
171 + value,
172 + };
173 + }
174 + }
175 +
176 + return types.reduce(
177 + (a, b) => {
178 + if (a.kind === 'None') {
179 + return b;
180 + } else if (b.kind === 'None') {
181 + return a;
182 + } else {
183 + return joinRefAccessRefTypes(a, b);
184 + }
185 + },
186 + {kind: 'None'},
187 + );
188 }
189
190 function validateNoRefAccessInRenderImpl(
191 fn: HIRFunction,
62 - state: State,
63 -): Result<void, CompilerError> {
192 + env: Env,
193 +): Result<RefAccessType, CompilerError> {
194 + let returnValues: Array<undefined | RefAccessType> = [];
195 let place;
196 for (const param of fn.params) {
197 if (param.kind === 'Identifier') {
@@ -68,293 +199,289 @@ function validateNoRefAccessInRenderImpl(
199 } else {
200 place = param.place;
201 }
71 -
72 - if (isRefValueType(place.identifier)) {
73 - state.refValues.set(place.identifier.id, null);
74 - }
75 - if (isUseRefType(place.identifier)) {
76 - state.refs.add(place.identifier.id);
77 - }
202 + const type = refTypeOfType(place.identifier);
203 + env.set(place.identifier.id, type);
204 }
79 - const errors = new CompilerError();
80 - for (const [, block] of fn.body.blocks) {
81 - for (const phi of block.phis) {
82 - phi.operands.forEach(operand => {
83 - if (state.refs.has(operand.id) || isUseRefType(phi.id)) {
84 - state.refs.add(phi.id.id);
85 - }
86 - const refValue = state.refValues.get(operand.id);
87 - if (refValue !== undefined || isRefValueType(operand)) {
88 - state.refValues.set(
89 - phi.id.id,
90 - refValue ?? state.refValues.get(phi.id.id) ?? null,
91 - );
92 - }
93 - if (state.refAccessingFunctions.has(operand.id)) {
94 - state.refAccessingFunctions.add(phi.id.id);
95 - }
96 - });
97 - }
205
99 - for (const instr of block.instructions) {
100 - for (const operand of eachInstructionValueOperand(instr.value)) {
101 - if (isRefValueType(operand.identifier)) {
102 - CompilerError.invariant(state.refValues.has(operand.identifier.id), {
103 - reason: 'Expected ref value to be in state',
104 - loc: operand.loc,
105 - });
106 - }
107 - if (isUseRefType(operand.identifier)) {
108 - CompilerError.invariant(state.refs.has(operand.identifier.id), {
109 - reason: 'Expected ref to be in state',
110 - loc: operand.loc,
111 - });
112 - }
206 + for (let i = 0; (i == 0 || env.hasChanged()) && i < 10; i++) {
207 + env.resetChanged();
208 + returnValues = [];
209 + const errors = new CompilerError();
210 + for (const [, block] of fn.body.blocks) {
211 + for (const phi of block.phis) {
212 + env.set(
213 + phi.id.id,
214 + joinRefAccessTypes(
215 + ...Array(...phi.operands.values()).map(
216 + operand => env.get(operand.id) ?? ({kind: 'None'} as const),
217 + ),
218 + ),
219 + );
220 }
221
115 - switch (instr.value.kind) {
116 - case 'JsxExpression':
117 - case 'JsxFragment': {
118 - for (const operand of eachInstructionValueOperand(instr.value)) {
119 - validateNoDirectRefValueAccess(errors, operand, state);
120 - }
121 - break;
122 - }
123 - case 'ComputedLoad':
124 - case 'PropertyLoad': {
125 - if (typeof instr.value.property !== 'string') {
126 - validateNoRefValueAccess(errors, state, instr.value.property);
127 - }
128 - if (
129 - state.refAccessingFunctions.has(instr.value.object.identifier.id)
130 - ) {
131 - state.refAccessingFunctions.add(instr.lvalue.identifier.id);
132 - }
133 - if (state.refs.has(instr.value.object.identifier.id)) {
134 - /*
135 - * Once an object contains a ref at any level, we treat it as a ref.
136 - * If we look something up from it, that value may either be a ref
137 - * or the ref value (or neither), so we conservatively assume it's both.
138 - */
139 - state.refs.add(instr.lvalue.identifier.id);
140 - state.refValues.set(instr.lvalue.identifier.id, instr.loc);
141 - }
142 - break;
143 - }
144 - case 'LoadContext':
145 - case 'LoadLocal': {
146 - if (
147 - state.refAccessingFunctions.has(instr.value.place.identifier.id)
148 - ) {
149 - state.refAccessingFunctions.add(instr.lvalue.identifier.id);
150 - }
151 - const refValue = state.refValues.get(instr.value.place.identifier.id);
152 - if (refValue !== undefined) {
153 - state.refValues.set(instr.lvalue.identifier.id, refValue);
222 + for (const instr of block.instructions) {
223 + switch (instr.value.kind) {
224 + case 'JsxExpression':
225 + case 'JsxFragment': {
226 + for (const operand of eachInstructionValueOperand(instr.value)) {
227 + validateNoDirectRefValueAccess(errors, operand, env);
228 + }
229 + break;
230 }
155 - if (state.refs.has(instr.value.place.identifier.id)) {
156 - state.refs.add(instr.lvalue.identifier.id);
231 + case 'ComputedLoad':
232 + case 'PropertyLoad': {
233 + if (typeof instr.value.property !== 'string') {
234 + validateNoDirectRefValueAccess(errors, instr.value.property, env);
235 + }
236 + const objType = env.get(instr.value.object.identifier.id);
237 + let lookupType: null | RefAccessType = null;
238 + if (objType?.kind === 'Structure') {
239 + lookupType = objType.value;
240 + } else if (objType?.kind === 'Ref') {
241 + lookupType = {kind: 'RefValue', loc: instr.loc};
242 + }
243 + env.set(
244 + instr.lvalue.identifier.id,
245 + lookupType ?? refTypeOfType(instr.lvalue.identifier),
246 + );
247 + break;
248 }
158 - break;
159 - }
160 - case 'StoreContext':
161 - case 'StoreLocal': {
162 - if (
163 - state.refAccessingFunctions.has(instr.value.value.identifier.id)
164 - ) {
165 - state.refAccessingFunctions.add(
166 - instr.value.lvalue.place.identifier.id,
249 + case 'LoadContext':
250 + case 'LoadLocal': {
251 + env.set(
252 + instr.lvalue.identifier.id,
253 + env.get(instr.value.place.identifier.id) ??
254 + refTypeOfType(instr.lvalue.identifier),
255 );
168 - state.refAccessingFunctions.add(instr.lvalue.identifier.id);
256 + break;
257 }
170 - const refValue = state.refValues.get(instr.value.value.identifier.id);
171 - if (
172 - refValue !== undefined ||
173 - isRefValueType(instr.value.lvalue.place.identifier)
174 - ) {
175 - state.refValues.set(
258 + case 'StoreContext':
259 + case 'StoreLocal': {
260 + env.set(
261 instr.value.lvalue.place.identifier.id,
177 - refValue ?? null,
262 + env.get(instr.value.value.identifier.id) ??
263 + refTypeOfType(instr.value.lvalue.place.identifier),
264 + );
265 + env.set(
266 + instr.lvalue.identifier.id,
267 + env.get(instr.value.value.identifier.id) ??
268 + refTypeOfType(instr.lvalue.identifier),
269 );
179 - state.refValues.set(instr.lvalue.identifier.id, refValue ?? null);
270 + break;
271 }
181 - if (state.refs.has(instr.value.value.identifier.id)) {
182 - state.refs.add(instr.value.lvalue.place.identifier.id);
183 - state.refs.add(instr.lvalue.identifier.id);
272 + case 'Destructure': {
273 + const objType = env.get(instr.value.value.identifier.id);
274 + let lookupType = null;
275 + if (objType?.kind === 'Structure') {
276 + lookupType = objType.value;
277 + }
278 + env.set(
279 + instr.lvalue.identifier.id,
280 + lookupType ?? refTypeOfType(instr.lvalue.identifier),
281 + );
282 + for (const lval of eachPatternOperand(instr.value.lvalue.pattern)) {
283 + env.set(
284 + lval.identifier.id,
285 + lookupType ?? refTypeOfType(lval.identifier),
286 + );
287 + }
288 + break;
289 }
185 - break;
186 - }
187 - case 'Destructure': {
188 - const destructuredFunction = state.refAccessingFunctions.has(
189 - instr.value.value.identifier.id,
190 - );
191 - const destructuredRef = state.refs.has(
192 - instr.value.value.identifier.id,
193 - );
194 - for (const lval of eachPatternOperand(instr.value.lvalue.pattern)) {
195 - if (isUseRefType(lval.identifier)) {
196 - state.refs.add(lval.identifier.id);
290 + case 'ObjectMethod':
291 + case 'FunctionExpression': {
292 + let returnType: RefAccessType = {kind: 'None'};
293 + let readRefEffect = false;
294 + const result = validateNoRefAccessInRenderImpl(
295 + instr.value.loweredFunc.func,
296 + env,
297 + );
298 + if (result.isOk()) {
299 + returnType = result.unwrap();
300 + } else if (result.isErr()) {
301 + readRefEffect = true;
302 }
198 - if (destructuredRef || isRefValueType(lval.identifier)) {
199 - state.refs.add(lval.identifier.id);
200 - state.refValues.set(lval.identifier.id, null);
303 + env.set(instr.lvalue.identifier.id, {
304 + kind: 'Structure',
305 + fn: {
306 + readRefEffect,
307 + returnType,
308 + },
309 + value: null,
310 + });
311 + break;
312 + }
313 + case 'MethodCall': {
314 + if (!isEffectHook(instr.value.property.identifier)) {
315 + for (const operand of eachInstructionValueOperand(instr.value)) {
316 + const hookKind = getHookKindForType(
317 + fn.env,
318 + instr.value.property.identifier.type,
319 + );
320 + if (hookKind != null) {
321 + validateNoRefValueAccess(errors, env, operand);
322 + } else {
323 + validateNoRefAccess(errors, env, operand, operand.loc);
324 + }
325 + }
326 }
202 - if (destructuredFunction) {
203 - state.refAccessingFunctions.add(lval.identifier.id);
327 + validateNoRefValueAccess(errors, env, instr.value.receiver);
328 + const methType = env.get(instr.value.property.identifier.id);
329 + let returnType: RefAccessType = {kind: 'None'};
330 + if (methType?.kind === 'Structure' && methType.fn !== null) {
331 + returnType = methType.fn.returnType;
332 }
333 + env.set(instr.lvalue.identifier.id, returnType);
334 + break;
335 }
206 - break;
207 - }
208 - case 'ObjectMethod':
209 - case 'FunctionExpression': {
210 - if (
211 - /*
212 - * check if the function expression accesses a ref *or* some other
213 - * function which accesses a ref
214 - */
215 - [...eachInstructionValueOperand(instr.value)].some(
216 - operand =>
217 - state.refValues.has(operand.identifier.id) ||
218 - state.refAccessingFunctions.has(operand.identifier.id),
219 - ) ||
220 - // check for cases where .current is accessed through an aliased ref
221 - ([...eachInstructionValueOperand(instr.value)].some(operand =>
222 - state.refs.has(operand.identifier.id),
223 - ) &&
224 - validateNoRefAccessInRenderImpl(
225 - instr.value.loweredFunc.func,
226 - state,
227 - ).isErr())
228 - ) {
229 - // This function expression unconditionally accesses a ref
230 - state.refAccessingFunctions.add(instr.lvalue.identifier.id);
336 + case 'CallExpression': {
337 + const callee = instr.value.callee;
338 + const hookKind = getHookKindForType(fn.env, callee.identifier.type);
339 + const isUseEffect = isEffectHook(callee.identifier);
340 + let returnType: RefAccessType = {kind: 'None'};
341 + if (!isUseEffect) {
342 + // Report a more precise error when calling a local function that accesses a ref
343 + const fnType = env.get(instr.value.callee.identifier.id);
344 + if (fnType?.kind === 'Structure' && fnType.fn !== null) {
345 + returnType = fnType.fn.returnType;
346 + if (fnType.fn.readRefEffect) {
347 + errors.push({
348 + severity: ErrorSeverity.InvalidReact,
349 + reason:
350 + 'This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)',
351 + loc: callee.loc,
352 + description:
353 + callee.identifier.name !== null &&
354 + callee.identifier.name.kind === 'named'
355 + ? `Function \`${callee.identifier.name.value}\` accesses a ref`
356 + : null,
357 + suggestions: null,
358 + });
359 + }
360 + }
361 + for (const operand of eachInstructionValueOperand(instr.value)) {
362 + if (hookKind != null) {
363 + validateNoRefValueAccess(errors, env, operand);
364 + } else {
365 + validateNoRefAccess(errors, env, operand, operand.loc);
366 + }
367 + }
368 + }
369 + env.set(instr.lvalue.identifier.id, returnType);
370 + break;
371 }
232 - break;
233 - }
234 - case 'MethodCall': {
235 - if (!isEffectHook(instr.value.property.identifier)) {
372 + case 'ObjectExpression':
373 + case 'ArrayExpression': {
374 + const types: Array<RefAccessType> = [];
375 for (const operand of eachInstructionValueOperand(instr.value)) {
237 - validateNoRefAccess(errors, state, operand, operand.loc);
376 + validateNoDirectRefValueAccess(errors, operand, env);
377 + types.push(env.get(operand.identifier.id) ?? {kind: 'None'});
378 }
239 - }
240 - break;
241 - }
242 - case 'CallExpression': {
243 - const callee = instr.value.callee;
244 - const isUseEffect = isEffectHook(callee.identifier);
245 - if (!isUseEffect) {
246 - // Report a more precise error when calling a local function that accesses a ref
247 - if (state.refAccessingFunctions.has(callee.identifier.id)) {
248 - errors.push({
249 - severity: ErrorSeverity.InvalidReact,
250 - reason:
251 - 'This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)',
252 - loc: callee.loc,
253 - description:
254 - callee.identifier.name !== null &&
255 - callee.identifier.name.kind === 'named'
256 - ? `Function \`${callee.identifier.name.value}\` accesses a ref`
257 - : null,
258 - suggestions: null,
379 + const value = joinRefAccessTypes(...types);
380 + if (value.kind === 'None') {
381 + env.set(instr.lvalue.identifier.id, {kind: 'None'});
382 + } else {
383 + env.set(instr.lvalue.identifier.id, {
384 + kind: 'Structure',
385 + value,
386 + fn: null,
387 });
388 }
389 + break;
390 + }
391 + case 'PropertyDelete':
392 + case 'PropertyStore':
393 + case 'ComputedDelete':
394 + case 'ComputedStore': {
395 + validateNoRefAccess(errors, env, instr.value.object, instr.loc);
396 for (const operand of eachInstructionValueOperand(instr.value)) {
262 - validateNoRefAccess(
263 - errors,
264 - state,
265 - operand,
266 - state.refValues.get(operand.identifier.id) ?? operand.loc,
267 - );
397 + if (operand === instr.value.object) {
398 + continue;
399 + }
400 + validateNoRefValueAccess(errors, env, operand);
401 }
402 + break;
403 }
270 - break;
271 - }
272 - case 'ObjectExpression':
273 - case 'ArrayExpression': {
274 - for (const operand of eachInstructionValueOperand(instr.value)) {
275 - validateNoDirectRefValueAccess(errors, operand, state);
276 - if (state.refAccessingFunctions.has(operand.identifier.id)) {
277 - state.refAccessingFunctions.add(instr.lvalue.identifier.id);
278 - }
279 - if (state.refs.has(operand.identifier.id)) {
280 - state.refs.add(instr.lvalue.identifier.id);
281 - }
282 - const refValue = state.refValues.get(operand.identifier.id);
283 - if (refValue !== undefined) {
284 - state.refValues.set(instr.lvalue.identifier.id, refValue);
404 + case 'StartMemoize':
405 + case 'FinishMemoize':
406 + break;
407 + default: {
408 + for (const operand of eachInstructionValueOperand(instr.value)) {
409 + validateNoRefValueAccess(errors, env, operand);
410 }
411 + break;
412 }
287 - break;
413 }
289 - case 'PropertyDelete':
290 - case 'PropertyStore':
291 - case 'ComputedDelete':
292 - case 'ComputedStore': {
293 - validateNoRefAccess(
294 - errors,
295 - state,
296 - instr.value.object,
297 - state.refValues.get(instr.value.object.identifier.id) ?? instr.loc,
414 + if (isUseRefType(instr.lvalue.identifier)) {
415 + env.set(
416 + instr.lvalue.identifier.id,
417 + joinRefAccessTypes(
418 + env.get(instr.lvalue.identifier.id) ?? {kind: 'None'},
419 + {kind: 'Ref'},
420 + ),
421 );
299 - for (const operand of eachInstructionValueOperand(instr.value)) {
300 - if (operand === instr.value.object) {
301 - continue;
302 - }
303 - validateNoRefValueAccess(errors, state, operand);
304 - }
305 - break;
422 }
307 - case 'StartMemoize':
308 - case 'FinishMemoize':
309 - break;
310 - default: {
311 - for (const operand of eachInstructionValueOperand(instr.value)) {
312 - validateNoRefValueAccess(errors, state, operand);
313 - }
314 - break;
423 + if (isRefValueType(instr.lvalue.identifier)) {
424 + env.set(
425 + instr.lvalue.identifier.id,
426 + joinRefAccessTypes(
427 + env.get(instr.lvalue.identifier.id) ?? {kind: 'None'},
428 + {kind: 'RefValue', loc: instr.loc},
429 + ),
430 + );
431 }
432 }
317 - if (isUseRefType(instr.lvalue.identifier)) {
318 - state.refs.add(instr.lvalue.identifier.id);
319 - }
320 - if (
321 - isRefValueType(instr.lvalue.identifier) &&
322 - !state.refValues.has(instr.lvalue.identifier.id)
323 - ) {
324 - state.refValues.set(instr.lvalue.identifier.id, instr.loc);
433 + for (const operand of eachTerminalOperand(block.terminal)) {
434 + if (block.terminal.kind !== 'return') {
435 + validateNoRefValueAccess(errors, env, operand);
436 + } else {
437 + // Allow functions containing refs to be returned, but not direct ref values
438 + validateNoDirectRefValueAccess(errors, operand, env);
439 + returnValues.push(env.get(operand.identifier.id));
440 + }
441 }
442 }
327 - for (const operand of eachTerminalOperand(block.terminal)) {
328 - if (block.terminal.kind !== 'return') {
329 - validateNoRefValueAccess(errors, state, operand);
330 - } else {
331 - // Allow functions containing refs to be returned, but not direct ref values
332 - validateNoDirectRefValueAccess(errors, operand, state);
333 - }
443 +
444 + if (errors.hasErrors()) {
445 + return Err(errors);
446 }
447 }
448
337 - if (errors.hasErrors()) {
338 - return Err(errors);
339 - } else {
340 - return Ok(undefined);
449 + CompilerError.invariant(!env.hasChanged(), {
450 + reason: 'Ref type environment did not converge',
451 + loc: null,
452 + });
453 +
454 + return Ok(
455 + joinRefAccessTypes(
456 + ...returnValues.filter((env): env is RefAccessType => env !== undefined),
457 + ),
458 + );
459 +}
460 +
461 +function destructure(
462 + type: RefAccessType | undefined,
463 +): RefAccessType | undefined {
464 + if (type?.kind === 'Structure' && type.value !== null) {
465 + return destructure(type.value);
466 }
467 + return type;
468 }
469
470 function validateNoRefValueAccess(
471 errors: CompilerError,
346 - state: State,
472 + env: Env,
473 operand: Place,
474 ): void {
475 + const type = destructure(env.get(operand.identifier.id));
476 if (
350 - state.refValues.has(operand.identifier.id) ||
351 - state.refAccessingFunctions.has(operand.identifier.id)
477 + type?.kind === 'RefValue' ||
478 + (type?.kind === 'Structure' && type.fn?.readRefEffect)
479 ) {
480 errors.push({
481 severity: ErrorSeverity.InvalidReact,
482 reason:
483 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
357 - loc: state.refValues.get(operand.identifier.id) ?? operand.loc,
484 + loc: (type.kind === 'RefValue' && type.loc) || operand.loc,
485 description:
486 operand.identifier.name !== null &&
487 operand.identifier.name.kind === 'named'
@@ -367,20 +494,21 @@ function validateNoRefValueAccess(
494
495 function validateNoRefAccess(
496 errors: CompilerError,
370 - state: State,
497 + env: Env,
498 operand: Place,
499 loc: SourceLocation,
500 ): void {
501 + const type = destructure(env.get(operand.identifier.id));
502 if (
375 - state.refs.has(operand.identifier.id) ||
376 - state.refValues.has(operand.identifier.id) ||
377 - state.refAccessingFunctions.has(operand.identifier.id)
503 + type?.kind === 'Ref' ||
504 + type?.kind === 'RefValue' ||
505 + (type?.kind === 'Structure' && type.fn?.readRefEffect)
506 ) {
507 errors.push({
508 severity: ErrorSeverity.InvalidReact,
509 reason:
510 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
383 - loc: loc,
511 + loc: (type.kind === 'RefValue' && type.loc) || loc,
512 description:
513 operand.identifier.name !== null &&
514 operand.identifier.name.kind === 'named'
@@ -394,14 +522,15 @@ function validateNoRefAccess(
522 function validateNoDirectRefValueAccess(
523 errors: CompilerError,
524 operand: Place,
397 - state: State,
525 + env: Env,
526 ): void {
399 - if (state.refValues.has(operand.identifier.id)) {
527 + const type = destructure(env.get(operand.identifier.id));
528 + if (type?.kind === 'RefValue') {
529 errors.push({
530 severity: ErrorSeverity.InvalidReact,
531 reason:
532 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
404 - loc: state.refValues.get(operand.identifier.id) ?? operand.loc,
533 + loc: type.loc ?? operand.loc,
534 description:
535 operand.identifier.name !== null &&
536 operand.identifier.name.kind === 'named'
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-ref-for-later-mutation.expect.md new
+69
@@ -0,0 +1,69 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {useRef} from 'react';
6 +import {addOne} from 'shared-runtime';
7 +
8 +function useKeyCommand() {
9 + const currentPosition = useRef(0);
10 + const handleKey = direction => () => {
11 + const position = currentPosition.current;
12 + const nextPosition = direction === 'left' ? addOne(position) : position;
13 + currentPosition.current = nextPosition;
14 + };
15 + const moveLeft = {
16 + handler: handleKey('left'),
17 + };
18 + const moveRight = {
19 + handler: handleKey('right'),
20 + };
21 + return [moveLeft, moveRight];
22 +}
23 +
24 +export const FIXTURE_ENTRYPOINT = {
25 + fn: useKeyCommand,
26 + params: [],
27 +};
28 +
29 +```
30 +
31 +## Code
32 +
33 +```javascript
34 +import { c as _c } from "react/compiler-runtime";
35 +import { useRef } from "react";
36 +import { addOne } from "shared-runtime";
37 +
38 +function useKeyCommand() {
39 + const $ = _c(1);
40 + const currentPosition = useRef(0);
41 + let t0;
42 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
43 + const handleKey = (direction) => () => {
44 + const position = currentPosition.current;
45 + const nextPosition = direction === "left" ? addOne(position) : position;
46 + currentPosition.current = nextPosition;
47 + };
48 +
49 + const moveLeft = { handler: handleKey("left") };
50 +
51 + const moveRight = { handler: handleKey("right") };
52 +
53 + t0 = [moveLeft, moveRight];
54 + $[0] = t0;
55 + } else {
56 + t0 = $[0];
57 + }
58 + return t0;
59 +}
60 +
61 +export const FIXTURE_ENTRYPOINT = {
62 + fn: useKeyCommand,
63 + params: [],
64 +};
65 +
66 +```
67 +
68 +### Eval output
69 +(kind: ok) [{"handler":"[[ function params=0 ]]"},{"handler":"[[ function params=0 ]]"}]
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-ref-for-later-mutation.tsx renamed
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.capture-ref-for-mutation.expect.md renamed
+5 -5
@@ -13,10 +13,10 @@ function useKeyCommand() {
13 currentPosition.current = nextPosition;
14 };
15 const moveLeft = {
16 - handler: handleKey('left'),
16 + handler: handleKey('left')(),
17 };
18 const moveRight = {
19 - handler: handleKey('right'),
19 + handler: handleKey('right')(),
20 };
21 return [moveLeft, moveRight];
22 }
@@ -34,8 +34,8 @@ export const FIXTURE_ENTRYPOINT = {
34 ```
35 10 | };
36 11 | const moveLeft = {
37 -> 12 | handler: handleKey('left'),
38 - | ^^^^^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (12:12)
37 +> 12 | handler: handleKey('left')(),
38 + | ^^^^^^^^^^^^^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (12:12)
39
40 InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (12:12)
41
@@ -44,7 +44,7 @@ InvalidReact: This function accesses a ref value (the `current` property), which
44 InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (15:15)
45 13 | };
46 14 | const moveRight = {
47 - 15 | handler: handleKey('right'),
47 + 15 | handler: handleKey('right')(),
48 ```
49
50
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.capture-ref-for-mutation.tsx new
+23
@@ -0,0 +1,23 @@
1 +import {useRef} from 'react';
2 +import {addOne} from 'shared-runtime';
3 +
4 +function useKeyCommand() {
5 + const currentPosition = useRef(0);
6 + const handleKey = direction => () => {
7 + const position = currentPosition.current;
8 + const nextPosition = direction === 'left' ? addOne(position) : position;
9 + currentPosition.current = nextPosition;
10 + };
11 + const moveLeft = {
12 + handler: handleKey('left')(),
13 + };
14 + const moveRight = {
15 + handler: handleKey('right')(),
16 + };
17 + return [moveLeft, moveRight];
18 +}
19 +
20 +export const FIXTURE_ENTRYPOINT = {
21 + fn: useKeyCommand,
22 + params: [],
23 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/original-reactive-scopes-fork/capture-ref-for-later-mutation.expect.md new
+70
@@ -0,0 +1,70 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableReactiveScopesInHIR:false
6 +import {useRef} from 'react';
7 +import {addOne} from 'shared-runtime';
8 +
9 +function useKeyCommand() {
10 + const currentPosition = useRef(0);
11 + const handleKey = direction => () => {
12 + const position = currentPosition.current;
13 + const nextPosition = direction === 'left' ? addOne(position) : position;
14 + currentPosition.current = nextPosition;
15 + };
16 + const moveLeft = {
17 + handler: handleKey('left'),
18 + };
19 + const moveRight = {
20 + handler: handleKey('right'),
21 + };
22 + return [moveLeft, moveRight];
23 +}
24 +
25 +export const FIXTURE_ENTRYPOINT = {
26 + fn: useKeyCommand,
27 + params: [],
28 +};
29 +
30 +```
31 +
32 +## Code
33 +
34 +```javascript
35 +import { c as _c } from "react/compiler-runtime"; // @enableReactiveScopesInHIR:false
36 +import { useRef } from "react";
37 +import { addOne } from "shared-runtime";
38 +
39 +function useKeyCommand() {
40 + const $ = _c(1);
41 + const currentPosition = useRef(0);
42 + let t0;
43 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
44 + const handleKey = (direction) => () => {
45 + const position = currentPosition.current;
46 + const nextPosition = direction === "left" ? addOne(position) : position;
47 + currentPosition.current = nextPosition;
48 + };
49 +
50 + const moveLeft = { handler: handleKey("left") };
51 +
52 + const moveRight = { handler: handleKey("right") };
53 +
54 + t0 = [moveLeft, moveRight];
55 + $[0] = t0;
56 + } else {
57 + t0 = $[0];
58 + }
59 + return t0;
60 +}
61 +
62 +export const FIXTURE_ENTRYPOINT = {
63 + fn: useKeyCommand,
64 + params: [],
65 +};
66 +
67 +```
68 +
69 +### Eval output
70 +(kind: ok) [{"handler":"[[ function params=0 ]]"},{"handler":"[[ function params=0 ]]"}]
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/original-reactive-scopes-fork/capture-ref-for-later-mutation.tsx new
+24
@@ -0,0 +1,24 @@
1 +// @enableReactiveScopesInHIR:false
2 +import {useRef} from 'react';
3 +import {addOne} from 'shared-runtime';
4 +
5 +function useKeyCommand() {
6 + const currentPosition = useRef(0);
7 + const handleKey = direction => () => {
8 + const position = currentPosition.current;
9 + const nextPosition = direction === 'left' ? addOne(position) : position;
10 + currentPosition.current = nextPosition;
11 + };
12 + const moveLeft = {
13 + handler: handleKey('left'),
14 + };
15 + const moveRight = {
16 + handler: handleKey('right'),
17 + };
18 + return [moveLeft, moveRight];
19 +}
20 +
21 +export const FIXTURE_ENTRYPOINT = {
22 + fn: useKeyCommand,
23 + params: [],
24 +};