feat: NumberParser assorted bug fixes by snowystinger · Pull Request #8592 · adobe/react-spectrum

snowystinger · 2025-07-21T07:44:44Z

Closes #7015, Closes #3862, Closes #6861, Closes #5927, Closes #9086

Finally had some thoughts on how to address some of the issues that were being tackled in #6520
But due to concerns, it has been removed #8592 (comment)
Will open a follow up PR that has these changes for further consideration.

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

See each bug for reproductions.

🧢 Your Project:

…cters

rspbot · 2025-07-21T07:46:09Z

Build successful! 🎉

rspbot · 2025-07-21T07:56:04Z

Build successful! 🎉

# Conflicts: # packages/@internationalized/number/src/NumberParser.ts # packages/@internationalized/number/test/NumberParser.test.js

rspbot · 2025-08-22T01:17:39Z

Build successful! 🎉

rspbot · 2025-08-27T04:52:25Z

Build successful! 🎉

rspbot · 2025-09-11T00:00:35Z

Build successful! 🎉

LFDanLu

Verified that this fixes the linked issues and the updates to the parser logic make sense. Perhaps a bit silly of a case, but what would we expect if someone types "123, 456"? It has the group separator and a non matching symbol from a different format and thus resolves to "123.456" since the space becomes the decimal separator but perhaps someone could expect it to resolve to "123,456"

LFDanLu · 2025-09-17T17:59:01Z

packages/react-aria-components/stories/NumberField.stories.tsx

verified the fix for #6861 here, but noticed that I can't go into the negatives it seems? It seems to cycle between what I think is 1 and 0.

interesting case. I'll look into that as follow up

snowystinger · 2025-09-17T23:28:58Z

Verified that this fixes the linked issues and the updates to the parser logic make sense. Perhaps a bit silly of a case, but what would we expect if someone types "123, 456"? It has the group separator and a non matching symbol from a different format and thus resolves to "123.456" since the space becomes the decimal separator but perhaps someone could expect it to resolve to "123,456"

Good question, that is a little ambiguous, think I should fail to parse it instead? I could limit the failure to parse case to no numbers between the two symbols.

LFDanLu · 2025-09-18T00:25:06Z

Yeah I think maybe err on the side of caution and fail to parse it if there aren't any numbers between the two symbols

rspbot · 2025-10-26T23:31:46Z

Build successful! 🎉

This reverts commit f459439.

rspbot · 2025-10-26T23:46:11Z

Build successful! 🎉

This reverts commit 6495950.

packages/react-aria-components/test/NumberField.test.js

rspbot · 2025-10-28T00:32:02Z

Build successful! 🎉

rspbot · 2026-01-05T00:40:50Z

Build successful! 🎉

rspbot · 2026-01-07T01:56:06Z

Build successful! 🎉

# Conflicts: # packages/react-aria-components/test/NumberField.test.js

rspbot · 2026-01-28T04:58:29Z

Build successful! 🎉

LFDanLu

LGTM, reverified that the issues linked are all fixed now and tried a variety of separator/decimal combinations. Might be worthwhile creating a ticket for the followup items just so we don't forget

devongovett · 2026-01-30T23:43:29Z

packages/@internationalized/number/src/NumberParser.ts

+        value = replaceAll(value, '__DECIMAL__', this.symbols.decimal!);
+      }
+    }
+


what is this doing? this makes me scared. 😂

These are just temp strings that allow you to swap group and decimal characters, for instance
"1.200,001"
If I try to replace either one immediately, then the number is changed in a way I can't recover
"1.200.001" or "1,200,001"

So I just replace them with the substitution
"1__GROUP__200__DECIMAL__001" so that I can easily fix it to "1,200.001"

Could I do it with one less set of replaces, yes. I am sometimes a little extra explicit for my own reading comprehension. I'll update it.

devongovett · 2026-01-31T02:27:54Z

I feel like we're making a lot of assumptions here. How often are people pasting a value that's formatted in a different locale? Looking through the issues above, several of them seem more related to parsing in the wrong locale (e.g. detecting , as arabic), or other issues. Maybe it would help me if you could point out which parts of the changes relate to which issues?

rspbot · 2026-02-01T22:22:08Z

Build successful! 🎉

snowystinger · 2026-02-02T01:45:33Z

Maybe it would help me if you could point out which parts of the changes relate to which issues?

Sure, I'll do my best. Please note that many of the changes came from running the fuzzing tests compliments of FastCheck.

#6861
Which is tested by, NumberParser.test line 207
It's implemented on line 205 of NumberParser and has a comment.

#5927 was fixed by line 244
It's test is here https://github.com/adobe/react-spectrum/pull/8592/changes#diff-65d8f601ed61f6cdfe627db4fbb5568cdb164084d0a8c637cc9d3e3ee58a6c77R326

#3862 and #7015 were fixed line 237 in tandem with the fix for 5927
It's test is here https://github.com/adobe/react-spectrum/pull/8592/changes#diff-7bf83599612e94f27f7a4a9ce5457ccc8c68b7ea557c05b16b02e2ea6515e426R417

#9086
Test is covered in NumberParser.test.js, lines 64-67, 427-430
The change was to check the resolvedOptions for useGrouping, line 135 of NumberParser, and return NaN if there is a group symbol but useGrouping is false, line 140.
This, and the logic for pulling the number out of the string in sanitize, line 209 because they can be valid partial numbers and there can be literals

There's more info about my thinking here: #9089

Note, the check for isGroupSymbolAllowed became a combined check with this.symbols.group.

Finally, the other changes in this PR came from #6520 and 9089 where I determined we're making assumptions and things are ambiguous currently. So this PR actually intends to make less assumptions. There are code comments on the other three blocks of code to explain.
Multiple decimal separators
Decimal separator before group
Different formats

And then the last block is in support of those three https://github.com/adobe/react-spectrum/pull/8592/changes#diff-983e1f4675c7ada3abd53503639a16a3f999170790ad28dc5b3f9b0ad6ec8731R209
Which is tested here: https://github.com/adobe/react-spectrum/pull/8592/changes#diff-65d8f601ed61f6cdfe627db4fbb5568cdb164084d0a8c637cc9d3e3ee58a6c77R215
From my testing, numbers may contains things we previously considered a global literal, but when it's inside the number may have meaning. One case was pasting from another format (someone copying from a non-locale aware context (PDF) into our app), but there was also the number as a literal case. So I've opted to preserve the number itself and reduce our global deletions in the string.

If it would make things better, I can split out some of these into their own PRs.

rspbot · 2026-02-04T21:16:41Z

Build successful! 🎉

devongovett · 2026-02-07T02:23:14Z

Yeah I'm still a bit concerned about trying to guess what people meant and auto-correct it, e.g. the swapping of group separators and decimals. Someone might be in the middle of editing, blur to go check another tab, come back and what they were doing has re-formatted to a completely different value. Disambiguating this characters is one of the main reasons we use the locale for parsing in the first place.

devongovett · 2026-02-07T02:27:00Z

packages/@internationalized/number/src/NumberParser.ts

+
+      // Replace literals in the parts outside the number
+      beforeNumber = beforeNumber.replace(this.symbols.literals, '');
+      afterNumber = afterNumber.replace(this.symbols.literals, '');


When are literals allowed inside the number?

group separators and decimals may be literals in other locales, very specifically, whitespaces

# Conflicts: # packages/react-aria-components/test/NumberField.test.js

snowystinger · 2026-02-09T00:30:43Z

Someone might be in the middle of editing, blur to go check another tab, come back and what they were doing has re-formatted to a completely different value

This is already the case, the value could be cleared, reverted to a previous valid value, or formatted as a complete value.

rspbot · 2026-02-09T01:47:03Z

Build successful! 🎉

snowystinger added 9 commits July 16, 2025 15:33

fix: NumberParsing

5b57af9

fix test usage

95874ef

fuzzier matching for numbers

6160d5f

fix ambiguous case with leading zeroes

fb0aa08

fix last breaking tests

3583f8c

fix case of formatted numbers with no numerals

9637ec7

fix cases of numbers with no numerals

8a312f3

remove ambiguous case but allow for numbers to start with group chara…

8897030

…cters

explanation

f826761

Merge branch 'main' into fix-some-number-validation

4727d7c

snowystinger added the ready for review label Jul 22, 2025

Merge branch 'main' into fix-some-number-validation

aa062a0

# Conflicts: # packages/@internationalized/number/src/NumberParser.ts # packages/@internationalized/number/test/NumberParser.test.js

Merge branch 'main' into fix-some-number-validation

56ab8ad

Merge branch 'main' into fix-some-number-validation

9ddff90

LFDanLu previously approved these changes Sep 17, 2025

View reviewed changes

handle ambiguous group vs decimal case

f459439

snowystinger dismissed LFDanLu’s stale review via f459439 October 26, 2025 23:23

snowystinger mentioned this pull request Oct 26, 2025

fix: NumberParser useGrouping false returned wrong value #9089

Closed

5 tasks

Revert "handle ambiguous group vs decimal case"

6495950

This reverts commit f459439.

Reapply "handle ambiguous group vs decimal case"

c6dcf4d

This reverts commit 6495950.

snowystinger commented Oct 26, 2025

View reviewed changes

packages/react-aria-components/test/NumberField.test.js Outdated Show resolved Hide resolved

Merge branch 'main' into fix-some-number-validation

a3c25ad

Merge branch 'main' into fix-some-number-validation

416adda

Merge branch 'main' into fix-some-number-validation

0c94f85

snowystinger mentioned this pull request Jan 7, 2026

NumberField should not allow spaces for percents or currencies that don't use spaces #9225

Open

devongovett mentioned this pull request Jan 15, 2026

fix: #4967 useNumberField mutates number so screenreader users won’t know that their value has changed #6520

Closed

5 tasks

snowystinger added the release label Jan 27, 2026

Merge branch 'main' into fix-some-number-validation

bf71d12

# Conflicts: # packages/react-aria-components/test/NumberField.test.js

LFDanLu previously approved these changes Jan 30, 2026

View reviewed changes

devongovett reviewed Jan 30, 2026

View reviewed changes

snowystinger removed the release label Feb 1, 2026

simplify

3e442ba

snowystinger dismissed LFDanLu’s stale review via 3e442ba February 1, 2026 22:12

add test for 5927

610bf69

Merge branch 'main' into fix-some-number-validation

5325557

devongovett reviewed Feb 7, 2026

View reviewed changes

Merge branch 'main' into fix-some-number-validation

49e7b5e

# Conflicts: # packages/react-aria-components/test/NumberField.test.js

remove extra handling

de2fbe8

snowystinger mentioned this pull request Feb 9, 2026

fix: parsing for numbers from other locales or common accidental form… #9623

Open

5 tasks

snowystinger changed the title ~~feat: fuzzier number matching for NumberParser~~ feat: NumberParser assorted bug fixes Feb 17, 2026

Conversation

snowystinger commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

rspbot commented Jul 21, 2025

Uh oh!

rspbot commented Jul 21, 2025

Uh oh!

rspbot commented Aug 22, 2025

Uh oh!

rspbot commented Aug 27, 2025

Uh oh!

rspbot commented Sep 11, 2025

Uh oh!

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

LFDanLu Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

snowystinger commented Sep 17, 2025

Uh oh!

LFDanLu commented Sep 18, 2025

Uh oh!

rspbot commented Oct 26, 2025

Uh oh!

rspbot commented Oct 26, 2025

Uh oh!

Uh oh!

rspbot commented Oct 28, 2025

Uh oh!

rspbot commented Jan 5, 2026

Uh oh!

rspbot commented Jan 7, 2026

Uh oh!

rspbot commented Jan 28, 2026

Uh oh!

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

devongovett Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

devongovett commented Jan 31, 2026

Uh oh!

rspbot commented Feb 1, 2026

Uh oh!

snowystinger commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rspbot commented Feb 4, 2026

Uh oh!

devongovett commented Feb 7, 2026

Uh oh!

devongovett Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snowystinger commented Feb 9, 2026

Uh oh!

rspbot commented Feb 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

snowystinger commented Jul 21, 2025 •

edited

Loading

snowystinger commented Feb 2, 2026 •

edited

Loading

snowystinger Feb 8, 2026 •

edited

Loading