[compiler] Validate against component/hook factories #34305

poteto · 2025-08-26T19:00:21Z

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

jorge-cab

A more general question. When does Program.ts run? does it run in-between compiler passes?

jorge-cab · 2025-08-26T20:20:17Z

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

+      return;
+    }
+
+    if (programContext.alreadyCompiled.has(fn.node)) {


How does alreadyCompiled works? I'm guessing it keep a cache of all compiled functions to avoid repeating work?

Yeah, it's just a simple Set containing all the functions we had already compiled. We added this a long time ago when we saw functions getting compiled multiple times.

jorge-cab · 2025-08-26T20:23:13Z

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

+    // In 'all' mode, compile only top level functions
+    if (
+      pass.opts.compilationMode === 'all' &&
+      fn.scope.getProgramParent() !== fn.scope.parent


What would fn.scope.getProgramParent === fn.scope.parent mean vs. what you wrote in this line?

scope.getProgramParent() is a Babel API that returns the Program node. Comparing these 2 nodes just checks if the fn is in module scope (if it was in module scope, it's parent would also be the Program), or if it's nested within another scope (such as in a BlockStatement). These ASTexplorer console logs might help visualize that better

jorge-cab · 2025-08-26T20:30:44Z

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

-    if (fnType === null || programContext.alreadyCompiled.has(fn.node)) {
+
+    if (pass.opts.environment?.validateNoComponentOrHookFactories) {
+      if (fnType === null || fnType === 'Other') {


This makes it so we don't try to validate top level React Components/Hooks right?

Yeah, for now this validation just checks to see if the parent of a component/hook is a non-React function.

jorge-cab · 2025-08-26T20:30:58Z

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

+    nestedFn.skip();
+  };
+
+  fn.traverse({


If I understand correctly fn: BabelFn is an AST node here and you just traverse recursively to check if you have an n deep React Component/Hook and parentName is always the direct parent of the nested function, is this accurate?

yeah this would traverse any nested Functions and just check each parent/child pair to see if the parent and child are both React functions

poteto · 2025-08-27T16:02:07Z

A more general question. When does Program.ts run? does it run in-between compiler passes?

It runs right at the start of compilation, before we lower the AST into HIR, and after we parse the compiler options. The babel plugin calls Program:compileProgram which then kicks off the core compiler passes.

I needed to make this fix at the AST level, since we were already skipping over non-React functions there (and thus wouldn't have been available within the core compiler pipeline, since it wouldn't have been a part of the HIR).

josephsavona

The validation logic looks good, but i think we should be more clear on how we frame the error to the user

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

josephsavona · 2025-08-27T17:15:19Z

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

+            {
+              kind: 'error',
+              message: 'the component is created here',
+              loc: nestedFn.node.loc ?? null,


I wonder about trying to target the location of the function name, and falling back to the full function scope only if there was no name. Unfortunately Babel doesn't provide a location for the function keyword that we could target.

I was thinking about that too. I think we can use the identifier if its present or fallback to the entire function if there's nothing available to use.

josephsavona

awesome

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

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 DiffTrain build for [b870042](b870042)

meta-cla bot added the CLA Signed label Aug 26, 2025

github-actions bot added the React Core Team Opened by a member of the React Core Team label Aug 26, 2025

poteto force-pushed the pr34305 branch 3 times, most recently from 991872c to e940573 Compare August 26, 2025 19:12

poteto requested review from josephsavona and mofeiZ August 26, 2025 19:13

poteto force-pushed the pr34305 branch from e940573 to 122b562 Compare August 26, 2025 19:37

jorge-cab reviewed Aug 26, 2025

View reviewed changes

josephsavona reviewed Aug 27, 2025

View reviewed changes

poteto force-pushed the pr34305 branch 3 times, most recently from f4a7f65 to 8990ebf Compare August 27, 2025 16:56

josephsavona reviewed Aug 27, 2025

View reviewed changes

josephsavona approved these changes Aug 27, 2025

View reviewed changes

poteto force-pushed the pr34305 branch from 8990ebf to b15fb37 Compare August 27, 2025 17:42

poteto force-pushed the pr34305 branch from b15fb37 to 526d5b9 Compare August 27, 2025 17:50

poteto merged commit b870042 into main Aug 27, 2025
21 checks passed

poteto deleted the pr34305 branch August 27, 2025 17:59

nextjs-bot mentioned this pull request Aug 28, 2025

Upgrade React from 33a1095d-20250827 to 8d7b5e49-20250827 vercel/next.js#83164

Merged

[compiler] Validate against component/hook factories #34305

[compiler] Validate against component/hook factories #34305

Conversation

poteto commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jorge-cab left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poteto commented Aug 27, 2025

Uh oh!

josephsavona left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephsavona left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

poteto commented Aug 26, 2025 •

edited

Loading