mirror of
https://github.com/zebrajr/react.git
synced 2025-12-06 12:20:20 +01:00
[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: e7408c0a55478b40d65489703d209e8fa7205e45
Pull Request resolved: https://github.com/facebook/react/pull/32683
This commit is contained in:
parent
112224d8d2
commit
5f4c5c920f
|
|
@ -102,6 +102,7 @@ import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls
|
|||
import {transformFire} from '../Transform';
|
||||
import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender';
|
||||
import {CompilerError} from '..';
|
||||
import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
|
||||
|
||||
export type CompilerPipelineValue =
|
||||
| {kind: 'ast'; name: string; value: CodegenFunction}
|
||||
|
|
@ -293,6 +294,10 @@ function runWithEnvironment(
|
|||
});
|
||||
|
||||
if (env.isInferredMemoEnabled) {
|
||||
if (env.config.validateStaticComponents) {
|
||||
env.logErrors(validateStaticComponents(hir));
|
||||
}
|
||||
|
||||
/**
|
||||
* Only create reactive scopes (which directly map to generated memo blocks)
|
||||
* if inferred memoization is enabled. This makes all later passes which
|
||||
|
|
|
|||
|
|
@ -330,6 +330,11 @@ const EnvironmentConfigSchema = z.object({
|
|||
*/
|
||||
validateNoJSXInTryStatements: z.boolean().default(false),
|
||||
|
||||
/**
|
||||
* Validates against dynamically creating components during render.
|
||||
*/
|
||||
validateStaticComponents: z.boolean().default(false),
|
||||
|
||||
/**
|
||||
* Validates that the dependencies of all effect hooks are memoized. This helps ensure
|
||||
* that Forget does not introduce infinite renders caused by a dependency changing,
|
||||
|
|
@ -932,6 +937,19 @@ export class Environment {
|
|||
return makeScopeId(this.#nextScope++);
|
||||
}
|
||||
|
||||
logErrors(errors: Result<void, CompilerError>): void {
|
||||
if (errors.isOk() || this.logger == null) {
|
||||
return;
|
||||
}
|
||||
for (const error of errors.unwrapErr().details) {
|
||||
this.logger.logEvent(this.filename, {
|
||||
kind: 'CompileError',
|
||||
detail: error,
|
||||
fnLoc: null,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
isContextIdentifier(node: t.Identifier): boolean {
|
||||
return this.#contextIdentifiers.has(node);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,87 @@
|
|||
/**
|
||||
* Copyright (c) Meta Platforms, Inc. and affiliates.
|
||||
*
|
||||
* This source code is licensed under the MIT license found in the
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {CompilerError, ErrorSeverity} from '../CompilerError';
|
||||
import {HIRFunction, IdentifierId, SourceLocation} from '../HIR';
|
||||
import {Err, Ok, Result} from '../Utils/Result';
|
||||
|
||||
/**
|
||||
* Validates against components that are created dynamically and whose identity is not guaranteed
|
||||
* to be stable (which would cause the component to reset on each re-render).
|
||||
*/
|
||||
export function validateStaticComponents(
|
||||
fn: HIRFunction,
|
||||
): Result<void, CompilerError> {
|
||||
const error = new CompilerError();
|
||||
const knownDynamicComponents = new Map<IdentifierId, SourceLocation>();
|
||||
for (const block of fn.body.blocks.values()) {
|
||||
phis: for (const phi of block.phis) {
|
||||
for (const operand of phi.operands.values()) {
|
||||
const loc = knownDynamicComponents.get(operand.identifier.id);
|
||||
if (loc != null) {
|
||||
knownDynamicComponents.set(phi.place.identifier.id, loc);
|
||||
continue phis;
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const instr of block.instructions) {
|
||||
const {lvalue, value} = instr;
|
||||
switch (value.kind) {
|
||||
case 'FunctionExpression':
|
||||
case 'NewExpression':
|
||||
case 'MethodCall':
|
||||
case 'CallExpression': {
|
||||
knownDynamicComponents.set(lvalue.identifier.id, value.loc);
|
||||
break;
|
||||
}
|
||||
case 'LoadLocal': {
|
||||
const loc = knownDynamicComponents.get(value.place.identifier.id);
|
||||
if (loc != null) {
|
||||
knownDynamicComponents.set(lvalue.identifier.id, loc);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case 'StoreLocal': {
|
||||
const loc = knownDynamicComponents.get(value.value.identifier.id);
|
||||
if (loc != null) {
|
||||
knownDynamicComponents.set(lvalue.identifier.id, loc);
|
||||
knownDynamicComponents.set(value.lvalue.place.identifier.id, loc);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case 'JsxExpression': {
|
||||
if (value.tag.kind === 'Identifier') {
|
||||
const location = knownDynamicComponents.get(
|
||||
value.tag.identifier.id,
|
||||
);
|
||||
if (location != null) {
|
||||
error.push({
|
||||
reason: `Components created during render will reset their state each time they are created. Declare components outside of render. `,
|
||||
severity: ErrorSeverity.InvalidReact,
|
||||
loc: value.tag.loc,
|
||||
description: null,
|
||||
suggestions: null,
|
||||
});
|
||||
error.push({
|
||||
reason: `The component may be created during render`,
|
||||
severity: ErrorSeverity.InvalidReact,
|
||||
loc: location,
|
||||
description: null,
|
||||
suggestions: null,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (error.hasErrors()) {
|
||||
return Err(error);
|
||||
} else {
|
||||
return Ok(undefined);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,59 @@
|
|||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
let Component;
|
||||
if (props.cond) {
|
||||
Component = createComponent();
|
||||
} else {
|
||||
Component = DefaultComponent;
|
||||
}
|
||||
return <Component />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const $ = _c(3);
|
||||
let Component;
|
||||
if (props.cond) {
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t0 = createComponent();
|
||||
$[0] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
}
|
||||
Component = t0;
|
||||
} else {
|
||||
Component = DefaultComponent;
|
||||
}
|
||||
let t0;
|
||||
if ($[1] !== Component) {
|
||||
t0 = <Component />;
|
||||
$[1] = Component;
|
||||
$[2] = t0;
|
||||
} else {
|
||||
t0 = $[2];
|
||||
}
|
||||
return t0;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"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}
|
||||
{"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}
|
||||
{"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}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
let Component;
|
||||
if (props.cond) {
|
||||
Component = createComponent();
|
||||
} else {
|
||||
Component = DefaultComponent;
|
||||
}
|
||||
return <Component />;
|
||||
}
|
||||
|
|
@ -0,0 +1,41 @@
|
|||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const Component = createComponent();
|
||||
return <Component />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const $ = _c(1);
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
const Component = createComponent();
|
||||
t0 = <Component />;
|
||||
$[0] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
}
|
||||
return t0;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"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}
|
||||
{"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}
|
||||
{"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}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const Component = createComponent();
|
||||
return <Component />;
|
||||
}
|
||||
|
|
@ -0,0 +1,46 @@
|
|||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
function Component() {
|
||||
return <div />;
|
||||
}
|
||||
return <Component />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const $ = _c(1);
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
const Component = function Component() {
|
||||
return <div />;
|
||||
};
|
||||
|
||||
t0 = <Component />;
|
||||
$[0] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
}
|
||||
return t0;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"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}
|
||||
{"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}
|
||||
{"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}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
function Component() {
|
||||
return <div />;
|
||||
}
|
||||
return <Component />;
|
||||
}
|
||||
|
|
@ -0,0 +1,50 @@
|
|||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const Component = props.foo.bar();
|
||||
return <Component />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const $ = _c(4);
|
||||
let t0;
|
||||
if ($[0] !== props.foo) {
|
||||
t0 = props.foo.bar();
|
||||
$[0] = props.foo;
|
||||
$[1] = t0;
|
||||
} else {
|
||||
t0 = $[1];
|
||||
}
|
||||
const Component = t0;
|
||||
let t1;
|
||||
if ($[2] !== Component) {
|
||||
t1 = <Component />;
|
||||
$[2] = Component;
|
||||
$[3] = t1;
|
||||
} else {
|
||||
t1 = $[3];
|
||||
}
|
||||
return t1;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"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}
|
||||
{"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}
|
||||
{"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}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const Component = props.foo.bar();
|
||||
return <Component />;
|
||||
}
|
||||
|
|
@ -0,0 +1,41 @@
|
|||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const Component = new ComponentFactory();
|
||||
return <Component />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const $ = _c(1);
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
const Component = new ComponentFactory();
|
||||
t0 = <Component />;
|
||||
$[0] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
}
|
||||
return t0;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"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":117},"end":{"line":4,"column":19,"index":126},"filename":"invalid-dynamically-constructed-component-new.ts"}}},"fnLoc":null}
|
||||
{"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":42,"index":105},"filename":"invalid-dynamically-constructed-component-new.ts"}}},"fnLoc":null}
|
||||
{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":37},"end":{"line":5,"column":1,"index":132},"filename":"invalid-dynamically-constructed-component-new.ts"},"fnName":"Example","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
// @logger @validateStaticComponents
|
||||
function Example(props) {
|
||||
const Component = new ComponentFactory();
|
||||
return <Component />;
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user