Skip to content

Fix React.memo not working with forwardRef components #686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
everettbu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix React.memo not working with forwardRef components #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/react/src/ReactForwardRef.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@

import {REACT_FORWARD_REF_TYPE, REACT_MEMO_TYPE} from 'shared/ReactSymbols';

export function isForwardRef(type: mixed): boolean %checks {
return typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;
}
Comment on lines +12 to +14
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isForwardRef helper is dead code — never imported or used

The new isForwardRef export is never imported anywhere. ReactMemo.js duplicates the same check inline:

// ReactMemo.js line 28
const isForwardRefComponent = typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;

If the intent was to share this utility, ReactMemo.js should import and use isForwardRef from this file instead of reimplementing it. As written, this export is unreachable dead code.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react/src/ReactForwardRef.js
Line: 12-14

Comment:
**`isForwardRef` helper is dead code — never imported or used**

The new `isForwardRef` export is never imported anywhere. `ReactMemo.js` duplicates the same check inline:

```js
// ReactMemo.js line 28
const isForwardRefComponent = typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;
```

If the intent was to share this utility, `ReactMemo.js` should import and use `isForwardRef` from this file instead of reimplementing it. As written, this export is unreachable dead code.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex


export function forwardRef<Props, ElementType: React$ElementType>(
render: (
props: Props,
Expand Down
19 changes: 16 additions & 3 deletions packages/react/src/ReactMemo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
* @noflow
*/

import {REACT_MEMO_TYPE} from 'shared/ReactSymbols';
import {REACT_MEMO_TYPE, REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols';
import shallowEqual from 'shared/shallowEqual';

export function memo<Props>(
type: React$ElementType,
compare?: (oldProps: Props, newProps: Props) => boolean,
compare?: (oldProps: Props, newProps: Props, oldRef: mixed, newRef: mixed) => boolean,
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading public API type signature change

The compare callback type has been changed to include oldRef and newRef parameters:

compare?: (oldProps: Props, newProps: Props, oldRef: mixed, newRef: mixed) => boolean,

However, as described above, the reconciler never passes these arguments — they will always be undefined when compare is invoked. Any user who reads this type signature and writes a custom compare function that relies on oldRef/newRef will find those parameters are always undefined, leading to subtle, hard-to-debug bugs. The public type signature should not be changed to advertise parameters that are never populated.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react/src/ReactMemo.js
Line: 15

Comment:
**Misleading public API type signature change**

The `compare` callback type has been changed to include `oldRef` and `newRef` parameters:

```js
compare?: (oldProps: Props, newProps: Props, oldRef: mixed, newRef: mixed) => boolean,
```

However, as described above, the reconciler never passes these arguments — they will always be `undefined` when `compare` is invoked. Any user who reads this type signature and writes a custom `compare` function that relies on `oldRef`/`newRef` will find those parameters are always `undefined`, leading to subtle, hard-to-debug bugs. The public type signature should not be changed to advertise parameters that are never populated.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

) {
if (__DEV__) {
if (type == null) {
Expand All @@ -22,10 +23,22 @@ export function memo<Props>(
);
}
}

// Create custom compare function that includes ref for forwardRef components
const isForwardRefComponent = typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;
let finalCompare = compare;

if (isForwardRefComponent && compare === undefined) {
// Default compare for forwardRef: shallow equal props + strict equal ref
finalCompare = function compareWithRef(oldProps, newProps, oldRef, newRef) {
return shallowEqual(oldProps, newProps) && oldRef === newRef;
};
}
Comment on lines +31 to +36
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is a no-op — the reconciler never passes ref arguments to compare

The custom compareWithRef function accepts (oldProps, newProps, oldRef, newRef), but the React reconciler in ReactFiberBeginWork.js always invokes the compare function with only two arguments:

// ReactFiberBeginWork.js ~line 526
compare = compare !== null ? compare : shallowEqual;
if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) {

Because oldRef and newRef are never passed by the reconciler, they will always be undefined inside compareWithRef. The check oldRef === newRef therefore always evaluates to undefined === undefined → true. This makes finalCompare functionally equivalent to just shallowEqual(oldProps, newProps) — which is already the default behavior when compare is null (the reconciler falls back to shallowEqual).

Furthermore, the reconciler already compares refs independently with current.ref === workInProgress.ref on the same line, so ref-based bailing-out already works without any changes here. This PR does not change the observable behavior of React.memo at all.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react/src/ReactMemo.js
Line: 31-36

Comment:
**The fix is a no-op — the reconciler never passes ref arguments to `compare`**

The custom `compareWithRef` function accepts `(oldProps, newProps, oldRef, newRef)`, but the React reconciler in `ReactFiberBeginWork.js` always invokes the `compare` function with **only two arguments**:

```js
// ReactFiberBeginWork.js ~line 526
compare = compare !== null ? compare : shallowEqual;
if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) {
```

Because `oldRef` and `newRef` are never passed by the reconciler, they will always be `undefined` inside `compareWithRef`. The check `oldRef === newRef` therefore always evaluates to `undefined === undefined` → `true`. This makes `finalCompare` functionally equivalent to just `shallowEqual(oldProps, newProps)` — which is **already the default behavior** when `compare` is `null` (the reconciler falls back to `shallowEqual`).

Furthermore, the reconciler already compares refs independently with `current.ref === workInProgress.ref` on the same line, so ref-based bailing-out already works without any changes here. This PR does not change the observable behavior of `React.memo` at all.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex


const elementType = {
$$typeof: REACT_MEMO_TYPE,
type,
compare: compare === undefined ? null : compare,
compare: finalCompare === undefined ? null : finalCompare,
};
if (__DEV__) {
let ownName;
Expand Down
Loading