[compiler] patch: retain UpdateExpression for globals
ghstack-source-id: aff6606f85698ccda13d1deffb6faa9d91f9821a Pull Request resolved: https://github.com/facebook/react/pull/30471
Mofei Zhang committed
Jul 26, 2024 at 17:36 UTC
a8f5c4e16d46666453a102665370b3119dbbb029
7 files changed
+67
-56
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+32
-21
@@ -1934,7 +1934,6 @@ function lowerExpression(
1934
switch (leftNode.type) {
1935
case 'Identifier': {
1936
const leftExpr = left as NodePath<t.Identifier>;
1937
- const identifier = lowerIdentifier(builder, leftExpr);
1937
const leftPlace = lowerExpressionToTemporary(builder, leftExpr);
1938
const right = lowerExpressionToTemporary(builder, expr.get('right'));
1939
const binaryPlace = lowerValueToTemporary(builder, {
@@ -1944,30 +1943,42 @@ function lowerExpression(
1943
right,
1944
loc: exprLoc,
1945
});
1947
- const kind = getStoreKind(builder, leftExpr);
1948
- if (kind === 'StoreLocal') {
1949
- lowerValueToTemporary(builder, {
1950
- kind: 'StoreLocal',
1951
- lvalue: {
1952
- place: {...identifier},
1953
- kind: InstructionKind.Reassign,
1954
- },
1955
- value: {...binaryPlace},
1956
- type: null,
1957
- loc: exprLoc,
1958
- });
1959
- return {kind: 'LoadLocal', place: identifier, loc: exprLoc};
1946
+ const binding = builder.resolveIdentifier(leftExpr);
1947
+ if (binding.kind === 'Identifier') {
1948
+ const identifier = lowerIdentifier(builder, leftExpr);
1949
+ const kind = getStoreKind(builder, leftExpr);
1950
+ if (kind === 'StoreLocal') {
1951
+ lowerValueToTemporary(builder, {
1952
+ kind: 'StoreLocal',
1953
+ lvalue: {
1954
+ place: {...identifier},
1955
+ kind: InstructionKind.Reassign,
1956
+ },
1957
+ value: {...binaryPlace},
1958
+ type: null,
1959
+ loc: exprLoc,
1960
+ });
1961
+ return {kind: 'LoadLocal', place: identifier, loc: exprLoc};
1962
+ } else {
1963
+ lowerValueToTemporary(builder, {
1964
+ kind: 'StoreContext',
1965
+ lvalue: {
1966
+ place: {...identifier},
1967
+ kind: InstructionKind.Reassign,
1968
+ },
1969
+ value: {...binaryPlace},
1970
+ loc: exprLoc,
1971
+ });
1972
+ return {kind: 'LoadContext', place: identifier, loc: exprLoc};
1973
+ }
1974
} else {
1961
- lowerValueToTemporary(builder, {
1962
- kind: 'StoreContext',
1963
- lvalue: {
1964
- place: {...identifier},
1965
- kind: InstructionKind.Reassign,
1966
- },
1975
+ const temporary = lowerValueToTemporary(builder, {
1976
+ kind: 'StoreGlobal',
1977
+ name: leftExpr.node.name,
1978
value: {...binaryPlace},
1979
loc: exprLoc,
1980
});
1970
- return {kind: 'LoadContext', place: identifier, loc: exprLoc};
1981
+ return {kind: 'LoadLocal', place: temporary, loc: temporary.loc};
1982
}
1983
}
1984
case 'MemberExpression': {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-update-global-should-bailout.expect.md
deleted
-32
@@ -1,32 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-let renderCount = 0;
6
-function useFoo() {
7
- renderCount += 1;
8
- return renderCount;
9
-}
10
-
11
-export const FIXTURE_ENTRYPOINT = {
12
- fn: useFoo,
13
- params: [],
14
-};
15
-
16
-```
17
-
18
-## Code
19
-
20
-```javascript
21
-let renderCount = 0;
22
-function useFoo() {
23
- return renderCount;
24
-}
25
-
26
-export const FIXTURE_ENTRYPOINT = {
27
- fn: useFoo,
28
- params: [],
29
-};
30
-
31
-```
32
-
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.expect.md
new
+31
@@ -0,0 +1,31 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+let renderCount = 0;
6
+function useFoo() {
7
+ renderCount += 1;
8
+ return renderCount;
9
+}
10
+
11
+export const FIXTURE_ENTRYPOINT = {
12
+ fn: useFoo,
13
+ params: [],
14
+};
15
+
16
+```
17
+
18
+
19
+## Error
20
+
21
+```
22
+ 1 | let renderCount = 0;
23
+ 2 | function useFoo() {
24
+> 3 | renderCount += 1;
25
+ | ^^^^^^^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
26
+ 4 | return renderCount;
27
+ 5 | }
28
+ 6 |
29
+```
30
+
31
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.tsx
renamed
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.expect.md
renamed
+4
-1
@@ -40,6 +40,7 @@ function Foo() {
40
return t0;
41
}
42
function _temp() {
43
+ renderCount = renderCount + 1;
44
return renderCount;
45
}
46
@@ -49,4 +50,6 @@ export const FIXTURE_ENTRYPOINT = {
50
};
51
52
```
52
-
\ No newline at end of file
53
+
54
+### Eval output
55
+(kind: ok) <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.tsx
renamed
compiler/packages/snap/src/SproutTodoFilter.ts
-2
@@ -484,8 +484,6 @@ const skipFilter = new Set([
484
'rules-of-hooks/rules-of-hooks-69521d94fa03',
485
486
// bugs
487
- 'bug-invalid-update-global-should-bailout',
488
- 'bug-update-global-in-callback',
487
'bug-invalid-hoisting-functionexpr',
488
'original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block',
489
'original-reactive-scopes-fork/bug-hoisted-declaration-with-scope',