@samitouri / QOS-React / commits / d06196c1cd

[compiler] Visit nested scopes in pruned scopes in PromoteUsedTemporaries

While debugging #30536 I happened to notice that the bug only reproduced when there was interleaving scopes, and observed that an unpruned scope nested inside of a pruned one was not being visited by CollectPromotableTemporaries, which keeps track of which identifiers should be promoted later. Therefore when actually promoting temporaries we were skipping over the identifiers in children of pruned scopes ghstack-source-id: d805f62f22fda04beedb6c7063312451f36d678c Pull Request resolved: https://github.com/facebook/react/pull/30537

Lauren Tan committed Jul 30, 2024 at 16:55 UTC d06196c1cd2dc0bb2288f3a3efa30669d4a7ea61
5 files changed +104 -137
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts
+1
@@ -138,6 +138,7 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
138 usedOutsideScope: false,
139 });
140 }
141 + this.visitBlock(scopeBlock.instructions, state);
142 }
143
144 override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.expect.md deleted
-107
@@ -1,107 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -import {Stringify, identity, useIdentity} from 'shared-runtime';
6 -
7 -/**
8 - * Currently, we're passing a lower-case jsx tag `t0`.
9 - * We should either reorder Stringify or rename the local to `T0`.
10 - *
11 - * See evaluator error:
12 - * Found differences in evaluator results
13 - * Non-forget (expected):
14 - * (kind: ok) <div>{"value":{}}</div><div>{"value":{}}</div>
15 - * Forget:
16 - * (kind: ok) <t1 value="[object Object]"></t1><div>{"value":{}}</div>
17 - * logs: ['Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','t1']
18 - */
19 -function Foo({}) {
20 - const x = {};
21 - const y = {};
22 - useIdentity(0);
23 - return (
24 - <>
25 - <Stringify value={identity(y)} />
26 - <Stringify value={identity(x)} />
27 - </>
28 - );
29 -}
30 -
31 -export const FIXTURE_ENTRYPOINT = {
32 - fn: Foo,
33 - params: [{}],
34 -};
35 -
36 -```
37 -
38 -## Code
39 -
40 -```javascript
41 -import { c as _c } from "react/compiler-runtime";
42 -import { Stringify, identity, useIdentity } from "shared-runtime";
43 -
44 -/**
45 - * Currently, we're passing a lower-case jsx tag `t0`.
46 - * We should either reorder Stringify or rename the local to `T0`.
47 - *
48 - * See evaluator error:
49 - * Found differences in evaluator results
50 - * Non-forget (expected):
51 - * (kind: ok) <div>{"value":{}}</div><div>{"value":{}}</div>
52 - * Forget:
53 - * (kind: ok) <t1 value="[object Object]"></t1><div>{"value":{}}</div>
54 - * logs: ['Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','t1']
55 - */
56 -function Foo(t0) {
57 - const $ = _c(9);
58 - const x = {};
59 - const y = {};
60 - useIdentity(0);
61 -
62 - const t1 = Stringify;
63 - const t2 = identity(y);
64 - let t3;
65 - if ($[0] !== t1 || $[1] !== t2) {
66 - t3 = <t1 value={t2} />;
67 - $[0] = t1;
68 - $[1] = t2;
69 - $[2] = t3;
70 - } else {
71 - t3 = $[2];
72 - }
73 - const T0 = Stringify;
74 - const t4 = identity(x);
75 - let t5;
76 - if ($[3] !== T0 || $[4] !== t4) {
77 - t5 = <T0 value={t4} />;
78 - $[3] = T0;
79 - $[4] = t4;
80 - $[5] = t5;
81 - } else {
82 - t5 = $[5];
83 - }
84 - let t6;
85 - if ($[6] !== t3 || $[7] !== t5) {
86 - t6 = (
87 - <>
88 - {t3}
89 - {t5}
90 - </>
91 - );
92 - $[6] = t3;
93 - $[7] = t5;
94 - $[8] = t6;
95 - } else {
96 - t6 = $[8];
97 - }
98 - return t6;
99 -}
100 -
101 -export const FIXTURE_ENTRYPOINT = {
102 - fn: Foo,
103 - params: [{}],
104 -};
105 -
106 -```
107 -
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.tsx deleted
-30
@@ -1,30 +0,0 @@
1 -import {Stringify, identity, useIdentity} from 'shared-runtime';
2 -
3 -/**
4 - * Currently, we're passing a lower-case jsx tag `t0`.
5 - * We should either reorder Stringify or rename the local to `T0`.
6 - *
7 - * See evaluator error:
8 - * Found differences in evaluator results
9 - * Non-forget (expected):
10 - * (kind: ok) <div>{"value":{}}</div><div>{"value":{}}</div>
11 - * Forget:
12 - * (kind: ok) <t1 value="[object Object]"></t1><div>{"value":{}}</div>
13 - * logs: ['Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','t1']
14 - */
15 -function Foo({}) {
16 - const x = {};
17 - const y = {};
18 - useIdentity(0);
19 - return (
20 - <>
21 - <Stringify value={identity(y)} />
22 - <Stringify value={identity(x)} />
23 - </>
24 - );
25 -}
26 -
27 -export const FIXTURE_ENTRYPOINT = {
28 - fn: Foo,
29 - params: [{}],
30 -};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.expect.md new
+85
@@ -0,0 +1,85 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {Stringify, identity, useIdentity} from 'shared-runtime';
6 +
7 +function Foo({}) {
8 + const x = {};
9 + const y = {};
10 + useIdentity(0);
11 + return (
12 + <>
13 + <Stringify value={identity(y)} />
14 + <Stringify value={identity(x)} />
15 + </>
16 + );
17 +}
18 +
19 +export const FIXTURE_ENTRYPOINT = {
20 + fn: Foo,
21 + params: [{}],
22 +};
23 +
24 +```
25 +
26 +## Code
27 +
28 +```javascript
29 +import { c as _c } from "react/compiler-runtime";
30 +import { Stringify, identity, useIdentity } from "shared-runtime";
31 +
32 +function Foo(t0) {
33 + const $ = _c(9);
34 + const x = {};
35 + const y = {};
36 + useIdentity(0);
37 +
38 + const T0 = Stringify;
39 + const t1 = identity(y);
40 + let t2;
41 + if ($[0] !== T0 || $[1] !== t1) {
42 + t2 = <T0 value={t1} />;
43 + $[0] = T0;
44 + $[1] = t1;
45 + $[2] = t2;
46 + } else {
47 + t2 = $[2];
48 + }
49 + const T1 = Stringify;
50 + const t3 = identity(x);
51 + let t4;
52 + if ($[3] !== T1 || $[4] !== t3) {
53 + t4 = <T1 value={t3} />;
54 + $[3] = T1;
55 + $[4] = t3;
56 + $[5] = t4;
57 + } else {
58 + t4 = $[5];
59 + }
60 + let t5;
61 + if ($[6] !== t2 || $[7] !== t4) {
62 + t5 = (
63 + <>
64 + {t2}
65 + {t4}
66 + </>
67 + );
68 + $[6] = t2;
69 + $[7] = t4;
70 + $[8] = t5;
71 + } else {
72 + t5 = $[8];
73 + }
74 + return t5;
75 +}
76 +
77 +export const FIXTURE_ENTRYPOINT = {
78 + fn: Foo,
79 + params: [{}],
80 +};
81 +
82 +```
83 +
84 +### Eval output
85 +(kind: ok) <div>{"value":{}}</div><div>{"value":{}}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.tsx new
+18
@@ -0,0 +1,18 @@
1 +import {Stringify, identity, useIdentity} from 'shared-runtime';
2 +
3 +function Foo({}) {
4 + const x = {};
5 + const y = {};
6 + useIdentity(0);
7 + return (
8 + <>
9 + <Stringify value={identity(y)} />
10 + <Stringify value={identity(x)} />
11 + </>
12 + );
13 +}
14 +
15 +export const FIXTURE_ENTRYPOINT = {
16 + fn: Foo,
17 + params: [{}],
18 +};