Fix: use Object.is for dependency comparison to handle NaN correctly

[everettbu]

Mirror of facebook/react#36034
Original author: MorikawaSouma

---

Summary

Previously, the React Compiler generated code using !==\ for dependency comparison, which doesn't match React's \Object.is\ semantics. This caused NaN in dependency arrays to always trigger re-evaluation since \NaN !== NaN\ is true, but \Object.is(NaN, NaN)\ is true.

Problem

When a dependency value is NaN, the compiled code would incorrectly detect a change on every render:

\\js
// Generated code (before fix):
if ($[0] !== nanValue) { // Always true because NaN !== NaN
// Re-computes every time
}
\\

Solution

Changed the generated code to use !Object.is()\ for dependency comparison:

\\js
// Generated code (after fix):
if (!Object.is($[0], nanValue)) { // Correctly returns false when both are NaN
// Only re-computes when value actually changes
}
\\

Test Plan

• Existing compiler tests should pass
• This matches React's documented equality semantics
• Manual verification with the playground repro from #35854

Fixes #35854

[greptile-apps]

Greptile Summary

This PR bundles two distinct, unrelated changes: (1) a correct and well-motivated fix in the React Compiler that replaces !== with !Object.is() for dependency comparisons to handle NaN correctly, and (2) modifications to React.memo and React.forwardRef that introduce a compareWithRef default comparator for memo(forwardRef(...)) components along with a new isForwardRef export.

Key concerns:

• Logic bug in ReactMemo.js: The compareWithRef function's oldRef and newRef parameters will always be undefined at runtime. The reconciler (ReactFiberBeginWork.js line 526) calls compare(prevProps, nextProps) with exactly two arguments — ref values are never passed to compare. The ref comparison inside compareWithRef (oldRef === newRef) therefore always evaluates to true (undefined === undefined), making it dead code and the overall behavior identical to plain shallowEqual. The new four-argument compare function signature on React.memo is misleading to consumers.
• Unused export in ReactForwardRef.js: The newly exported isForwardRef utility function is never imported by ReactMemo.js, which duplicates the same check inline. This results in dead exports and code duplication.
• Unrelated concerns should be separate PRs: The compiler NaN fix is sound and independent of the memo/forwardRef changes; mixing them makes this PR harder to review and reason about.

Confidence Score: 2/5

• Not safe to merge as-is due to a logic bug in the memo/forwardRef changes, though the compiler NaN fix itself is correct.
• The compiler change is correct and isolated. However, the ReactMemo.js change introduces a compareWithRef function whose ref-comparison logic is never exercised because the reconciler does not pass ref arguments to the compare callback. This makes the stated purpose of the change (including refs in the memoization check for forwardRef components) ineffective. The API surface change (4-argument compare signature) is also misleading. Additionally, the isForwardRef helper is exported but never used.
• packages/react/src/ReactMemo.js requires close attention — the compareWithRef default comparator does not function as intended.

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts	Core NaN fix: replaces !== with !Object.is() for dependency comparisons in generated code, correctly aligning with React's equality semantics. The change is focused, well-commented, and correct.
packages/react/src/ReactMemo.js	Adds compareWithRef default comparator for memo(forwardRef(...)) components, but the ref parameters (oldRef, newRef) are never provided by the reconciler — it only calls compare(prevProps, nextProps) with two arguments. The ref comparison is dead code and the changed function signature is misleading.
packages/react/src/ReactForwardRef.js	Exports a new isForwardRef utility function, but it is unused — ReactMemo.js duplicates the same logic inline instead of importing it.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: packages/react/src/ReactMemo.js
Line: 33-35

Comment:
**Ref parameters in `compareWithRef` are always `undefined`**

The reconciler in `ReactFiberBeginWork.js` (line 526) calls the `compare` function with only two arguments — `prevProps` and `nextProps`:

```js
// ReactFiberBeginWork.js
if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) {
```

No ref values are passed. This means `oldRef` and `newRef` inside `compareWithRef` will always be `undefined`, making `oldRef === newRef` unconditionally `true` (i.e., `undefined === undefined`). The ref comparison logic here is effectively dead code and will never behave as described in the comment above.

Additionally, the reconciler already checks ref equality *outside* the compare call (`&& current.ref === workInProgress.ref`), so even if refs were passed they would be checked twice.

The `compare` function signature change to accept `(oldProps, newProps, oldRef, newRef)` (line 15) is also misleading to consumers, since the runtime never supplies those arguments.

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

---

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

Comment:
**`isForwardRef` exported but never imported**

The new `isForwardRef` helper is exported here, but `ReactMemo.js` does not import or use it. Instead, `ReactMemo.js` duplicates the exact same check inline:

```js
// ReactMemo.js line 28 — duplicated, not using this export
const isForwardRefComponent = typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;
```

Either `ReactMemo.js` should import and use this helper, or this export can be removed to avoid dead exports.

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

_{Last reviewed commit: bbad60d}

[greptile-apps]

Ref parameters in compareWithRef are always undefined

The reconciler in ReactFiberBeginWork.js (line 526) calls the compare function with only two arguments — prevProps and nextProps:

// ReactFiberBeginWork.js
if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) {

No ref values are passed. This means oldRef and newRef inside compareWithRef will always be undefined, making oldRef === newRef unconditionally true (i.e., undefined === undefined). The ref comparison logic here is effectively dead code and will never behave as described in the comment above.

Additionally, the reconciler already checks ref equality outside the compare call (&& current.ref === workInProgress.ref), so even if refs were passed they would be checked twice.

The compare function signature change to accept (oldProps, newProps, oldRef, newRef) (line 15) is also misleading to consumers, since the runtime never supplies those arguments.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/react/src/ReactMemo.js
Line: 33-35

Comment:
**Ref parameters in `compareWithRef` are always `undefined`**

The reconciler in `ReactFiberBeginWork.js` (line 526) calls the `compare` function with only two arguments — `prevProps` and `nextProps`:

```js
// ReactFiberBeginWork.js
if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) {
```

No ref values are passed. This means `oldRef` and `newRef` inside `compareWithRef` will always be `undefined`, making `oldRef === newRef` unconditionally `true` (i.e., `undefined === undefined`). The ref comparison logic here is effectively dead code and will never behave as described in the comment above.

Additionally, the reconciler already checks ref equality *outside* the compare call (`&& current.ref === workInProgress.ref`), so even if refs were passed they would be checked twice.

The `compare` function signature change to accept `(oldProps, newProps, oldRef, newRef)` (line 15) is also misleading to consumers, since the runtime never supplies those arguments.

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

[greptile-apps]

isForwardRef exported but never imported

The new isForwardRef helper is exported here, but ReactMemo.js does not import or use it. Instead, ReactMemo.js duplicates the exact same check inline:

// ReactMemo.js line 28 — duplicated, not using this export
const isForwardRefComponent = typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;

Either ReactMemo.js should import and use this helper, or this export can be removed to avoid dead exports.

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` exported but never imported**

The new `isForwardRef` helper is exported here, but `ReactMemo.js` does not import or use it. Instead, `ReactMemo.js` duplicates the exact same check inline:

```js
// ReactMemo.js line 28 — duplicated, not using this export
const isForwardRefComponent = typeof type === 'object' && type !== null && type.$$typeof === REACT_FORWARD_REF_TYPE;
```

Either `ReactMemo.js` should import and use this helper, or this export can be removed to avoid dead exports.

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

[everettbu]

Upstream PR was closed or merged. Code is synced via branch mirror.