Skip to content

Commit 5f4c5c9

Browse files
committed
[compiler] Validate static components
React uses function identity to determine whether a given JSX expression represents the same type of component and should reconcile (keep state, update props) or replace (teardown state, create a new instance). This PR adds off-by-default validation to check that developers are not dynamically creating components during render. The check is local and intentionally conservative. We specifically look for the results of call expressions, new expressions, or function expressions that are then used directly (or aliased) as a JSX tag. This allows common sketchy but fine-in-practice cases like passing a reference to a component from a parent as props, but catches very obvious mistakes such as: ```js function Example() { const Component = createComponent(); return <Component />; } ``` We could expand this to catch more cases, but this seems like a reasonable starting point. Note that I tried enabling the validation by default and the only fixtures that error are the new ones added here. I'll also test this internally. What i'm imagining is that we enable this in the linter but not the compiler. ghstack-source-id: e7408c0 Pull Request resolved: facebook#32683
1 parent 112224d commit 5f4c5c9

13 files changed

+379
-0
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls
102102
import {transformFire} from '../Transform';
103103
import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender';
104104
import {CompilerError} from '..';
105+
import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
105106

106107
export type CompilerPipelineValue =
107108
| {kind: 'ast'; name: string; value: CodegenFunction}
@@ -293,6 +294,10 @@ function runWithEnvironment(
293294
});
294295

295296
if (env.isInferredMemoEnabled) {
297+
if (env.config.validateStaticComponents) {
298+
env.logErrors(validateStaticComponents(hir));
299+
}
300+
296301
/**
297302
* Only create reactive scopes (which directly map to generated memo blocks)
298303
* if inferred memoization is enabled. This makes all later passes which

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,11 @@ const EnvironmentConfigSchema = z.object({
330330
*/
331331
validateNoJSXInTryStatements: z.boolean().default(false),
332332

333+
/**
334+
* Validates against dynamically creating components during render.
335+
*/
336+
validateStaticComponents: z.boolean().default(false),
337+
333338
/**
334339
* Validates that the dependencies of all effect hooks are memoized. This helps ensure
335340
* that Forget does not introduce infinite renders caused by a dependency changing,
@@ -932,6 +937,19 @@ export class Environment {
932937
return makeScopeId(this.#nextScope++);
933938
}
934939

940+
logErrors(errors: Result<void, CompilerError>): void {
941+
if (errors.isOk() || this.logger == null) {
942+
return;
943+
}
944+
for (const error of errors.unwrapErr().details) {
945+
this.logger.logEvent(this.filename, {
946+
kind: 'CompileError',
947+
detail: error,
948+
fnLoc: null,
949+
});
950+
}
951+
}
952+
935953
isContextIdentifier(node: t.Identifier): boolean {
936954
return this.#contextIdentifiers.has(node);
937955
}
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {CompilerError, ErrorSeverity} from '../CompilerError';
9+
import {HIRFunction, IdentifierId, SourceLocation} from '../HIR';
10+
import {Err, Ok, Result} from '../Utils/Result';
11+
12+
/**
13+
* Validates against components that are created dynamically and whose identity is not guaranteed
14+
* to be stable (which would cause the component to reset on each re-render).
15+
*/
16+
export function validateStaticComponents(
17+
fn: HIRFunction,
18+
): Result<void, CompilerError> {
19+
const error = new CompilerError();
20+
const knownDynamicComponents = new Map<IdentifierId, SourceLocation>();
21+
for (const block of fn.body.blocks.values()) {
22+
phis: for (const phi of block.phis) {
23+
for (const operand of phi.operands.values()) {
24+
const loc = knownDynamicComponents.get(operand.identifier.id);
25+
if (loc != null) {
26+
knownDynamicComponents.set(phi.place.identifier.id, loc);
27+
continue phis;
28+
}
29+
}
30+
}
31+
for (const instr of block.instructions) {
32+
const {lvalue, value} = instr;
33+
switch (value.kind) {
34+
case 'FunctionExpression':
35+
case 'NewExpression':
36+
case 'MethodCall':
37+
case 'CallExpression': {
38+
knownDynamicComponents.set(lvalue.identifier.id, value.loc);
39+
break;
40+
}
41+
case 'LoadLocal': {
42+
const loc = knownDynamicComponents.get(value.place.identifier.id);
43+
if (loc != null) {
44+
knownDynamicComponents.set(lvalue.identifier.id, loc);
45+
}
46+
break;
47+
}
48+
case 'StoreLocal': {
49+
const loc = knownDynamicComponents.get(value.value.identifier.id);
50+
if (loc != null) {
51+
knownDynamicComponents.set(lvalue.identifier.id, loc);
52+
knownDynamicComponents.set(value.lvalue.place.identifier.id, loc);
53+
}
54+
break;
55+
}
56+
case 'JsxExpression': {
57+
if (value.tag.kind === 'Identifier') {
58+
const location = knownDynamicComponents.get(
59+
value.tag.identifier.id,
60+
);
61+
if (location != null) {
62+
error.push({
63+
reason: `Components created during render will reset their state each time they are created. Declare components outside of render. `,
64+
severity: ErrorSeverity.InvalidReact,
65+
loc: value.tag.loc,
66+
description: null,
67+
suggestions: null,
68+
});
69+
error.push({
70+
reason: `The component may be created during render`,
71+
severity: ErrorSeverity.InvalidReact,
72+
loc: location,
73+
description: null,
74+
suggestions: null,
75+
});
76+
}
77+
}
78+
}
79+
}
80+
}
81+
}
82+
if (error.hasErrors()) {
83+
return Err(error);
84+
} else {
85+
return Ok(undefined);
86+
}
87+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @logger @validateStaticComponents
6+
function Example(props) {
7+
let Component;
8+
if (props.cond) {
9+
Component = createComponent();
10+
} else {
11+
Component = DefaultComponent;
12+
}
13+
return <Component />;
14+
}
15+
16+
```
17+
18+
## Code
19+
20+
```javascript
21+
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
22+
function Example(props) {
23+
const $ = _c(3);
24+
let Component;
25+
if (props.cond) {
26+
let t0;
27+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
28+
t0 = createComponent();
29+
$[0] = t0;
30+
} else {
31+
t0 = $[0];
32+
}
33+
Component = t0;
34+
} else {
35+
Component = DefaultComponent;
36+
}
37+
let t0;
38+
if ($[1] !== Component) {
39+
t0 = <Component />;
40+
$[1] = Component;
41+
$[2] = t0;
42+
} else {
43+
t0 = $[2];
44+
}
45+
return t0;
46+
}
47+
48+
```
49+
50+
## Logs
51+
52+
```
53+
{"kind":"CompileError","detail":{"options":{"reason":"Components created during render will reset their state each time they are created. Declare components outside of render. ","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":9,"column":10,"index":194},"end":{"line":9,"column":19,"index":203},"filename":"invalid-conditionally-assigned-dynamically-constructed-component-in-render.ts"}}},"fnLoc":null}
54+
{"kind":"CompileError","detail":{"options":{"reason":"The component may be created during render","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":5,"column":16,"index":116},"end":{"line":5,"column":33,"index":133},"filename":"invalid-conditionally-assigned-dynamically-constructed-component-in-render.ts"}}},"fnLoc":null}
55+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":37},"end":{"line":10,"column":1,"index":209},"filename":"invalid-conditionally-assigned-dynamically-constructed-component-in-render.ts"},"fnName":"Example","memoSlots":3,"memoBlocks":2,"memoValues":2,"prunedMemoBlocks":0,"prunedMemoValues":0}
56+
```
57+
58+
### Eval output
59+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// @logger @validateStaticComponents
2+
function Example(props) {
3+
let Component;
4+
if (props.cond) {
5+
Component = createComponent();
6+
} else {
7+
Component = DefaultComponent;
8+
}
9+
return <Component />;
10+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @logger @validateStaticComponents
6+
function Example(props) {
7+
const Component = createComponent();
8+
return <Component />;
9+
}
10+
11+
```
12+
13+
## Code
14+
15+
```javascript
16+
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
17+
function Example(props) {
18+
const $ = _c(1);
19+
let t0;
20+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
21+
const Component = createComponent();
22+
t0 = <Component />;
23+
$[0] = t0;
24+
} else {
25+
t0 = $[0];
26+
}
27+
return t0;
28+
}
29+
30+
```
31+
32+
## Logs
33+
34+
```
35+
{"kind":"CompileError","detail":{"options":{"reason":"Components created during render will reset their state each time they are created. Declare components outside of render. ","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":4,"column":10,"index":112},"end":{"line":4,"column":19,"index":121},"filename":"invalid-dynamically-construct-component-in-render.ts"}}},"fnLoc":null}
36+
{"kind":"CompileError","detail":{"options":{"reason":"The component may be created during render","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":3,"column":20,"index":83},"end":{"line":3,"column":37,"index":100},"filename":"invalid-dynamically-construct-component-in-render.ts"}}},"fnLoc":null}
37+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":37},"end":{"line":5,"column":1,"index":127},"filename":"invalid-dynamically-construct-component-in-render.ts"},"fnName":"Example","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
38+
```
39+
40+
### Eval output
41+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// @logger @validateStaticComponents
2+
function Example(props) {
3+
const Component = createComponent();
4+
return <Component />;
5+
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @logger @validateStaticComponents
6+
function Example(props) {
7+
function Component() {
8+
return <div />;
9+
}
10+
return <Component />;
11+
}
12+
13+
```
14+
15+
## Code
16+
17+
```javascript
18+
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
19+
function Example(props) {
20+
const $ = _c(1);
21+
let t0;
22+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
23+
const Component = function Component() {
24+
return <div />;
25+
};
26+
27+
t0 = <Component />;
28+
$[0] = t0;
29+
} else {
30+
t0 = $[0];
31+
}
32+
return t0;
33+
}
34+
35+
```
36+
37+
## Logs
38+
39+
```
40+
{"kind":"CompileError","detail":{"options":{"reason":"Components created during render will reset their state each time they are created. Declare components outside of render. ","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":6,"column":10,"index":122},"end":{"line":6,"column":19,"index":131},"filename":"invalid-dynamically-constructed-component-function.ts"}}},"fnLoc":null}
41+
{"kind":"CompileError","detail":{"options":{"reason":"The component may be created during render","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":3,"column":2,"index":65},"end":{"line":5,"column":3,"index":111},"filename":"invalid-dynamically-constructed-component-function.ts"}}},"fnLoc":null}
42+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":37},"end":{"line":7,"column":1,"index":137},"filename":"invalid-dynamically-constructed-component-function.ts"},"fnName":"Example","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
43+
```
44+
45+
### Eval output
46+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// @logger @validateStaticComponents
2+
function Example(props) {
3+
function Component() {
4+
return <div />;
5+
}
6+
return <Component />;
7+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @logger @validateStaticComponents
6+
function Example(props) {
7+
const Component = props.foo.bar();
8+
return <Component />;
9+
}
10+
11+
```
12+
13+
## Code
14+
15+
```javascript
16+
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
17+
function Example(props) {
18+
const $ = _c(4);
19+
let t0;
20+
if ($[0] !== props.foo) {
21+
t0 = props.foo.bar();
22+
$[0] = props.foo;
23+
$[1] = t0;
24+
} else {
25+
t0 = $[1];
26+
}
27+
const Component = t0;
28+
let t1;
29+
if ($[2] !== Component) {
30+
t1 = <Component />;
31+
$[2] = Component;
32+
$[3] = t1;
33+
} else {
34+
t1 = $[3];
35+
}
36+
return t1;
37+
}
38+
39+
```
40+
41+
## Logs
42+
43+
```
44+
{"kind":"CompileError","detail":{"options":{"reason":"Components created during render will reset their state each time they are created. Declare components outside of render. ","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":4,"column":10,"index":110},"end":{"line":4,"column":19,"index":119},"filename":"invalid-dynamically-constructed-component-method-call.ts"}}},"fnLoc":null}
45+
{"kind":"CompileError","detail":{"options":{"reason":"The component may be created during render","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":3,"column":20,"index":83},"end":{"line":3,"column":35,"index":98},"filename":"invalid-dynamically-constructed-component-method-call.ts"}}},"fnLoc":null}
46+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":37},"end":{"line":5,"column":1,"index":125},"filename":"invalid-dynamically-constructed-component-method-call.ts"},"fnName":"Example","memoSlots":4,"memoBlocks":2,"memoValues":2,"prunedMemoBlocks":0,"prunedMemoValues":0}
47+
```
48+
49+
### Eval output
50+
(kind: exception) Fixture not implemented

0 commit comments

Comments
 (0)