Skip to content

Warn when NaN is passed as an aria-* attribute value#36340

Open
starboyvarun wants to merge 1 commit intofacebook:mainfrom
starboyvarun:feat/warn-nan-aria-attributes
Open

Warn when NaN is passed as an aria-* attribute value#36340
starboyvarun wants to merge 1 commit intofacebook:mainfrom
starboyvarun:feat/warn-nan-aria-attributes

Conversation

@starboyvarun
Copy link
Copy Markdown

Summary

aria-* attributes currently bypass the NaN check in ReactDOMUnknownPropertyHook because they return early (line 97-99 of that file) before the check is reached. When a NaN number flows through, the DOM stringifies it to the literal string "NaN", which is invalid for every ARIA attribute type and silently breaks accessibility.

This is a common pitfall when a computed numeric value is derived from user data or state that can be undefined/null:

// e.g. items.findIndex(...) returns -1 → some derived math → NaN
<div aria-valuenow={computedValue} />  // renders aria-valuenow="NaN" — silent bug

Changes:

  • ReactDOMInvalidARIAHook.js — extend validateProperty to accept the prop value and add a NaN check for valid, correctly-cased aria-* attributes. Mirrors the warning message already used by ReactDOMUnknownPropertyHook for non-aria attributes.
  • ReactDOMInvalidARIAHook-test.js — three new test cases: NaN on a numeric aria attribute (aria-valuenow), NaN on a string-type aria attribute (aria-label), and no false-positive for valid integers.

Test plan

  • yarn test ReactDOMInvalidARIAHook — all tests pass (new + existing)
  • Existing tests unaffected — only the validateProperty signature changes (internal)

aria-* attributes bypass the existing NaN check in ReactDOMUnknownPropertyHook
because they return early before that check is reached. When a NaN number
reaches the DOM it gets stringified to "NaN", which is invalid for every ARIA
attribute type and is almost always a developer mistake (e.g. a missing guard
around a computed numeric value).

This adds the same NaN check directly inside ReactDOMInvalidARIAHook so that
any valid aria-* attribute receiving a NaN value produces a clear warning in
development, matching the message already used for non-aria attributes.
@meta-cla meta-cla Bot added the CLA Signed label Apr 24, 2026
Copy link
Copy Markdown

@liufang88789-ui liufang88789-ui left a comment

Choose a reason for hiding this comment

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

Nice fix. The NaN → "NaN" stringification in the DOM is an insidious silent bug, especially since computed numeric values (like derived state from findIndex returning -1) flow into aria-* props frequently.

The placement is right — checking in validateProperty before the return, and mirroring the existing error message pattern keeps it consistent with the rest of the hook. Passing props[key] as the third argument is minimal and clean.

One edge case worth considering: what about Infinity? It has the same stringification problem ("Infinity" is equally invalid for aria attributes), though it's less common in practice. Not necessarily a blocker for this PR, just something to keep in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants