tc39 / proposal-intl-numberformat-v3

Additional features for Intl.NumberFormat to solve key pain points.
MIT License
53 stars 12 forks source link

Can the order in which properties are read be more comprehensible? #122

Closed gibson042 closed 1 year ago

gibson042 commented 1 year ago

The current state in ECMA-402 is already confusing (neither alphabetical nor logical), but I'm concerned about v3 making things even worse:

 localeMatcher
 numberingSystem
 in SetNumberFormatUnitOptions:
   style
   currency
   currencyDisplay
   currencySign
   unit
   unitDisplay
+roundingIncrement
 notation
 in SetNumberFormatDigitOptions:
   minimumIntegerDigits
   minimumFractionDigits
   maximumFractionDigits
   minimumSignificantDigits
   maximumSignificantDigits
+  roundingPriority
+trailingZeroDisplay
 compactDisplay
 useGrouping
 signDisplay
+roundingMode

Likewise for Intl.NumberFormat.prototype.resolvedOptions, which similarly doesn't clearly map to anything:

 locale
 numberingSystem
 style
 currency
 currencyDisplay
 currencySign
 unit
 unitDisplay
 minimumIntegerDigits
 minimumFractionDigits
 maximumFractionDigits
 minimumSignificantDigits
 maximumSignificantDigits
 useGrouping
 notation
 compactDisplay
 signDisplay
+roundingMode
+roundingIncrement
+trailingZeroDisplay
sffc commented 1 year ago

It's logical in the sense that there are dependencies going on. roundingIncrement affects _mxfdDefault_, which is needed for determining the resolved value of maximumFractionDigits, which is combined with roundingPriority to determine the final rounding strategy. Then, the new options trailingZeroDisplay and roundingMode don't have any dependencies, so they go at the end. I guess one improvement would be to actually append TrailingZeroDisplay to the end so that the order reflects the order in which these options were proposed.

ptomato commented 1 year ago

For Temporal we took the design principle of reading properties in alphabetical order, validating their values immediately in cases where there are no dependencies, and in case of dependencies validating them once all prerequisite values have been read.

For example, GetDifferenceSettings:

ljharb commented 1 year ago

(it'd be nice to s/sanity/consistency in the issue title)

ljharb commented 1 year ago

Regarding the issue, why not unconditionally eagerly copy the entire options object into a new null object, so that any further accesses are not observable?

sffc commented 1 year ago

If this were a new spec, reading out everything in alphabetical order and the resolving it later sounds like the best approach. However, since this is pre-existing spec text, the best we can do with least disruption is to slot the property reads in with all the others that are already there.

Since this particular proposal is Stage 3 and nearing Stage 4, I'm inclined to put this aside and revisit it as a normative PR to change all 402 constructors to read properties in alphabetical order.

sffc commented 1 year ago

TG2 discussion: https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-01-12.md#can-the-order-in-which-properties-are-read-be-more-comprehensible

Conclusion: Revisit this across ECMA-402 and get alignment at that point. In the short term, there is not a change that can be made to NFv3 that is clearly better than the status quo when weighing the cost of change.

sffc commented 1 year ago

@gibson042 Would you mind opening an issue in the 402 repo for this?

gibson042 commented 1 year ago

@gibson042 Would you mind opening an issue in the 402 repo for this?

Done: https://github.com/tc39/ecma402/issues/747