Skip to content

Commit 4cb6dd3

Browse files
authored
NumberParser Percent and SigFig fixes (#5156)
1 parent 7427349 commit 4cb6dd3

File tree

2 files changed

+173
-16
lines changed

2 files changed

+173
-16
lines changed

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

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13+
import {NumberFormatter} from './NumberFormatter';
14+
1315
interface Symbols {
1416
minusSign: string,
1517
plusSign: string,
@@ -102,11 +104,16 @@ class NumberParserImpl {
102104
formatter: Intl.NumberFormat;
103105
options: Intl.ResolvedNumberFormatOptions;
104106
symbols: Symbols;
107+
locale: string;
105108

106109
constructor(locale: string, options: Intl.NumberFormatOptions = {}) {
110+
this.locale = locale;
107111
this.formatter = new Intl.NumberFormat(locale, options);
108112
this.options = this.formatter.resolvedOptions();
109-
this.symbols = getSymbols(this.formatter, this.options, options);
113+
this.symbols = getSymbols(locale, this.formatter, this.options, options);
114+
if (this.options.style === 'percent' && ((this.options.minimumFractionDigits ?? 0) > 18 || (this.options.maximumFractionDigits ?? 0) > 18)) {
115+
console.warn('NumberParser cannot handle percentages with greater than 18 decimal places, please reduce the number in your options.');
116+
}
110117
}
111118

112119
parse(value: string) {
@@ -119,23 +126,50 @@ class NumberParserImpl {
119126
.replace(this.symbols.minusSign, '-')
120127
.replace(this.symbols.numeral, this.symbols.index);
121128

129+
if (this.options.style === 'percent') {
130+
// javascript is bad at dividing by 100 and maintaining the same significant figures, so perform it on the string before parsing
131+
let isNegative = fullySanitizedValue.indexOf('-');
132+
fullySanitizedValue = fullySanitizedValue.replace('-', '');
133+
let index = fullySanitizedValue.indexOf('.');
134+
if (index === -1) {
135+
index = fullySanitizedValue.length;
136+
}
137+
fullySanitizedValue = fullySanitizedValue.replace('.', '');
138+
if (index - 2 === 0) {
139+
fullySanitizedValue = `0.${fullySanitizedValue}`;
140+
} else if (index - 2 === -1) {
141+
fullySanitizedValue = `0.0${fullySanitizedValue}`;
142+
} else if (index - 2 === -2) {
143+
fullySanitizedValue = '0.00';
144+
} else {
145+
fullySanitizedValue = `${fullySanitizedValue.slice(0, index - 2)}.${fullySanitizedValue.slice(index - 2)}`;
146+
}
147+
if (isNegative > -1) {
148+
fullySanitizedValue = `-${fullySanitizedValue}`;
149+
}
150+
}
151+
122152
let newValue = fullySanitizedValue ? +fullySanitizedValue : NaN;
123153
if (isNaN(newValue)) {
124154
return NaN;
125155
}
126156

157+
if (this.options.style === 'percent') {
158+
// extra step for rounding percents to what our formatter would output
159+
let options = {
160+
...this.options,
161+
style: 'decimal',
162+
minimumFractionDigits: Math.min(this.options.minimumFractionDigits + 2, 20),
163+
maximumFractionDigits: Math.min(this.options.maximumFractionDigits + 2, 20)
164+
};
165+
return (new NumberParser(this.locale, options)).parse(new NumberFormatter(this.locale, options).format(newValue));
166+
}
167+
127168
// accounting will always be stripped to a positive number, so if it's accounting and has a () around everything, then we need to make it negative again
128169
if (this.options.currencySign === 'accounting' && CURRENCY_SIGN_REGEX.test(value)) {
129170
newValue = -1 * newValue;
130171
}
131172

132-
// when reading the number, if it's a percent, then it should be interpreted as being divided by 100
133-
if (this.options.style === 'percent') {
134-
newValue /= 100;
135-
// after dividing to get the percent value, javascript may get .0210999999 instead of .0211, so fix the number of fraction digits
136-
newValue = +newValue.toFixed((this.options.maximumFractionDigits ?? 0) + 2);
137-
}
138-
139173
return newValue;
140174
}
141175

@@ -179,6 +213,11 @@ class NumberParserImpl {
179213
return false;
180214
}
181215

216+
// Numbers that can't have any decimal values fail if a decimal character is typed
217+
if (value.indexOf(this.symbols.decimal) > -1 && this.options.maximumFractionDigits === 0) {
218+
return false;
219+
}
220+
182221
// Remove numerals, groups, and decimals
183222
value = replaceAll(value, this.symbols.group, '')
184223
.replace(this.symbols.numeral, '')
@@ -198,11 +237,13 @@ const pluralNumbers = [
198237
0, 4, 2, 1, 11, 20, 3, 7, 100, 21, 0.1, 1.1
199238
];
200239

201-
function getSymbols(formatter: Intl.NumberFormat, intlOptions: Intl.ResolvedNumberFormatOptions, originalOptions: Intl.NumberFormatOptions): Symbols {
240+
function getSymbols(locale: string, formatter: Intl.NumberFormat, intlOptions: Intl.ResolvedNumberFormatOptions, originalOptions: Intl.NumberFormatOptions): Symbols {
241+
// formatter needs access to all decimal places in order to generate the correct literal strings for the plural set
242+
let symbolFormatter = new Intl.NumberFormat(locale, {...intlOptions, minimumSignificantDigits: 1, maximumSignificantDigits: 21});
202243
// Note: some locale's don't add a group symbol until there is a ten thousands place
203-
let allParts = formatter.formatToParts(-10000.111);
204-
let posAllParts = formatter.formatToParts(10000.111);
205-
let pluralParts = pluralNumbers.map(n => formatter.formatToParts(n));
244+
let allParts = symbolFormatter.formatToParts(-10000.111);
245+
let posAllParts = symbolFormatter.formatToParts(10000.111);
246+
let pluralParts = pluralNumbers.map(n => symbolFormatter.formatToParts(n));
206247

207248
let minusSign = allParts.find(p => p.type === 'minusSign')?.value ?? '-';
208249
let plusSign = posAllParts.find(p => p.type === 'plusSign')?.value;
@@ -214,7 +255,11 @@ function getSymbols(formatter: Intl.NumberFormat, intlOptions: Intl.ResolvedNumb
214255
plusSign = '+';
215256
}
216257

217-
let decimal = allParts.find(p => p.type === 'decimal')?.value;
258+
// If maximumSignificantDigits is 1 (the minimum) then we won't get decimal characters out of the above formatters
259+
// Percent also defaults to 0 fractionDigits, so we need to make a new one that isn't percent to get an accurate decimal
260+
let decimalParts = new Intl.NumberFormat(locale, {...intlOptions, minimumFractionDigits: 2, maximumFractionDigits: 2}).formatToParts(0.001);
261+
262+
let decimal = decimalParts.find(p => p.type === 'decimal')?.value;
218263
let group = allParts.find(p => p.type === 'group')?.value;
219264

220265
// this set is also for a regex, it's all literals that might be in the string we want to eventually parse that
@@ -223,7 +268,7 @@ function getSymbols(formatter: Intl.NumberFormat, intlOptions: Intl.ResolvedNumb
223268
let pluralPartsLiterals = pluralParts.flatMap(p => p.filter(p => !nonLiteralParts.has(p.type)).map(p => escapeRegex(p.value)));
224269
let sortedLiterals = [...new Set([...allPartsLiterals, ...pluralPartsLiterals])].sort((a, b) => b.length - a.length);
225270

226-
let literals = sortedLiterals.length === 0 ?
271+
let literals = sortedLiterals.length === 0 ?
227272
new RegExp('[\\p{White_Space}]', 'gu') :
228273
new RegExp(`${sortedLiterals.join('|')}|[\\p{White_Space}]`, 'gu');
229274

@@ -247,5 +292,5 @@ function replaceAll(str: string, find: string, replace: string) {
247292
}
248293

249294
function escapeRegex(string: string) {
250-
return string.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&');
295+
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
251296
}

packages/@internationalized/number/test/NumberParser.test.js

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13+
import fc from 'fast-check';
14+
import messages from '../../../@react-aria/numberfield/intl/*';
1315
import {NumberParser} from '../src/NumberParser';
1416

17+
// for some reason hu-HU isn't supported in jsdom/node
18+
let locales = Object.keys(messages).map(locale => locale.replace('.json', '')).filter(locale => locale !== 'hu-HU');
19+
1520
describe('NumberParser', function () {
1621
describe('parse', function () {
1722
it('should support basic numbers', function () {
@@ -155,10 +160,117 @@ describe('NumberParser', function () {
155160
});
156161

157162
it('should parse a percent with decimals', function () {
158-
expect(new NumberParser('en-US', {style: 'percent'}).parse('10.5%')).toBe(0.1);
163+
expect(new NumberParser('en-US', {style: 'percent'}).parse('10.5%')).toBe(0.11);
159164
expect(new NumberParser('en-US', {style: 'percent', minimumFractionDigits: 2}).parse('10.5%')).toBe(0.105);
160165
});
161166
});
167+
168+
describe('round trips', function () {
169+
// Locales have to include: 'de-DE', 'ar-EG', 'fr-FR' and possibly others
170+
// But for the moment they are not properly supported
171+
const localesArb = fc.constantFrom(...locales);
172+
const styleOptsArb = fc.oneof(
173+
{withCrossShrink: true},
174+
fc.record({style: fc.constant('decimal')}),
175+
// 'percent' should be part of the possible options, but for the moment it fails for some tests
176+
fc.record({style: fc.constant('percent')}),
177+
fc.record(
178+
{style: fc.constant('currency'), currency: fc.constantFrom('USD', 'EUR', 'CNY', 'JPY'), currencyDisplay: fc.constantFrom('symbol', 'code', 'name')},
179+
{requiredKeys: ['style', 'currency']}
180+
),
181+
fc.record(
182+
{style: fc.constant('unit'), unit: fc.constantFrom('inch', 'liter', 'kilometer-per-hour')},
183+
{requiredKeys: ['style', 'unit']}
184+
)
185+
);
186+
const genericOptsArb = fc.record({
187+
localeMatcher: fc.constantFrom('best fit', 'lookup'),
188+
unitDisplay: fc.constantFrom('narrow', 'short', 'long'),
189+
useGrouping: fc.boolean(),
190+
minimumIntegerDigits: fc.integer({min: 1, max: 21}),
191+
minimumFractionDigits: fc.integer({min: 0, max: 20}),
192+
maximumFractionDigits: fc.integer({min: 0, max: 20}),
193+
minimumSignificantDigits: fc.integer({min: 1, max: 21}),
194+
maximumSignificantDigits: fc.integer({min: 1, max: 21})
195+
}, {requiredKeys: []});
196+
197+
// We restricted the set of possible values to avoid unwanted overflows to infinity and underflows to zero
198+
// and stay in the domain of legit values.
199+
const DOUBLE_MIN = Number.EPSILON;
200+
const valueArb = fc.tuple(
201+
fc.constantFrom(1, -1),
202+
fc.double({next: true, noNaN: true, min: DOUBLE_MIN, max: 1 / DOUBLE_MIN})
203+
).map(([sign, value]) => sign * value);
204+
205+
const inputsArb = fc.tuple(valueArb, localesArb, styleOptsArb, genericOptsArb)
206+
.map(([d, locale, styleOpts, genericOpts]) => ({d, opts: {...styleOpts, ...genericOpts}, locale}))
207+
.filter(({opts}) => opts.minimumFractionDigits === undefined || opts.maximumFractionDigits === undefined || opts.minimumFractionDigits <= opts.maximumFractionDigits)
208+
.filter(({opts}) => opts.minimumSignificantDigits === undefined || opts.maximumSignificantDigits === undefined || opts.minimumSignificantDigits <= opts.maximumSignificantDigits)
209+
.map(({d, opts, locale}) => {
210+
if (opts.style === 'percent') {
211+
opts.minimumFractionDigits = opts.minimumFractionDigits > 18 ? 18 : opts.minimumFractionDigits;
212+
opts.maximumFractionDigits = opts.maximumFractionDigits > 18 ? 18 : opts.maximumFractionDigits;
213+
}
214+
return {d, opts, locale};
215+
})
216+
.map(({d, opts, locale}) => {
217+
let adjustedNumberForFractions = d;
218+
if (Math.abs(d) < 1 && opts.minimumFractionDigits && opts.minimumFractionDigits > 1) {
219+
adjustedNumberForFractions = d * (10 ** (opts.minimumFractionDigits || 2));
220+
} else if (Math.abs(d) > 1 && opts.minimumFractionDigits && opts.minimumFractionDigits > 1) {
221+
adjustedNumberForFractions = d / (10 ** (opts.minimumFractionDigits || 2));
222+
}
223+
return {adjustedNumberForFractions, opts, locale};
224+
});
225+
226+
// skipping until we can reliably run it, until then, it's good to run manually
227+
// track counter examples below
228+
it.skip('should fully reverse NumberFormat', function () {
229+
fc.assert(
230+
fc.property(
231+
inputsArb,
232+
function ({adjustedNumberForFractions, locale, opts}) {
233+
const formatter = new Intl.NumberFormat(locale, opts);
234+
const parser = new NumberParser(locale, opts);
235+
236+
const formattedOnce = formatter.format(adjustedNumberForFractions);
237+
expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce);
238+
}
239+
)
240+
);
241+
});
242+
});
243+
describe('counter examples', () => {
244+
it('can still get all plural literals with minimum significant digits', () => {
245+
let locale = 'pl-PL';
246+
let options = {
247+
style: 'unit',
248+
unit: 'inch',
249+
minimumSignificantDigits: 4,
250+
maximumSignificantDigits: 6
251+
};
252+
const formatter = new Intl.NumberFormat(locale, options);
253+
const parser = new NumberParser(locale, options);
254+
255+
const formattedOnce = formatter.format(60048.95);
256+
expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce);
257+
});
258+
// See Bug https://github.com/nodejs/node/issues/49919
259+
it.skip('formatted units keep their number', () => {
260+
let locale = 'da-DK';
261+
let options = {
262+
style: 'unit',
263+
unit: 'kilometer-per-hour',
264+
unitDisplay: 'long',
265+
minimumSignificantDigits: 1
266+
};
267+
const formatter = new Intl.NumberFormat(locale, options);
268+
const parser = new NumberParser(locale, options);
269+
270+
const formattedOnce = formatter.format(1);
271+
expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce);
272+
});
273+
});
162274
});
163275

164276
describe('isValidPartialNumber', function () {

0 commit comments

Comments
 (0)