@samitouri / QOS-React-1 / commits / 83d538e0d0

[compiler] Treat ref-like named objects as refs (#29916)

If a component uses the `useRef` hook directly then we type it's return value as a ref. But if it's wrapped in a custom hook then we lose out on this type information as the compiler doesn't look at the hook definition. This has resulted in some false positives in our analysis like the ones reported in #29160 and #29196. This PR will treat objects named as `ref` or if their names end with the substring `Ref`, and contain a property named `current`, as React refs. ``` const ref = useMyRef(); const myRef = useMyRef2(); useEffect(() => { ref.current = ...; myRef.current = ...; }) ``` In the above example, `ref` and `myRef` will be treated as React refs.

Sathya Gunasekaran committed Jun 18, 2024 at 18:02 UTC 83d538e0d04aac2a32d335e6b3963273729a9fe6
13 files changed +525 -10
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
+17
@@ -407,6 +407,23 @@ const EnvironmentConfigSchema = z.object({
407 * and identifiers have been changed.
408 */
409 hookPattern: z.string().nullable().default(null),
410 +
411 + /**
412 + * If enabled, this will treat objects named as `ref` or if their names end with the substring `Ref`,
413 + * and contain a property named `current`, as React refs.
414 + *
415 + * ```
416 + * const ref = useMyRef();
417 + * const myRef = useMyRef2();
418 + * useEffect(() => {
419 + * ref.current = ...;
420 + * myRef.current = ...;
421 + * })
422 + * ```
423 + *
424 + * Here the variables `ref` and `myRef` will be typed as Refs.
425 + */
426 + enableTreatRefLikeIdentifiersAsRefs: z.boolean().nullable().default(false),
427 });
428
429 export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts
+8 -4
@@ -57,7 +57,8 @@ export type PhiType = {
57 };
58 export type PropType = {
59 kind: "Property";
60 - object: Type;
60 + objectType: Type;
61 + objectName: string;
62 propertyName: string;
63 };
64
@@ -124,7 +125,8 @@ export function duplicateType(type: Type): Type {
125 case "Property": {
126 return {
127 kind: "Property",
127 - object: duplicateType(type.object),
128 + objectType: duplicateType(type.objectType),
129 + objectName: type.objectName,
130 propertyName: type.propertyName,
131 };
132 }
@@ -165,11 +167,13 @@ function objectMethodTypeEquals(tA: Type, tB: Type): boolean {
167
168 function propTypeEquals(tA: Type, tB: Type): boolean {
169 if (tA.kind === "Property" && tB.kind === "Property") {
168 - if (!typeEquals(tA.object, tB.object)) {
170 + if (!typeEquals(tA.objectType, tB.objectType)) {
171 return false;
172 }
173
172 - return tA.propertyName === tB.propertyName;
174 + return (
175 + tA.propertyName === tB.propertyName && tA.objectName === tB.objectName
176 + );
177 }
178
179 return false;
compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts
+50 -6
@@ -11,8 +11,11 @@ import { Environment } from "../HIR";
11 import { lowerType } from "../HIR/BuildHIR";
12 import {
13 HIRFunction,
14 + Identifier,
15 + IdentifierId,
16 Instruction,
17 makeType,
18 + PropType,
19 Type,
20 typeEquals,
21 TypeId,
@@ -24,6 +27,7 @@ import {
27 BuiltInJsxId,
28 BuiltInObjectId,
29 BuiltInPropsId,
30 + BuiltInRefValueId,
31 BuiltInUseRefId,
32 } from "../HIR/ObjectShape";
33 import { eachInstructionLValue, eachInstructionOperand } from "../HIR/visitors";
@@ -117,6 +121,7 @@ function* generate(
121 }
122 }
123
124 + const names = new Map();
125 for (const [_, block] of func.body.blocks) {
126 for (const phi of block.phis) {
127 yield equation(phi.type, {
@@ -126,13 +131,28 @@ function* generate(
131 }
132
133 for (const instr of block.instructions) {
129 - yield* generateInstructionTypes(func.env, instr);
134 + yield* generateInstructionTypes(func.env, names, instr);
135 }
136 }
137 }
138
139 +function setName(
140 + names: Map<IdentifierId, string>,
141 + id: IdentifierId,
142 + name: Identifier
143 +): void {
144 + if (name.name?.kind === "named") {
145 + names.set(id, name.name.value);
146 + }
147 +}
148 +
149 +function getName(names: Map<IdentifierId, string>, id: IdentifierId): string {
150 + return names.get(id) ?? "";
151 +}
152 +
153 function* generateInstructionTypes(
154 env: Environment,
155 + names: Map<IdentifierId, string>,
156 instr: Instruction
157 ): Generator<TypeEquation, void, undefined> {
158 const { lvalue, value } = instr;
@@ -152,6 +172,7 @@ function* generateInstructionTypes(
172 }
173
174 case "LoadLocal": {
175 + setName(names, lvalue.identifier.id, value.place.identifier);
176 yield equation(left, value.place.identifier.type);
177 break;
178 }
@@ -250,7 +271,8 @@ function* generateInstructionTypes(
271 case "PropertyLoad": {
272 yield equation(left, {
273 kind: "Property",
253 - object: value.object.identifier.type,
274 + objectType: value.object.identifier.type,
275 + objectName: getName(names, value.object.identifier.id),
276 propertyName: value.property,
277 });
278 break;
@@ -278,7 +300,8 @@ function* generateInstructionTypes(
300 const propertyName = String(i);
301 yield equation(item.identifier.type, {
302 kind: "Property",
281 - object: value.value.identifier.type,
303 + objectType: value.value.identifier.type,
304 + objectName: getName(names, value.value.identifier.id),
305 propertyName,
306 });
307 } else {
@@ -294,7 +317,8 @@ function* generateInstructionTypes(
317 ) {
318 yield equation(property.place.identifier.type, {
319 kind: "Property",
297 - object: value.value.identifier.type,
320 + objectType: value.value.identifier.type,
321 + objectName: getName(names, value.value.identifier.id),
322 propertyName: property.key.name,
323 });
324 }
@@ -342,11 +366,11 @@ function* generateInstructionTypes(
366 yield equation(left, { kind: "Object", shapeId: BuiltInJsxId });
367 break;
368 }
369 + case "PropertyStore":
370 case "DeclareLocal":
371 case "NewExpression":
372 case "RegExpLiteral":
373 case "MetaProperty":
349 - case "PropertyStore":
374 case "ComputedStore":
375 case "ComputedLoad":
376 case "TaggedTemplateExpression":
@@ -375,7 +399,21 @@ class Unifier {
399
400 unify(tA: Type, tB: Type): void {
401 if (tB.kind === "Property") {
378 - const objectType = this.get(tB.object);
402 + if (
403 + this.env.config.enableTreatRefLikeIdentifiersAsRefs &&
404 + isRefLikeName(tB)
405 + ) {
406 + this.unify(tB.objectType, {
407 + kind: "Object",
408 + shapeId: BuiltInUseRefId,
409 + });
410 + this.unify(tA, {
411 + kind: "Object",
412 + shapeId: BuiltInRefValueId,
413 + });
414 + return;
415 + }
416 + const objectType = this.get(tB.objectType);
417 const propertyType = this.env.getPropertyType(
418 objectType,
419 tB.propertyName
@@ -483,3 +521,9 @@ class Unifier {
521 return type;
522 }
523 }
524 +
525 +const RefLikeNameRE = /^(?:[a-zA-Z$_][a-zA-Z$_0-9]*)Ref$|^ref$/;
526 +
527 +function isRefLikeName(t: PropType): boolean {
528 + return RefLikeNameRE.test(t.objectName) && t.propertyName === "current";
529 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md new
+47
@@ -0,0 +1,47 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validatePreserveExistingMemoizationGuarantees
6 +import { useCallback, useRef } from "react";
7 +
8 +function useCustomRef() {
9 + return useRef({ click: () => {} });
10 +}
11 +
12 +function Foo() {
13 + const Ref = useCustomRef();
14 +
15 + const onClick = useCallback(() => {
16 + Ref.current?.click();
17 + }, []);
18 +
19 + return <button onClick={onClick} />;
20 +}
21 +
22 +export const FIXTURE_ENTRYPOINT = {
23 + fn: Foo,
24 + params: [],
25 + isComponent: true,
26 +};
27 +
28 +```
29 +
30 +
31 +## Error
32 +
33 +```
34 + 9 | const Ref = useCustomRef();
35 + 10 |
36 +> 11 | const onClick = useCallback(() => {
37 + | ^^^^^^^
38 +> 12 | Ref.current?.click();
39 + | ^^^^^^^^^^^^^^^^^^^^^^^^^
40 +> 13 | }, []);
41 + | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (11:13)
42 + 14 |
43 + 15 | return <button onClick={onClick} />;
44 + 16 | }
45 +```
46 +
47 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.js new
+22
@@ -0,0 +1,22 @@
1 +// @validatePreserveExistingMemoizationGuarantees
2 +import { useCallback, useRef } from "react";
3 +
4 +function useCustomRef() {
5 + return useRef({ click: () => {} });
6 +}
7 +
8 +function Foo() {
9 + const Ref = useCustomRef();
10 +
11 + const onClick = useCallback(() => {
12 + Ref.current?.click();
13 + }, []);
14 +
15 + return <button onClick={onClick} />;
16 +}
17 +
18 +export const FIXTURE_ENTRYPOINT = {
19 + fn: Foo,
20 + params: [],
21 + isComponent: true,
22 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md new
+47
@@ -0,0 +1,47 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validatePreserveExistingMemoizationGuarantees
6 +import { useCallback, useRef } from "react";
7 +
8 +function useCustomRef() {
9 + return useRef({ click: () => {} });
10 +}
11 +
12 +function Foo() {
13 + const notaref = useCustomRef();
14 +
15 + const onClick = useCallback(() => {
16 + notaref.current?.click();
17 + }, []);
18 +
19 + return <button onClick={onClick} />;
20 +}
21 +
22 +export const FIXTURE_ENTRYPOINT = {
23 + fn: Foo,
24 + params: [],
25 + isComponent: true,
26 +};
27 +
28 +```
29 +
30 +
31 +## Error
32 +
33 +```
34 + 9 | const notaref = useCustomRef();
35 + 10 |
36 +> 11 | const onClick = useCallback(() => {
37 + | ^^^^^^^
38 +> 12 | notaref.current?.click();
39 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40 +> 13 | }, []);
41 + | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (11:13)
42 + 14 |
43 + 15 | return <button onClick={onClick} />;
44 + 16 | }
45 +```
46 +
47 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.js new
+22
@@ -0,0 +1,22 @@
1 +// @validatePreserveExistingMemoizationGuarantees
2 +import { useCallback, useRef } from "react";
3 +
4 +function useCustomRef() {
5 + return useRef({ click: () => {} });
6 +}
7 +
8 +function Foo() {
9 + const notaref = useCustomRef();
10 +
11 + const onClick = useCallback(() => {
12 + notaref.current?.click();
13 + }, []);
14 +
15 + return <button onClick={onClick} />;
16 +}
17 +
18 +export const FIXTURE_ENTRYPOINT = {
19 + fn: Foo,
20 + params: [],
21 + isComponent: true,
22 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md new
+84
@@ -0,0 +1,84 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
6 +import { useRef, useEffect } from "react";
7 +
8 +function useCustomRef() {
9 + return useRef({ click: () => {} });
10 +}
11 +
12 +function Foo() {
13 + const ref = useCustomRef();
14 +
15 + useEffect(() => {
16 + ref.current?.click();
17 + }, []);
18 +
19 + return <div>foo</div>;
20 +}
21 +
22 +export const FIXTURE_ENTRYPOINT = {
23 + fn: Foo,
24 + params: [],
25 + isComponent: true,
26 +};
27 +
28 +```
29 +
30 +## Code
31 +
32 +```javascript
33 +import { c as _c } from "react/compiler-runtime"; // @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
34 +import { useRef, useEffect } from "react";
35 +
36 +function useCustomRef() {
37 + const $ = _c(1);
38 + let t0;
39 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = { click: () => {} };
41 + $[0] = t0;
42 + } else {
43 + t0 = $[0];
44 + }
45 + return useRef(t0);
46 +}
47 +
48 +function Foo() {
49 + const $ = _c(3);
50 + const ref = useCustomRef();
51 + let t0;
52 + let t1;
53 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
54 + t0 = () => {
55 + ref.current?.click();
56 + };
57 + t1 = [];
58 + $[0] = t0;
59 + $[1] = t1;
60 + } else {
61 + t0 = $[0];
62 + t1 = $[1];
63 + }
64 + useEffect(t0, t1);
65 + let t2;
66 + if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
67 + t2 = <div>foo</div>;
68 + $[2] = t2;
69 + } else {
70 + t2 = $[2];
71 + }
72 + return t2;
73 +}
74 +
75 +export const FIXTURE_ENTRYPOINT = {
76 + fn: Foo,
77 + params: [],
78 + isComponent: true,
79 +};
80 +
81 +```
82 +
83 +### Eval output
84 +(kind: ok) <div>foo</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.js new
+22
@@ -0,0 +1,22 @@
1 +// @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
2 +import { useRef, useEffect } from "react";
3 +
4 +function useCustomRef() {
5 + return useRef({ click: () => {} });
6 +}
7 +
8 +function Foo() {
9 + const ref = useCustomRef();
10 +
11 + useEffect(() => {
12 + ref.current?.click();
13 + }, []);
14 +
15 + return <div>foo</div>;
16 +}
17 +
18 +export const FIXTURE_ENTRYPOINT = {
19 + fn: Foo,
20 + params: [],
21 + isComponent: true,
22 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md new
+81
@@ -0,0 +1,81 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
6 +import { useRef, useCallback } from "react";
7 +
8 +function useCustomRef() {
9 + return useRef({ click: () => {} });
10 +}
11 +
12 +function Foo() {
13 + const ref = useCustomRef();
14 +
15 + const onClick = useCallback(() => {
16 + ref.current?.click();
17 + }, []);
18 +
19 + return <button onClick={onClick} />;
20 +}
21 +
22 +export const FIXTURE_ENTRYPOINT = {
23 + fn: Foo,
24 + params: [],
25 + isComponent: true,
26 +};
27 +
28 +```
29 +
30 +## Code
31 +
32 +```javascript
33 +import { c as _c } from "react/compiler-runtime"; // @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
34 +import { useRef, useCallback } from "react";
35 +
36 +function useCustomRef() {
37 + const $ = _c(1);
38 + let t0;
39 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = { click: () => {} };
41 + $[0] = t0;
42 + } else {
43 + t0 = $[0];
44 + }
45 + return useRef(t0);
46 +}
47 +
48 +function Foo() {
49 + const $ = _c(3);
50 + const ref = useCustomRef();
51 + let t0;
52 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53 + t0 = () => {
54 + ref.current?.click();
55 + };
56 + $[0] = t0;
57 + } else {
58 + t0 = $[0];
59 + }
60 + const onClick = t0;
61 + let t1;
62 + if ($[1] !== onClick) {
63 + t1 = <button onClick={onClick} />;
64 + $[1] = onClick;
65 + $[2] = t1;
66 + } else {
67 + t1 = $[2];
68 + }
69 + return t1;
70 +}
71 +
72 +export const FIXTURE_ENTRYPOINT = {
73 + fn: Foo,
74 + params: [],
75 + isComponent: true,
76 +};
77 +
78 +```
79 +
80 +### Eval output
81 +(kind: ok) <button></button>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.js new
+22
@@ -0,0 +1,22 @@
1 +// @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
2 +import { useRef, useCallback } from "react";
3 +
4 +function useCustomRef() {
5 + return useRef({ click: () => {} });
6 +}
7 +
8 +function Foo() {
9 + const ref = useCustomRef();
10 +
11 + const onClick = useCallback(() => {
12 + ref.current?.click();
13 + }, []);
14 +
15 + return <button onClick={onClick} />;
16 +}
17 +
18 +export const FIXTURE_ENTRYPOINT = {
19 + fn: Foo,
20 + params: [],
21 + isComponent: true,
22 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback.expect.md new
+81
@@ -0,0 +1,81 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
6 +import { useRef, useCallback } from "react";
7 +
8 +function useCustomRef() {
9 + return useRef({ click: () => {} });
10 +}
11 +
12 +function Foo() {
13 + const customRef = useCustomRef();
14 +
15 + const onClick = useCallback(() => {
16 + customRef.current?.click();
17 + }, []);
18 +
19 + return <button onClick={onClick} />;
20 +}
21 +
22 +export const FIXTURE_ENTRYPOINT = {
23 + fn: Foo,
24 + params: [],
25 + isComponent: true,
26 +};
27 +
28 +```
29 +
30 +## Code
31 +
32 +```javascript
33 +import { c as _c } from "react/compiler-runtime"; // @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
34 +import { useRef, useCallback } from "react";
35 +
36 +function useCustomRef() {
37 + const $ = _c(1);
38 + let t0;
39 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = { click: () => {} };
41 + $[0] = t0;
42 + } else {
43 + t0 = $[0];
44 + }
45 + return useRef(t0);
46 +}
47 +
48 +function Foo() {
49 + const $ = _c(3);
50 + const customRef = useCustomRef();
51 + let t0;
52 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53 + t0 = () => {
54 + customRef.current?.click();
55 + };
56 + $[0] = t0;
57 + } else {
58 + t0 = $[0];
59 + }
60 + const onClick = t0;
61 + let t1;
62 + if ($[1] !== onClick) {
63 + t1 = <button onClick={onClick} />;
64 + $[1] = onClick;
65 + $[2] = t1;
66 + } else {
67 + t1 = $[2];
68 + }
69 + return t1;
70 +}
71 +
72 +export const FIXTURE_ENTRYPOINT = {
73 + fn: Foo,
74 + params: [],
75 + isComponent: true,
76 +};
77 +
78 +```
79 +
80 +### Eval output
81 +(kind: ok) <button></button>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback.js new
+22
@@ -0,0 +1,22 @@
1 +// @enableTreatRefLikeIdentifiersAsRefs @validatePreserveExistingMemoizationGuarantees
2 +import { useRef, useCallback } from "react";
3 +
4 +function useCustomRef() {
5 + return useRef({ click: () => {} });
6 +}
7 +
8 +function Foo() {
9 + const customRef = useCustomRef();
10 +
11 + const onClick = useCallback(() => {
12 + customRef.current?.click();
13 + }, []);
14 +
15 + return <button onClick={onClick} />;
16 +}
17 +
18 +export const FIXTURE_ENTRYPOINT = {
19 + fn: Foo,
20 + params: [],
21 + isComponent: true,
22 +};