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}], +};