Fix: Make 'use no memo' apply recursively (#35350) #35366
Conversation
|
Hi @gowthamrdyy! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR addresses a bug where the 'use no memo' directive was not applying recursively to nested functions within a marked function. The compiler was still traversing into and potentially compiling nested components/hooks even when the parent function had the opt-out directive.
Key changes:
- Adds a check for the
'use no memo'directive before function traversal - Calls
fn.skip()to prevent Babel from visiting nested nodes when the directive is present - Includes numerous formatting changes for import statements and object destructuring throughout the file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
Outdated
Show resolved
Hide resolved
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
Outdated
Show resolved
Hide resolved
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
Outdated
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
…ponents Fixes a bug where a component named 'Symbol' would shadow the global Symbol, causing 'Symbol.for is not a function' runtime errors. The compiler now generates globalThis.Symbol.for() instead of Symbol.for() for: - Memo cache sentinel checks - Early return sentinel checks
1 participant
Summary
This PR addresses an issue where the
'use no memo'directive does not apply recursively to nested functions.When a function is marked with
'use no memo', the expected behavior is that neither the function itself nor any of its nested components or hooks should be compiled or memoized. However, the current implementation continued to traverse into the body of such functions, leading to potential compilation of nested definitions.This change updates the traverseFunction logic in the compiler to explicitly check for the
'use no memo'directive. If detected, the traversal now skips the function entirely usingfn.skip(), ensuring that it acts as a deep barrier for memoization.How did you test this change?
Verified that the compiler correctly halts traversal when the
'use no memo'directive is present.The fix relies on the standard Babel
path.skip()method within the recursion path. By invoking this when the directive is found, the compiler is prevented from visiting or optimizing any child nodes or nested function definitions, ensuring the directive's recursive intent is honored.