From 213594860f456850bd615fcf08c0f0da25a998c5 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Wed, 27 Aug 2025 16:00:06 +0200 Subject: [PATCH 1/4] [DevTools] Better scrolling in Suspense tab (#34299) --- .../views/SuspenseTab/SuspenseTab.css | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css index dc82b25da2178..921ce0cff2a7b 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css @@ -15,15 +15,15 @@ } .TreeWrapper { + border-top: 1px solid var(--color-border); flex: 1 1 var(--horizontal-resize-tree-percentage); display: flex; flex-direction: row; - overflow: auto; - border-top: 1px solid var(--color-border); + height: 100%; } .InspectedElementWrapper { - flex: 1 1 calc(100% - var(--horizontal-resize-tree-percentage)); + flex: 1 1 35%; overflow-x: hidden; overflow-y: auto; } @@ -52,8 +52,6 @@ display: none; } - - @container devtools (width < 600px) { .SuspenseTab { flex-direction: column; @@ -61,7 +59,8 @@ .TreeWrapper { border-top: 1px solid var(--color-border); - flex: 1 0 var(--vertical-resize-tree-percentage); + flex: 1 1 var(--vertical-resize-tree-percentage); + overflow: hidden; } .InspectedElementWrapper { @@ -79,6 +78,7 @@ .TreeView footer { display: flex; justify-content: end; + border-top: 1px solid var(--color-border); } .ToggleInspectedElement[data-orientation="horizontal"] { @@ -89,21 +89,23 @@ .TreeList { flex: 0 0 var(--horizontal-resize-tree-list-percentage); border-right: 1px solid var(--color-border); - padding: 0.25rem + padding: 0.25rem; + overflow: auto; } .TreeView { flex: 1 1 35%; display: flex; flex-direction: column; + height: 100%; + overflow: auto; } - - .Rects { border-top: 1px solid var(--color-border); padding: 0.25rem; flex-grow: 1; + overflow: auto; } .TimelineWrapper { From 33a1095d724c5ad0a6238e03e04461b6df5e73e2 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Wed, 27 Aug 2025 08:44:09 -0700 Subject: [PATCH 2/4] [compiler] Infer render helpers for additional validation (#33647) We currently assume that any functions passes as props may be event handlers or effect functions, and thus don't check for side effects such as mutating globals. However, if a prop is a function that returns JSX that is a sure sign that it's actually a render helper and not an event handler or effect function. So we now emit a `Render` effect for any prop that is a JSX-returning function, triggering all of our render validation. This required a small fix to InferTypes: we weren't correctly populating the `return` type of function types during unification. I also improved the printing of types so we can see the inferred return types. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33647). * #33643 * #33650 * #33642 * __->__ #33647 --- .../src/HIR/PrintHIR.ts | 3 +- .../Inference/InferMutationAliasingEffects.ts | 18 ++++++++ .../src/TypeInference/InferTypes.ts | 9 ++++ ...in-render-helper-phi-return-prop.expect.md | 44 +++++++++++++++++++ ...global-in-render-helper-phi-return-prop.js | 16 +++++++ ...ate-global-in-render-helper-prop.expect.md | 40 +++++++++++++++++ ...lid-mutate-global-in-render-helper-prop.js | 12 +++++ 7 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 7d2b0b234c7f4..c673ac53d9b10 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -880,7 +880,8 @@ export function printType(type: Type): string { if (type.kind === 'Object' && type.shapeId != null) { return `:T${type.kind}<${type.shapeId}>`; } else if (type.kind === 'Function' && type.shapeId != null) { - return `:T${type.kind}<${type.shapeId}>`; + const returnType = printType(type.return); + return `:T${type.kind}<${type.shapeId}>()${returnType !== '' ? `: ${returnType}` : ''}`; } else { return `:T${type.kind}`; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 36f32213eab79..8dd79409ce151 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -27,6 +27,7 @@ import { InstructionKind, InstructionValue, isArrayType, + isJsxType, isMapType, isPrimitiveType, isRefOrRefValue, @@ -1871,6 +1872,23 @@ function computeSignatureForInstruction( }); } } + for (const prop of value.props) { + if ( + prop.kind === 'JsxAttribute' && + prop.place.identifier.type.kind === 'Function' && + (isJsxType(prop.place.identifier.type.return) || + (prop.place.identifier.type.return.kind === 'Phi' && + prop.place.identifier.type.return.operands.some(operand => + isJsxType(operand), + ))) + ) { + // Any props which return jsx are assumed to be called during render + effects.push({ + kind: 'Render', + place: prop.place, + }); + } + } } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts index 488d988b9715d..d3a297e2e51c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -777,6 +777,15 @@ class Unifier { return {kind: 'Phi', operands: type.operands.map(o => this.get(o))}; } + if (type.kind === 'Function') { + return { + kind: 'Function', + isConstructor: type.isConstructor, + shapeId: type.shapeId, + return: this.get(type.return), + }; + } + return type; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.expect.md new file mode 100644 index 0000000000000..3e3dcab818682 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.expect.md @@ -0,0 +1,44 @@ + +## Input + +```javascript +function Component() { + const renderItem = item => { + // Multiple returns so that the return type is a Phi (union) + if (item == null) { + return null; + } + // Normally we assume that it's safe to mutate globals in a function passed + // as a prop, because the prop could be used as an event handler or effect. + // But if the function returns JSX we can assume it's a render helper, ie + // called during render, and thus it's unsafe to mutate globals or call + // other impure code. + global.property = true; + return ; + }; + return ; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: This value cannot be modified + +Modifying a variable defined outside a component or hook is not allowed. Consider using an effect. + +error.invalid-mutate-global-in-render-helper-phi-return-prop.ts:12:4 + 10 | // called during render, and thus it's unsafe to mutate globals or call + 11 | // other impure code. +> 12 | global.property = true; + | ^^^^^^ value cannot be modified + 13 | return ; + 14 | }; + 15 | return ; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.js new file mode 100644 index 0000000000000..b6ca0a04aeea8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.js @@ -0,0 +1,16 @@ +function Component() { + const renderItem = item => { + // Multiple returns so that the return type is a Phi (union) + if (item == null) { + return null; + } + // Normally we assume that it's safe to mutate globals in a function passed + // as a prop, because the prop could be used as an event handler or effect. + // But if the function returns JSX we can assume it's a render helper, ie + // called during render, and thus it's unsafe to mutate globals or call + // other impure code. + global.property = true; + return ; + }; + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md new file mode 100644 index 0000000000000..9e995d5124d4e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md @@ -0,0 +1,40 @@ + +## Input + +```javascript +function Component() { + const renderItem = item => { + // Normally we assume that it's safe to mutate globals in a function passed + // as a prop, because the prop could be used as an event handler or effect. + // But if the function returns JSX we can assume it's a render helper, ie + // called during render, and thus it's unsafe to mutate globals or call + // other impure code. + global.property = true; + return ; + }; + return ; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: This value cannot be modified + +Modifying a variable defined outside a component or hook is not allowed. Consider using an effect. + +error.invalid-mutate-global-in-render-helper-prop.ts:8:4 + 6 | // called during render, and thus it's unsafe to mutate globals or call + 7 | // other impure code. +> 8 | global.property = true; + | ^^^^^^ value cannot be modified + 9 | return ; + 10 | }; + 11 | return ; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js new file mode 100644 index 0000000000000..9355c482fb61a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js @@ -0,0 +1,12 @@ +function Component() { + const renderItem = item => { + // Normally we assume that it's safe to mutate globals in a function passed + // as a prop, because the prop could be used as an event handler or effect. + // But if the function returns JSX we can assume it's a render helper, ie + // called during render, and thus it's unsafe to mutate globals or call + // other impure code. + global.property = true; + return ; + }; + return ; +} From b870042915c8a7cc1524feb7e0b5cbe7453a7648 Mon Sep 17 00:00:00 2001 From: lauren Date: Wed, 27 Aug 2025 13:59:26 -0400 Subject: [PATCH 3/4] [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 --- .../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 964217c399317..f3007034c3b7d 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 92151501622bf..5a9ef9495fa1d 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 fd68830f929dd..d85c6b19c6738 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 0000000000000..14d86ffe825a5 --- /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 0000000000000..36be05e3ee8bc --- /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 0000000000000..0ab2b977561ee --- /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 0000000000000..306e78f7b1c69 --- /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}], +}; From 0a1f1fcd5080320139bb51021b4325be65d6e2bd Mon Sep 17 00:00:00 2001 From: lauren Date: Wed, 27 Aug 2025 14:37:18 -0400 Subject: [PATCH 4/4] [ci] Cache playwright in run_devtools_e2e_tests (#34320) I happened to notice that I forgot to cache playwright in run_devtools_e2e_tests, so it would try to install it every time which can randomly take a while to complete (I'm not sure why it's not deterministic, but the dependencies appear to be installed inconsistently across multiple workflows). This PR adds the same cache we use for other steps that use playwright, which should shave off some time from this workflow when the cache is warm. Additionally I omitted the standalone install-deps command as it appears to be redundant and adds a lot of extra time to CI, due to the fact that it installs many unrelated dependencies. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34320). * #34321 * __->__ #34320 --- .github/workflows/compiler_playground.yml | 2 -- .github/workflows/runtime_build_and_test.yml | 17 +++++++++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/compiler_playground.yml b/.github/workflows/compiler_playground.yml index 34349f584ef26..a19e87e25e78e 100644 --- a/.github/workflows/compiler_playground.yml +++ b/.github/workflows/compiler_playground.yml @@ -57,8 +57,6 @@ jobs: key: playwright-browsers-v6-${{ runner.arch }}-${{ runner.os }}-${{ steps.playwright_version.outputs.playwright_version }} - run: npx playwright install --with-deps chromium if: steps.cache_playwright_browsers.outputs.cache-hit != 'true' - - run: npx playwright install-deps - if: steps.cache_playwright_browsers.outputs.cache-hit == 'true' - run: CI=true yarn test - run: ls -R test-results if: '!cancelled()' diff --git a/.github/workflows/runtime_build_and_test.yml b/.github/workflows/runtime_build_and_test.yml index f0dbc069f082c..d9fb47da3b204 100644 --- a/.github/workflows/runtime_build_and_test.yml +++ b/.github/workflows/runtime_build_and_test.yml @@ -316,7 +316,7 @@ jobs: if: steps.node_modules.outputs.cache-hit != 'true' - run: ./scripts/react-compiler/build-compiler.sh && ./scripts/react-compiler/link-compiler.sh - run: yarn workspace eslint-plugin-react-hooks test - + # ----- BUILD ----- build_and_lint: name: yarn build and lint @@ -811,9 +811,18 @@ jobs: pattern: _build_* path: build merge-multiple: true - - run: | - npx playwright install - sudo npx playwright install-deps + - name: Check Playwright version + id: playwright_version + run: echo "playwright_version=$(npm ls @playwright/test | grep @playwright | sed 's/.*@//' | head -1)" >> "$GITHUB_OUTPUT" + - name: Cache Playwright Browsers for version ${{ steps.playwright_version.outputs.playwright_version }} + id: cache_playwright_browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-browsers-v6-${{ runner.arch }}-${{ runner.os }}-${{ steps.playwright_version.outputs.playwright_version }} + - name: Playwright install deps + if: steps.cache_playwright_browsers.outputs.cache-hit != 'true' + run: npx playwright install --with-deps chromium - run: ./scripts/ci/run_devtools_e2e_tests.js env: RELEASE_CHANNEL: experimental