[compiler][repro] False positives for ValidatePreserveMemo
ghstack-source-id: 7fa94fea867371f3d77737dad80a321094f10600 Pull Request resolved: https://github.com/facebook/react/pull/30629
Mofei Zhang committed
Aug 7, 2024 at 16:11 UTC
2f8ff3deb29ec869a8ce8744d15afd6fb1421f9b
6 files changed
+235
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-extended-contextvar-scope.expect.md
new
+57
@@ -0,0 +1,57 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validatePreserveExistingMemoizationGuarantees:true
6
+
7
+import {useCallback} from 'react';
8
+import {Stringify, useIdentity} from 'shared-runtime';
9
+
10
+/**
11
+ * Here, the *inferred* dependencies of cb are `a` and `t1 = LoadContext capture x_@1`.
12
+ * - t1 does not have a scope as it captures `x` after x's mutable range
13
+ * - `x` is a context variable, which means its mutable range extends to all
14
+ * references / aliases.
15
+ * - `a`, `b`, and `x` get the same mutable range due to potential aliasing.
16
+ *
17
+ * We currently bail out because `a` has a scope and is not transitively memoized
18
+ * (as its scope is pruned due to a hook call)
19
+ */
20
+function useBar({a, b}, cond) {
21
+ let x = useIdentity({val: 3});
22
+ if (cond) {
23
+ x = b;
24
+ }
25
+
26
+ const cb = useCallback(() => {
27
+ return [a, x];
28
+ }, [a, x]);
29
+
30
+ return <Stringify cb={cb} shouldInvoke={true} />;
31
+}
32
+
33
+export const FIXTURE_ENTRYPOINT = {
34
+ fn: useBar,
35
+ params: [{a: 1, b: 2}, true],
36
+};
37
+
38
+```
39
+
40
+
41
+## Error
42
+
43
+```
44
+ 20 | }
45
+ 21 |
46
+> 22 | const cb = useCallback(() => {
47
+ | ^^^^^^^
48
+> 23 | return [a, x];
49
+ | ^^^^^^^^^^^^^^^^^^
50
+> 24 | }, [a, x]);
51
+ | ^^^^ 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. (22:24)
52
+ 25 |
53
+ 26 | return <Stringify cb={cb} shouldInvoke={true} />;
54
+ 27 | }
55
+```
56
+
57
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-extended-contextvar-scope.tsx
new
+32
@@ -0,0 +1,32 @@
1
+// @validatePreserveExistingMemoizationGuarantees:true
2
+
3
+import {useCallback} from 'react';
4
+import {Stringify, useIdentity} from 'shared-runtime';
5
+
6
+/**
7
+ * Here, the *inferred* dependencies of cb are `a` and `t1 = LoadContext capture x_@1`.
8
+ * - t1 does not have a scope as it captures `x` after x's mutable range
9
+ * - `x` is a context variable, which means its mutable range extends to all
10
+ * references / aliases.
11
+ * - `a`, `b`, and `x` get the same mutable range due to potential aliasing.
12
+ *
13
+ * We currently bail out because `a` has a scope and is not transitively memoized
14
+ * (as its scope is pruned due to a hook call)
15
+ */
16
+function useBar({a, b}, cond) {
17
+ let x = useIdentity({val: 3});
18
+ if (cond) {
19
+ x = b;
20
+ }
21
+
22
+ const cb = useCallback(() => {
23
+ return [a, x];
24
+ }, [a, x]);
25
+
26
+ return <Stringify cb={cb} shouldInvoke={true} />;
27
+}
28
+
29
+export const FIXTURE_ENTRYPOINT = {
30
+ fn: useBar,
31
+ params: [{a: 1, b: 2}, true],
32
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-dep-scope-pruned.expect.md
new
+42
@@ -0,0 +1,42 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validatePreserveExistingMemoizationGuarantees
6
+import {useCallback} from 'react';
7
+import {identity, useIdentity} from 'shared-runtime';
8
+
9
+/**
10
+ * Repro showing a manual memo whose declaration (useCallback's 1st argument)
11
+ * is memoized, but not its dependency (x). In this case, `x`'s scope is pruned
12
+ * due to hook-call flattening.
13
+ */
14
+function useFoo(a) {
15
+ const x = identity(a);
16
+ useIdentity(2);
17
+ mutate(x);
18
+
19
+ return useCallback(() => [x, []], [x]);
20
+}
21
+
22
+export const FIXTURE_ENTRYPOINT = {
23
+ fn: useFoo,
24
+ params: [3],
25
+};
26
+
27
+```
28
+
29
+
30
+## Error
31
+
32
+```
33
+ 13 | mutate(x);
34
+ 14 |
35
+> 15 | return useCallback(() => [x, []], [x]);
36
+ | ^^^^^^^^^^^^^ 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. (15:15)
37
+ 16 | }
38
+ 17 |
39
+ 18 | export const FIXTURE_ENTRYPOINT = {
40
+```
41
+
42
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-dep-scope-pruned.ts
new
+21
@@ -0,0 +1,21 @@
1
+// @validatePreserveExistingMemoizationGuarantees
2
+import {useCallback} from 'react';
3
+import {identity, useIdentity} from 'shared-runtime';
4
+
5
+/**
6
+ * Repro showing a manual memo whose declaration (useCallback's 1st argument)
7
+ * is memoized, but not its dependency (x). In this case, `x`'s scope is pruned
8
+ * due to hook-call flattening.
9
+ */
10
+function useFoo(a) {
11
+ const x = identity(a);
12
+ useIdentity(2);
13
+ mutate(x);
14
+
15
+ return useCallback(() => [x, []], [x]);
16
+}
17
+
18
+export const FIXTURE_ENTRYPOINT = {
19
+ fn: useFoo,
20
+ params: [3],
21
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-overlap-scopes.expect.md
new
+52
@@ -0,0 +1,52 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validatePreserveExistingMemoizationGuarantees:true
6
+import {useMemo} from 'react';
7
+import {arrayPush} from 'shared-runtime';
8
+
9
+/**
10
+ * Repro showing differences between mutable ranges and scope ranges.
11
+ *
12
+ * For useMemo dependency `x`:
13
+ * - mutable range ends after the `arrayPush(x, b)` instruction
14
+ * - scope range is extended due to MergeOverlappingScopes
15
+ *
16
+ * Since manual memo deps are guaranteed to be named (guaranteeing valid
17
+ * codegen), it's correct to take a dependency on a dep *before* the end
18
+ * of its scope (but after its mutable range ends).
19
+ */
20
+
21
+function useFoo(a, b) {
22
+ const x = [];
23
+ const y = [];
24
+ arrayPush(x, b);
25
+ const result = useMemo(() => {
26
+ return [Math.max(x[1], a)];
27
+ }, [a, x]);
28
+ arrayPush(y, 3);
29
+ return {result, y};
30
+}
31
+
32
+export const FIXTURE_ENTRYPOINT = {
33
+ fn: useFoo,
34
+ params: [1, 2],
35
+};
36
+
37
+```
38
+
39
+
40
+## Error
41
+
42
+```
43
+ 21 | const result = useMemo(() => {
44
+ 22 | return [Math.max(x[1], a)];
45
+> 23 | }, [a, x]);
46
+ | ^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly (23:23)
47
+ 24 | arrayPush(y, 3);
48
+ 25 | return {result, y};
49
+ 26 | }
50
+```
51
+
52
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-overlap-scopes.ts
new
+31
@@ -0,0 +1,31 @@
1
+// @validatePreserveExistingMemoizationGuarantees:true
2
+import {useMemo} from 'react';
3
+import {arrayPush} from 'shared-runtime';
4
+
5
+/**
6
+ * Repro showing differences between mutable ranges and scope ranges.
7
+ *
8
+ * For useMemo dependency `x`:
9
+ * - mutable range ends after the `arrayPush(x, b)` instruction
10
+ * - scope range is extended due to MergeOverlappingScopes
11
+ *
12
+ * Since manual memo deps are guaranteed to be named (guaranteeing valid
13
+ * codegen), it's correct to take a dependency on a dep *before* the end
14
+ * of its scope (but after its mutable range ends).
15
+ */
16
+
17
+function useFoo(a, b) {
18
+ const x = [];
19
+ const y = [];
20
+ arrayPush(x, b);
21
+ const result = useMemo(() => {
22
+ return [Math.max(x[1], a)];
23
+ }, [a, x]);
24
+ arrayPush(y, 3);
25
+ return {result, y};
26
+}
27
+
28
+export const FIXTURE_ENTRYPOINT = {
29
+ fn: useFoo,
30
+ params: [1, 2],
31
+};