Fix evaluation order for JSX element tags
When lowering a JSX element we were correctly lowering to a temporary in all but one case: the common case of an identifier. That is fine in practice but breaks in the presence of the tag identifier being reassigned in the props/children. This PR fixes to always lower the tag to a temporary.
Joe Savona committed
Apr 20, 2023 at 16:48 UTC
e9abc41ea3f0170659292ce1151a015300105999
8 files changed
+95
-74
compiler/forget/src/HIR/BuildHIR.ts
+6
-2
@@ -27,7 +27,6 @@ import {
27
InstructionKind,
28
InstructionValue,
29
JsxAttribute,
30
- makeInstructionId,
30
ObjectPattern,
31
ObjectProperty,
32
Place,
@@ -35,6 +34,7 @@ import {
34
SourceLocation,
35
SpreadPattern,
36
ThrowTerminal,
37
+ makeInstructionId,
38
} from "./HIR";
39
import HIRBuilder, { Bindings } from "./HIRBuilder";
40
@@ -2041,7 +2041,11 @@ function lowerJsxElementName(
2041
if (exprPath.isJSXIdentifier()) {
2042
const tag: string = exprPath.node.name;
2043
if (tag.match(/^[A-Z]/)) {
2044
- return lowerIdentifier(builder, exprPath);
2044
+ return lowerValueToTemporary(builder, {
2045
+ kind: "LoadLocal",
2046
+ place: lowerIdentifier(builder, exprPath),
2047
+ loc: exprLoc,
2048
+ });
2049
} else {
2050
if (tag.indexOf(":") !== -1) {
2051
builder.errors.push({
compiler/forget/src/__tests__/fixtures/compiler/_bug.jsx-tag-evaluation-order.expect.md
deleted
-68
@@ -1,68 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function Component(props) {
6
- const maybeMutable = new MaybeMutable();
7
- let Tag = View;
8
- // NOTE: the order of evaluation in the lowering is incorrect:
9
- // the jsx element's tag observes `Tag` after reassignment, but should observe
10
- // it before the reassignment.
11
- return (
12
- <Tag>
13
- {((Tag = HScroll), maybeMutate(maybeMutable))}
14
- <Tag />
15
- </Tag>
16
- );
17
-}
18
-
19
-```
20
-
21
-## Code
22
-
23
-```javascript
24
-import * as React from "react";
25
-function Component(props) {
26
- const $ = React.unstable_useMemoCache(5);
27
- let Tag;
28
- let t0;
29
- let t1;
30
- if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
31
- const maybeMutable = new MaybeMutable();
32
-
33
- t0 = "\n ";
34
- Tag = HScroll;
35
- t1 = maybeMutate(maybeMutable);
36
- $[0] = Tag;
37
- $[1] = t0;
38
- $[2] = t1;
39
- } else {
40
- Tag = $[0];
41
- t0 = $[1];
42
- t1 = $[2];
43
- }
44
- let t2;
45
- if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
46
- t2 = <Tag />;
47
- $[3] = t2;
48
- } else {
49
- t2 = $[3];
50
- }
51
- let t3;
52
- if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
53
- t3 = (
54
- <Tag>
55
- {t0}
56
- {t1}
57
- {t2}
58
- </Tag>
59
- );
60
- $[4] = t3;
61
- } else {
62
- t3 = $[4];
63
- }
64
- return t3;
65
-}
66
-
67
-```
68
-
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.jsx-tag-evaluation-order.expect.md
new
+28
@@ -0,0 +1,28 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const maybeMutable = new MaybeMutable();
7
+ let Tag = props.component;
8
+ // NOTE: the order of evaluation in the lowering is incorrect:
9
+ // the jsx element's tag observes `Tag` after reassignment, but should observe
10
+ // it before the reassignment.
11
+ return (
12
+ <Tag>
13
+ {((Tag = props.alternateComponent), maybeMutate(maybeMutable))}
14
+ <Tag />
15
+ </Tag>
16
+ );
17
+}
18
+
19
+```
20
+
21
+
22
+## Error
23
+
24
+```
25
+[ReactForget] Invariant: [Codegen] No value found for temporary. Value for 'read $33' was not set in the codegen context (8:8)
26
+```
27
+
28
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.jsx-tag-evaluation-order.js
renamed
+2
-2
@@ -1,12 +1,12 @@
1
function Component(props) {
2
const maybeMutable = new MaybeMutable();
3
- let Tag = View;
3
+ let Tag = props.component;
4
// NOTE: the order of evaluation in the lowering is incorrect:
5
// the jsx element's tag observes `Tag` after reassignment, but should observe
6
// it before the reassignment.
7
return (
8
<Tag>
9
- {((Tag = HScroll), maybeMutate(maybeMutable))}
9
+ {((Tag = props.alternateComponent), maybeMutate(maybeMutable))}
10
<Tag />
11
</Tag>
12
);
compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.expect.md
+1
-1
@@ -25,7 +25,7 @@ function Component(props) {
25
## Error
26
27
```
28
-[ReactForget] InvalidInput: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $43:TObject<BuiltInArray> (frozen) (13:13)
28
+[ReactForget] InvalidInput: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $46:TObject<BuiltInArray> (frozen) (13:13)
29
```
30
31
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.expect.md
+1
-1
@@ -19,7 +19,7 @@ function Component(props) {
19
## Error
20
21
```
22
-[ReactForget] InvalidInput: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $26:TObject<BuiltInArray> (frozen) (7:7)
22
+[ReactForget] InvalidInput: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $28:TObject<BuiltInArray> (frozen) (7:7)
23
```
24
25
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/jsx-tag-evaluation-order.expect.md
new
+48
@@ -0,0 +1,48 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let Tag = View;
7
+ return (
8
+ <Tag>
9
+ {((Tag = HScroll), props.value)}
10
+ <Tag />
11
+ </Tag>
12
+ );
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+import * as React from "react";
21
+function Component(props) {
22
+ const $ = React.unstable_useMemoCache(3);
23
+ let t0;
24
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
25
+ t0 = <HScroll />;
26
+ $[0] = t0;
27
+ } else {
28
+ t0 = $[0];
29
+ }
30
+ const c_1 = $[1] !== props.value;
31
+ let t1;
32
+ if (c_1) {
33
+ t1 = (
34
+ <View>
35
+ {props.value}
36
+ {t0}
37
+ </View>
38
+ );
39
+ $[1] = props.value;
40
+ $[2] = t1;
41
+ } else {
42
+ t1 = $[2];
43
+ }
44
+ return t1;
45
+}
46
+
47
+```
48
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/jsx-tag-evaluation-order.js
new
+9
@@ -0,0 +1,9 @@
1
+function Component(props) {
2
+ let Tag = View;
3
+ return (
4
+ <Tag>
5
+ {((Tag = HScroll), props.value)}
6
+ <Tag />
7
+ </Tag>
8
+ );
9
+}