The library for web and native user interfaces.
Mirrored from https://github.com/247TaxiDriver/QOS-React.git
Synced Nov 10, 20:15 UTC
[compiler] Exclude refs and ref values from having mutable ranges
Summary: Refs, as stable values that the rules of react around mutability do not apply to, currently are treated as having mutable ranges, and through aliasing, this can extend the mutable range for other values and disrupt good memoization for those values. This PR excludes refs and their .current values from having mutable ranges. Note that this is unsafe if ref access is allowed in render: if a mutable value is assigned to ref.current and then ref.current is mutated later, we won't realize that the original mutable value's range extends. ghstack-source-id: e8f36ac25e2c9aadb0bf13bd8142e4593ee9f984 Pull Request resolved: https://github.com/facebook/react/pull/30713
Mike Vitousek committed
Aug 16, 2024 at 13:27 UTC
5030e08575c295ef352c5ae928e2366cc4765d32
17 files changed
+159
-130
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+4
@@ -1591,6 +1591,10 @@ export function isUseStateType(id: Identifier): boolean {
1591
return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInUseState';
1592
}
1593
1594
+export function isRefOrRefValue(id: Identifier): boolean {
1595
+ return isUseRefType(id) || isRefValueType(id);
1596
+}
1597
+
1598
export function isSetStateType(id: Identifier): boolean {
1599
return id.type.kind === 'Function' && id.type.shapeId === 'BuiltInSetState';
1600
}
compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts
+2
-3
@@ -14,8 +14,7 @@ import {
14
LoweredFunction,
15
Place,
16
ReactiveScopeDependency,
17
- isRefValueType,
18
- isUseRefType,
17
+ isRefOrRefValue,
18
makeInstructionId,
19
} from '../HIR';
20
import {deadCodeElimination} from '../Optimization';
@@ -139,7 +138,7 @@ function infer(
138
name = dep.identifier.name;
139
}
140
142
- if (isUseRefType(dep.identifier) || isRefValueType(dep.identifier)) {
141
+ if (isRefOrRefValue(dep.identifier)) {
142
/*
143
* TODO: this is a hack to ensure we treat functions which reference refs
144
* as having a capture and therefore being considered mutable. this ensures
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts
+8
-2
@@ -11,6 +11,7 @@ import {
11
Identifier,
12
InstructionId,
13
InstructionKind,
14
+ isRefOrRefValue,
15
makeInstructionId,
16
Place,
17
} from '../HIR/HIR';
@@ -66,7 +67,9 @@ import {assertExhaustive} from '../Utils/utils';
67
*/
68
69
function infer(place: Place, instrId: InstructionId): void {
69
- place.identifier.mutableRange.end = makeInstructionId(instrId + 1);
70
+ if (!isRefOrRefValue(place.identifier)) {
71
+ place.identifier.mutableRange.end = makeInstructionId(instrId + 1);
72
+ }
73
}
74
75
function inferPlace(
@@ -171,7 +174,10 @@ export function inferMutableLifetimes(
174
const declaration = contextVariableDeclarationInstructions.get(
175
instr.value.lvalue.place.identifier,
176
);
174
- if (declaration != null) {
177
+ if (
178
+ declaration != null &&
179
+ !isRefOrRefValue(instr.value.lvalue.place.identifier)
180
+ ) {
181
const range = instr.value.lvalue.place.identifier.mutableRange;
182
if (range.start === 0) {
183
range.start = declaration;
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForAlias.ts
+12
-3
@@ -5,7 +5,12 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
-import {HIRFunction, Identifier, InstructionId} from '../HIR/HIR';
8
+import {
9
+ HIRFunction,
10
+ Identifier,
11
+ InstructionId,
12
+ isRefOrRefValue,
13
+} from '../HIR/HIR';
14
import DisjointSet from '../Utils/DisjointSet';
15
16
export function inferMutableRangesForAlias(
@@ -19,7 +24,8 @@ export function inferMutableRangesForAlias(
24
* mutated.
25
*/
26
const mutatingIdentifiers = [...aliasSet].filter(
22
- id => id.mutableRange.end - id.mutableRange.start > 1,
27
+ id =>
28
+ id.mutableRange.end - id.mutableRange.start > 1 && !isRefOrRefValue(id),
29
);
30
31
if (mutatingIdentifiers.length > 0) {
@@ -36,7 +42,10 @@ export function inferMutableRangesForAlias(
42
* last mutation.
43
*/
44
for (const alias of aliasSet) {
39
- if (alias.mutableRange.end < lastMutatingInstructionId) {
45
+ if (
46
+ alias.mutableRange.end < lastMutatingInstructionId &&
47
+ !isRefOrRefValue(alias)
48
+ ) {
49
alias.mutableRange.end = lastMutatingInstructionId as InstructionId;
50
}
51
}
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
+3
-10
@@ -30,8 +30,7 @@ import {
30
isArrayType,
31
isMutableEffect,
32
isObjectType,
33
- isRefValueType,
34
- isUseRefType,
33
+ isRefOrRefValue,
34
} from '../HIR/HIR';
35
import {FunctionSignature} from '../HIR/ObjectShape';
36
import {
@@ -523,10 +522,7 @@ class InferenceState {
522
break;
523
}
524
case Effect.Mutate: {
526
- if (
527
- isRefValueType(place.identifier) ||
528
- isUseRefType(place.identifier)
529
- ) {
525
+ if (isRefOrRefValue(place.identifier)) {
526
// no-op: refs are validate via ValidateNoRefAccessInRender
527
} else if (valueKind.kind === ValueKind.Context) {
528
functionEffect = {
@@ -567,10 +563,7 @@ class InferenceState {
563
break;
564
}
565
case Effect.Store: {
570
- if (
571
- isRefValueType(place.identifier) ||
572
- isUseRefType(place.identifier)
573
- ) {
566
+ if (isRefOrRefValue(place.identifier)) {
567
// no-op: refs are validate via ValidateNoRefAccessInRender
568
} else if (valueKind.kind === ValueKind.Context) {
569
functionEffect = {
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
+2
-2
@@ -11,6 +11,7 @@ import {
11
IdentifierId,
12
Place,
13
SourceLocation,
14
+ isRefOrRefValue,
15
isRefValueType,
16
isUseRefType,
17
} from '../HIR';
@@ -231,8 +232,7 @@ function validateNoRefAccess(
232
loc: SourceLocation,
233
): void {
234
if (
234
- isRefValueType(operand.identifier) ||
235
- isUseRefType(operand.identifier) ||
235
+ isRefOrRefValue(operand.identifier) ||
236
refAccessingFunctions.has(operand.identifier.id)
237
) {
238
errors.push({
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-ref-for-later-mutation.expect.md
+14
-15
@@ -36,27 +36,26 @@ import { useRef } from "react";
36
import { addOne } from "shared-runtime";
37
38
function useKeyCommand() {
39
- const $ = _c(2);
39
+ const $ = _c(1);
40
const currentPosition = useRef(0);
41
- const handleKey = (direction) => () => {
42
- const position = currentPosition.current;
43
- const nextPosition = direction === "left" ? addOne(position) : position;
44
- currentPosition.current = nextPosition;
45
- };
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
47
- const moveLeft = { handler: handleKey("left") };
49
+ const moveLeft = { handler: handleKey("left") };
50
49
- const t0 = handleKey("right");
50
- let t1;
51
- if ($[0] !== t0) {
52
- t1 = { handler: t0 };
51
+ const moveRight = { handler: handleKey("right") };
52
+
53
+ t0 = [moveLeft, moveRight];
54
$[0] = t0;
54
- $[1] = t1;
55
} else {
56
- t1 = $[1];
56
+ t0 = $[0];
57
}
58
- const moveRight = t1;
59
- return [moveLeft, moveRight];
58
+ return t0;
59
}
60
61
export const FIXTURE_ENTRYPOINT = {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md
+4
-2
@@ -2,7 +2,7 @@
2
## Input
3
4
```javascript
5
-// @enablePreserveExistingMemoizationGuarantees
5
+// @enablePreserveExistingMemoizationGuarantees @validateRefAccessDuringRender
6
import {useCallback, useRef} from 'react';
7
8
function Component(props) {
@@ -42,7 +42,9 @@ export const FIXTURE_ENTRYPOINT = {
42
> 10 | ref.current.inner = event.target.value;
43
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44
> 11 | });
45
- | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. (7:11)
45
+ | ^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $44:TObject<BuiltInFunction> (7:11)
46
+
47
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14)
48
12 |
49
13 | // The ref is modified later, extending its range and preventing memoization of onChange
50
14 | ref.current.inner = null;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.js
+1
-1
@@ -1,4 +1,4 @@
1
-// @enablePreserveExistingMemoizationGuarantees
1
+// @enablePreserveExistingMemoizationGuarantees @validateRefAccessDuringRender
2
import {useCallback, useRef} from 'react';
3
4
function Component(props) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md
renamed
+6
-2
@@ -2,7 +2,7 @@
2
## Input
3
4
```javascript
5
-// @enablePreserveExistingMemoizationGuarantees
5
+// @enablePreserveExistingMemoizationGuarantees @validateRefAccessDuringRender
6
import {useCallback, useRef} from 'react';
7
8
function Component(props) {
@@ -45,7 +45,11 @@ export const FIXTURE_ENTRYPOINT = {
45
> 10 | ref.current.inner = event.target.value;
46
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47
> 11 | });
48
- | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. (7:11)
48
+ | ^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $53:TObject<BuiltInFunction> (7:11)
49
+
50
+InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef). Function mutate? $77[20:22]:TObject<BuiltInFunction> accesses a ref (17:17)
51
+
52
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (17:17)
53
12 |
54
13 | // The ref is modified later, extending its range and preventing memoization of onChange
55
14 | const reset = () => {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.js
renamed
+1
-1
@@ -1,4 +1,4 @@
1
-// @enablePreserveExistingMemoizationGuarantees
1
+// @enablePreserveExistingMemoizationGuarantees @validateRefAccessDuringRender
2
import {useCallback, useRef} from 'react';
3
4
function Component(props) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/original-reactive-scopes-fork/capture-ref-for-later-mutation.expect.md
+13
-29
@@ -37,42 +37,26 @@ import { useRef } from "react";
37
import { addOne } from "shared-runtime";
38
39
function useKeyCommand() {
40
- const $ = _c(6);
40
+ const $ = _c(1);
41
const currentPosition = useRef(0);
42
- const handleKey = (direction) => () => {
43
- const position = currentPosition.current;
44
- const nextPosition = direction === "left" ? addOne(position) : position;
45
- currentPosition.current = nextPosition;
46
- };
42
let t0;
43
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
49
- t0 = { handler: handleKey("left") };
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
}
54
- const moveLeft = t0;
55
-
56
- const t1 = handleKey("right");
57
- let t2;
58
- if ($[1] !== t1) {
59
- t2 = { handler: t1 };
60
- $[1] = t1;
61
- $[2] = t2;
62
- } else {
63
- t2 = $[2];
64
- }
65
- const moveRight = t2;
66
- let t3;
67
- if ($[3] !== moveLeft || $[4] !== moveRight) {
68
- t3 = [moveLeft, moveRight];
69
- $[3] = moveLeft;
70
- $[4] = moveRight;
71
- $[5] = t3;
72
- } else {
73
- t3 = $[5];
74
- }
75
- return t3;
59
+ return t0;
60
}
61
62
export const FIXTURE_ENTRYPOINT = {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-mutable-ref-not-preserved.expect.md
deleted
-35
@@ -1,35 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-// @validatePreserveExistingMemoizationGuarantees:true
6
-
7
-import {useRef, useMemo} from 'react';
8
-import {makeArray} from 'shared-runtime';
9
-
10
-function useFoo() {
11
- const r = useRef();
12
- return useMemo(() => makeArray(r), []);
13
-}
14
-
15
-export const FIXTURE_ENTRYPOINT = {
16
- fn: useFoo,
17
- params: [],
18
-};
19
-
20
-```
21
-
22
-
23
-## Error
24
-
25
-```
26
- 6 | function useFoo() {
27
- 7 | const r = useRef();
28
-> 8 | return useMemo(() => makeArray(r), []);
29
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. (8:8)
30
- 9 | }
31
- 10 |
32
- 11 | export const FIXTURE_ENTRYPOINT = {
33
-```
34
-
35
-
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.expect.md
new
+53
@@ -0,0 +1,53 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validatePreserveExistingMemoizationGuarantees:true
6
+
7
+import {useRef, useMemo} from 'react';
8
+import {makeArray} from 'shared-runtime';
9
+
10
+function useFoo() {
11
+ const r = useRef();
12
+ return useMemo(() => makeArray(r), []);
13
+}
14
+
15
+export const FIXTURE_ENTRYPOINT = {
16
+ fn: useFoo,
17
+ params: [],
18
+};
19
+
20
+```
21
+
22
+## Code
23
+
24
+```javascript
25
+import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees:true
26
+
27
+import { useRef, useMemo } from "react";
28
+import { makeArray } from "shared-runtime";
29
+
30
+function useFoo() {
31
+ const $ = _c(1);
32
+ const r = useRef();
33
+ let t0;
34
+ let t1;
35
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
36
+ t1 = makeArray(r);
37
+ $[0] = t1;
38
+ } else {
39
+ t1 = $[0];
40
+ }
41
+ t0 = t1;
42
+ return t0;
43
+}
44
+
45
+export const FIXTURE_ENTRYPOINT = {
46
+ fn: useFoo,
47
+ params: [],
48
+};
49
+
50
+```
51
+
52
+### Eval output
53
+(kind: ok) [{}]
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/maybe-mutable-ref-not-preserved.ts
renamed
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-ref-mutable-range.expect.md
+20
-14
@@ -31,7 +31,7 @@ import { c as _c } from "react/compiler-runtime";
31
import { Stringify, identity, mutate, CONST_TRUE } from "shared-runtime";
32
33
function Foo(props, ref) {
34
- const $ = _c(5);
34
+ const $ = _c(7);
35
let value;
36
let t0;
37
if ($[0] !== ref) {
@@ -45,19 +45,6 @@ function Foo(props, ref) {
45
}
46
47
mutate(value);
48
- if (CONST_TRUE) {
49
- const t1 = identity(ref);
50
- let t2;
51
- if ($[3] !== t1) {
52
- t2 = <Stringify ref={t1} />;
53
- $[3] = t1;
54
- $[4] = t2;
55
- } else {
56
- t2 = $[4];
57
- }
58
- t0 = t2;
59
- break bb0;
60
- }
48
}
49
$[0] = ref;
50
$[1] = value;
@@ -69,6 +56,25 @@ function Foo(props, ref) {
56
if (t0 !== Symbol.for("react.early_return_sentinel")) {
57
return t0;
58
}
59
+ if (CONST_TRUE) {
60
+ let t1;
61
+ if ($[3] !== ref) {
62
+ t1 = identity(ref);
63
+ $[3] = ref;
64
+ $[4] = t1;
65
+ } else {
66
+ t1 = $[4];
67
+ }
68
+ let t2;
69
+ if ($[5] !== t1) {
70
+ t2 = <Stringify ref={t1} />;
71
+ $[5] = t1;
72
+ $[6] = t2;
73
+ } else {
74
+ t2 = $[6];
75
+ }
76
+ return t2;
77
+ }
78
return value;
79
}
80
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-set-ref-nested-property-dont-preserve-memoization.expect.md
+16
-11
@@ -42,21 +42,26 @@ function Component(props) {
42
t0 = $[0];
43
}
44
const ref = useRef(t0);
45
-
46
- const onChange = (event) => {
47
- ref.current.inner = event.target.value;
48
- };
45
+ let t1;
46
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
47
+ t1 = (event) => {
48
+ ref.current.inner = event.target.value;
49
+ };
50
+ $[1] = t1;
51
+ } else {
52
+ t1 = $[1];
53
+ }
54
+ const onChange = t1;
55
56
ref.current.inner = null;
51
- let t1;
52
- if ($[1] !== onChange) {
53
- t1 = <input onChange={onChange} />;
54
- $[1] = onChange;
55
- $[2] = t1;
57
+ let t2;
58
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
59
+ t2 = <input onChange={onChange} />;
60
+ $[2] = t2;
61
} else {
57
- t1 = $[2];
62
+ t2 = $[2];
63
}
59
- return t1;
64
+ return t2;
65
}
66
67
export const FIXTURE_ENTRYPOINT = {