mirror of
https://github.com/facebook/react.git
synced 2026-02-22 03:42:05 +00:00
[compiler] Validate against component/hook factories (#34305)
Previously, the compiler would incorrectly attempt to compile nested components/hooks defined inside non-React functions. This would lead to scope reference errors at runtime because the compiler would optimize the nested React function without understanding its closure over the parent function's variables. This PR adds detection when non-React functions declare components or hooks, and reports a clear error before compilation. I put this under a new compiler flag defaulting to false. I'll run a test on this internally first, but I expect we should be able to just turn it on in both compiler (so we stop miscompiling) and linter. Closes #33978 Playground example: https://react-compiler-playground-git-pr34305-fbopensource.vercel.app/#N4Igzg9grgTgxgUxALhAejQAgAIDcCGANgJYAm+ALggHIQAiAngHb4C2xcRhDAwjApQSkeEVgAcITBEwpgA8jAASECAGswAHSkAPCTAqYAZlCZwKxSZgDmCCgEkmYqBQAU+AJSZgWzJjiSwAwB1GHwxMQQYTABeTBdPaIA+Lx9fPwCDAAt8JlJCBB5sphsYuITk7yY0tPwAOklCnJt4gG5U3wBfNqZ2zH4KWCqAHmJHZ0wGopto4CK8gqmEDsw0RO7O7tT+wcwQsIiYbo6QDqA
This commit is contained in:
@@ -541,6 +541,9 @@ export enum ErrorCategory {
|
||||
// Checking for valid usage of manual memoization
|
||||
UseMemo = 'UseMemo',
|
||||
|
||||
// Checking for higher order functions acting as factories for components/hooks
|
||||
Factories = 'Factories',
|
||||
|
||||
// Checks that manual memoization is preserved
|
||||
PreserveManualMemo = 'PreserveManualMemo',
|
||||
|
||||
@@ -703,6 +706,16 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
|
||||
recommended: true,
|
||||
};
|
||||
}
|
||||
case ErrorCategory.Factories: {
|
||||
return {
|
||||
category,
|
||||
name: 'component-hook-factories',
|
||||
description:
|
||||
'Validates against higher order functions defining nested components or hooks. ' +
|
||||
'Components and hooks should be defined at the module level',
|
||||
recommended: true,
|
||||
};
|
||||
}
|
||||
case ErrorCategory.FBT: {
|
||||
return {
|
||||
category,
|
||||
|
||||
@@ -494,7 +494,20 @@ function findFunctionsToCompile(
|
||||
): Array<CompileSource> {
|
||||
const queue: Array<CompileSource> = [];
|
||||
const traverseFunction = (fn: BabelFn, pass: CompilerPass): void => {
|
||||
// In 'all' mode, compile only top level functions
|
||||
if (
|
||||
pass.opts.compilationMode === 'all' &&
|
||||
fn.scope.getProgramParent() !== fn.scope.parent
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const fnType = getReactFunctionType(fn, pass);
|
||||
|
||||
if (pass.opts.environment.validateNoDynamicallyCreatedComponentsOrHooks) {
|
||||
validateNoDynamicallyCreatedComponentsOrHooks(fn, pass, programContext);
|
||||
}
|
||||
|
||||
if (fnType === null || programContext.alreadyCompiled.has(fn.node)) {
|
||||
return;
|
||||
}
|
||||
@@ -839,6 +852,73 @@ function shouldSkipCompilation(
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that Components/Hooks are always defined at module level. This prevents scope reference
|
||||
* errors that occur when the compiler attempts to optimize the nested component/hook while its
|
||||
* parent function remains uncompiled.
|
||||
*/
|
||||
function validateNoDynamicallyCreatedComponentsOrHooks(
|
||||
fn: BabelFn,
|
||||
pass: CompilerPass,
|
||||
programContext: ProgramContext,
|
||||
): void {
|
||||
const parentNameExpr = getFunctionName(fn);
|
||||
const parentName =
|
||||
parentNameExpr !== null && parentNameExpr.isIdentifier()
|
||||
? parentNameExpr.node.name
|
||||
: '<anonymous>';
|
||||
|
||||
const validateNestedFunction = (
|
||||
nestedFn: NodePath<
|
||||
t.FunctionDeclaration | t.FunctionExpression | t.ArrowFunctionExpression
|
||||
>,
|
||||
): void => {
|
||||
if (
|
||||
nestedFn.node === fn.node ||
|
||||
programContext.alreadyCompiled.has(nestedFn.node)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (nestedFn.scope.getProgramParent() !== nestedFn.scope.parent) {
|
||||
const nestedFnType = getReactFunctionType(nestedFn as BabelFn, pass);
|
||||
const nestedFnNameExpr = getFunctionName(nestedFn as BabelFn);
|
||||
const nestedName =
|
||||
nestedFnNameExpr !== null && nestedFnNameExpr.isIdentifier()
|
||||
? nestedFnNameExpr.node.name
|
||||
: '<anonymous>';
|
||||
if (nestedFnType === 'Component' || nestedFnType === 'Hook') {
|
||||
CompilerError.throwDiagnostic({
|
||||
category: ErrorCategory.Factories,
|
||||
severity: ErrorSeverity.InvalidReact,
|
||||
reason: `Components and hooks cannot be created dynamically`,
|
||||
description: `The function \`${nestedName}\` appears to be a React ${nestedFnType.toLowerCase()}, but it's defined inside \`${parentName}\`. Components and Hooks should always be declared at module scope`,
|
||||
details: [
|
||||
{
|
||||
kind: 'error',
|
||||
message: 'this function dynamically created a component/hook',
|
||||
loc: parentNameExpr?.node.loc ?? fn.node.loc ?? null,
|
||||
},
|
||||
{
|
||||
kind: 'error',
|
||||
message: 'the component is created here',
|
||||
loc: nestedFnNameExpr?.node.loc ?? nestedFn.node.loc ?? null,
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
nestedFn.skip();
|
||||
};
|
||||
|
||||
fn.traverse({
|
||||
FunctionDeclaration: validateNestedFunction,
|
||||
FunctionExpression: validateNestedFunction,
|
||||
ArrowFunctionExpression: validateNestedFunction,
|
||||
});
|
||||
}
|
||||
|
||||
function getReactFunctionType(
|
||||
fn: BabelFn,
|
||||
pass: CompilerPass,
|
||||
@@ -877,11 +957,6 @@ function getReactFunctionType(
|
||||
return componentSyntaxType;
|
||||
}
|
||||
case 'all': {
|
||||
// Compile only top level functions
|
||||
if (fn.scope.getProgramParent() !== fn.scope.parent) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return getComponentOrHookLike(fn, hookPattern) ?? 'Other';
|
||||
}
|
||||
default: {
|
||||
|
||||
@@ -650,6 +650,13 @@ export const EnvironmentConfigSchema = z.object({
|
||||
* useMemo(() => { ... }, [...]);
|
||||
*/
|
||||
validateNoVoidUseMemo: z.boolean().default(false),
|
||||
|
||||
/**
|
||||
* Validates that Components/Hooks are always defined at module level. This prevents scope
|
||||
* reference errors that occur when the compiler attempts to optimize the nested component/hook
|
||||
* while its parent function remains uncompiled.
|
||||
*/
|
||||
validateNoDynamicallyCreatedComponentsOrHooks: z.boolean().default(false),
|
||||
});
|
||||
|
||||
export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoDynamicallyCreatedComponentsOrHooks
|
||||
export function getInput(a) {
|
||||
const Wrapper = () => {
|
||||
const handleChange = () => {
|
||||
a.onChange();
|
||||
};
|
||||
|
||||
return <input onChange={handleChange} />;
|
||||
};
|
||||
|
||||
return Wrapper;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: getInput,
|
||||
isComponent: false,
|
||||
params: [{onChange() {}}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Components and hooks cannot be created dynamically
|
||||
|
||||
The function `Wrapper` appears to be a React component, but it's defined inside `getInput`. Components and Hooks should always be declared at module scope
|
||||
|
||||
error.nested-component-in-normal-function.ts:2:16
|
||||
1 | // @validateNoDynamicallyCreatedComponentsOrHooks
|
||||
> 2 | export function getInput(a) {
|
||||
| ^^^^^^^^ this function dynamically created a component/hook
|
||||
3 | const Wrapper = () => {
|
||||
4 | const handleChange = () => {
|
||||
5 | a.onChange();
|
||||
|
||||
error.nested-component-in-normal-function.ts:3:8
|
||||
1 | // @validateNoDynamicallyCreatedComponentsOrHooks
|
||||
2 | export function getInput(a) {
|
||||
> 3 | const Wrapper = () => {
|
||||
| ^^^^^^^ the component is created here
|
||||
4 | const handleChange = () => {
|
||||
5 | a.onChange();
|
||||
6 | };
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
// @validateNoDynamicallyCreatedComponentsOrHooks
|
||||
export function getInput(a) {
|
||||
const Wrapper = () => {
|
||||
const handleChange = () => {
|
||||
a.onChange();
|
||||
};
|
||||
|
||||
return <input onChange={handleChange} />;
|
||||
};
|
||||
|
||||
return Wrapper;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: getInput,
|
||||
isComponent: false,
|
||||
params: [{onChange() {}}],
|
||||
};
|
||||
@@ -0,0 +1,59 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoDynamicallyCreatedComponentsOrHooks
|
||||
import {useState} from 'react';
|
||||
|
||||
function createCustomHook(config) {
|
||||
function useConfiguredState() {
|
||||
const [state, setState] = useState(0);
|
||||
|
||||
const increment = () => {
|
||||
setState(state + config.step);
|
||||
};
|
||||
|
||||
return [state, increment];
|
||||
}
|
||||
|
||||
return useConfiguredState;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: createCustomHook,
|
||||
isComponent: false,
|
||||
params: [{step: 1}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Components and hooks cannot be created dynamically
|
||||
|
||||
The function `useConfiguredState` appears to be a React hook, but it's defined inside `createCustomHook`. Components and Hooks should always be declared at module scope
|
||||
|
||||
error.nested-hook-in-normal-function.ts:4:9
|
||||
2 | import {useState} from 'react';
|
||||
3 |
|
||||
> 4 | function createCustomHook(config) {
|
||||
| ^^^^^^^^^^^^^^^^ this function dynamically created a component/hook
|
||||
5 | function useConfiguredState() {
|
||||
6 | const [state, setState] = useState(0);
|
||||
7 |
|
||||
|
||||
error.nested-hook-in-normal-function.ts:5:11
|
||||
3 |
|
||||
4 | function createCustomHook(config) {
|
||||
> 5 | function useConfiguredState() {
|
||||
| ^^^^^^^^^^^^^^^^^^ the component is created here
|
||||
6 | const [state, setState] = useState(0);
|
||||
7 |
|
||||
8 | const increment = () => {
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
// @validateNoDynamicallyCreatedComponentsOrHooks
|
||||
import {useState} from 'react';
|
||||
|
||||
function createCustomHook(config) {
|
||||
function useConfiguredState() {
|
||||
const [state, setState] = useState(0);
|
||||
|
||||
const increment = () => {
|
||||
setState(state + config.step);
|
||||
};
|
||||
|
||||
return [state, increment];
|
||||
}
|
||||
|
||||
return useConfiguredState;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: createCustomHook,
|
||||
isComponent: false,
|
||||
params: [{step: 1}],
|
||||
};
|
||||
Reference in New Issue
Block a user