[devtools] Prevent incorrect render detection for user components in didFiberRender (#33423)

[developerjhp]

Summary

Fixes false positive rendering detection in React DevTools Profiler by improving the didFiberRender function to accurately detect when user components actually re-render, preventing misleading "The parent component rendered" messages.

Problem

Previously, React DevTools would incorrectly mark components as "rendered" even when they didn't actually re-render due to bailouts. This happened because the didFiberRender function only checked the PerformedWork flag, but React can set this flag even during bailout scenarios.

Example scenario:

• Parent component state changes
• Sibling component with unchanged props shows "The parent component rendered"
• But the sibling component console.log shows it didn't actually re-render

Solution

Enhanced didFiberRender function for user components (ClassComponent, FunctionComponent, etc.):

// Before
const PerformedWork = 0b000000000000000000000000001;
return (getFiberFlags(nextFiber) & PerformedWork) === PerformedWork;

// After  
if ((getFiberFlags(nextFiber) & PerformedWork) === 0) {
  return false;
}
if (
  prevFiber != null &&
  prevFiber.memoizedProps === nextFiber.memoizedProps &&
  prevFiber.memoizedState === nextFiber.memoizedState &&
  prevFiber.ref === nextFiber.ref
) {
  // React may mark PerformedWork even if we bailed out. Double check
  // that inputs actually changed before reporting a render.
  return false;
}
return true;

This change ensures that:

1. We first check the PerformedWork flag (performance optimization)
2. Then verify that props/state/ref actually changed (accuracy check)
3. Only report rendering when inputs genuinely changed

Testing

Test Setup:
Used the following test case with independent Count and Greeting components:

const Count = () => {
    const [count, setCount] = useState(0);
    console.log('Count Component Rerendered');
    return (
        <button onClick={() => setCount(c => c + 1)}>
            Count: {count}
        </button>
    );
};

const Greeting = () => {
    console.log('Greeting Component Rerendered');
    return <span>Hello World!</span>;
};

const App = () => {
    const [appState, setAppState] = useState(0);
    console.log('App Component Rerendered');
    
    return (
        <main>
            <div>App State: {appState}</div>
            <button onClick={() => setAppState(s => s + 1)}>
                App Rerender Trigger (All children rerender)
            </button>
            <hr />
            <Count />
            <div>
                <Greeting />
            </div>
        </main>
    );
};

Test Results:
✅ Tested and verified with this code

// Before

Screen.Recording.2025-06-04.at.13.17.03.mov

// After

Screen.Recording.2025-06-04.at.13.17.35.mov

Before Fix:

• Click Count button → Console shows only "Count Component Rerendered"
• DevTools Profiler → Greeting component incorrectly shows "The parent component rendered"

After Fix:

• Click Count button → Console shows only "Count Component Rerendered"
• DevTools Profiler → Greeting component correctly shows no rendering information

Related

This change specifically targets user components (Function/Class components) and maintains existing behavior for host components, ensuring accurate rendering detection across the React component tree.

Fixes #33423 , #19732

[michaelboyles]

I tested your solution and it seems to work. Adding this to the top of the function instead seems to achieve the same thing. Not sure if they are practically equivalent or not

if (prevFiber === nextFiber) {
    return false;
}

[developerjhp]

@michaelboyles

Thanks for testing it out!

You're right — prevFiber === nextFiber can short-circuit some cases and slightly improve performance.
However, it's not functionally equivalent to checking memoizedProps, memoizedState, and ref,
since React may sometimes create a new fiber even when nothing actually changed.

So while it's a great optimization to add on top, it can't fully replace the deeper equality check.
We could consider including both, to balance correctness and performance.

Open to thoughts on this!

[hoxyq]

Thank you for upstreaming this, but that's not the right fix for this problem.

Please see my comment here for more context.

[michaelboyles]

@hoxyq I read your comment there and understood it, but I don't see how that alone rules out this change. Perhaps you just know from experience and the intent of this method that it's not right. Can you explain?

The issue when I debugged this seems to be that didFiberRender returns true for fibers that didn't render in the current pass, but potentially rendered a long time ago.

In the error case, it's called with the same fiber instance in both arguments, which is why I suggested simple equality #33434 (comment) That also appeared to fix it in my testing without any of the changes in this PR being necessary

Are you saying the function is correct, but it should never be called for these fibers?

[dcq01]

Hi! I'm a grad student working on a research project about using large language models to automate code review. Based on your commit 995f9da and the changes in packages/react-devtools-shared/src/backend/fiber/renderer.js, my tool generated this comment:

1. Null Checks: Ensure that nextFiber is validated before accessing its properties to avoid potential null reference exceptions.
2. Behavioral Change: The logic change from returning a boolean based on PerformedWork to a more complex condition may affect existing components that rely on the previous behavior. Ensure that any components using this function are tested to confirm they handle the new logic correctly.
3. Testing Required: Given the changes in logic, thorough testing is required to ensure that existing functionality is not broken. Pay special attention to components that may not have been updated to handle the new checks.
4. Flag Check Logic: The new logic correctly identifies that if no work was performed (PerformedWork flag is not set), it should not report a render.
5. Return Type Consistency: Ensure that the function's return type is consistent and well-documented, especially since it now has multiple return points.
6. Comparison of Fiber Properties: The additional checks comparing prevFiber and nextFiber properties (i.e., memoizedProps, memoizedState, and ref) are a necessary optimization to prevent unnecessary renders.
7. Combine Conditions: Extract the logic that checks multiple properties of prevFiber and nextFiber into a separate function (e.g., areFibersEqual(prevFiber, nextFiber)) to improve readability.
8. Early Return Optimization: Consider optimizing the order of checks. If getFiberFlags(nextFiber) is expensive, check prevFiber properties first, especially if they are often equal. This could reduce calls to getFiberFlags.
9. Separation of Concerns: The logic for determining whether to report a render is tightly coupled within the attach function. It may be beneficial to extract this logic into a separate utility function.
10. Magic Numbers: The use of the PerformedWork constant as a magic number could be abstracted further. Consider defining a more descriptive constant or an enum that explains its purpose.
11. Documentation Update: If this function is part of a public API, consider updating the documentation to reflect the new behavior and any version dependencies to avoid confusion for users relying on the previous implementation.

As part of my research, I'm trying to understand how useful these comments are in real-world development. If you have a moment, I'd be super grateful if you could quickly reply to these two yes/no questions:

1. Does this comment provide suggestions from a dimension you hadn’t considered?
2. 2. Do you find this comment helpful?

Thanks a lot for your time and feedback! And sorry again if this message is a bother.