From 526d5b9d3bf864b9e062c080ce90c978b90b0fb5 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 27 Aug 2025 13:49:50 -0400 Subject: [PATCH] [compiler] Validate against component/hook factories 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 --- .../src/CompilerError.ts | 13 +++ .../src/Entrypoint/Program.ts | 85 +++++++++++++++++-- .../src/HIR/Environment.ts | 7 ++ ...ted-component-in-normal-function.expect.md | 54 ++++++++++++ ...ror.nested-component-in-normal-function.js | 18 ++++ ...r.nested-hook-in-normal-function.expect.md | 59 +++++++++++++ .../error.nested-hook-in-normal-function.js | 22 +++++ 7 files changed, 253 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 964217c39931..f3007034c3b7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -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, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index 92151501622b..5a9ef9495fa1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -494,7 +494,20 @@ function findFunctionsToCompile( ): Array { const queue: Array = []; 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 + : ''; + + 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 + : ''; + 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: { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index fd68830f929d..d85c6b19c673 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -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; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.expect.md new file mode 100644 index 000000000000..14d86ffe825a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.expect.md @@ -0,0 +1,54 @@ + +## Input + +```javascript +// @validateNoDynamicallyCreatedComponentsOrHooks +export function getInput(a) { + const Wrapper = () => { + const handleChange = () => { + a.onChange(); + }; + + return ; + }; + + 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 | }; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.js new file mode 100644 index 000000000000..36be05e3ee8b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-component-in-normal-function.js @@ -0,0 +1,18 @@ +// @validateNoDynamicallyCreatedComponentsOrHooks +export function getInput(a) { + const Wrapper = () => { + const handleChange = () => { + a.onChange(); + }; + + return ; + }; + + return Wrapper; +} + +export const FIXTURE_ENTRYPOINT = { + fn: getInput, + isComponent: false, + params: [{onChange() {}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.expect.md new file mode 100644 index 000000000000..0ab2b977561e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.expect.md @@ -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 = () => { +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.js new file mode 100644 index 000000000000..306e78f7b1c6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.nested-hook-in-normal-function.js @@ -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}], +};