intl.d.ts: cleanup, add missing features, fix library discrepancies #58084
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- narrow resolved option types - progressively type DateTimeFormatOptionsTimeZoneName - hourCycle comes under es2018 - dayPeriod, dateStyle, timeStyle come under es2021 - remove duplicates NB. resolved timeZone option may be undefined in es5, but will be defaulted from es2015 onwards
- overriding collation to es2021 - resolved options caseFirst and numeric may be omitted (implementation-dependent)
- ECMA-402 Array.prototype.toLocaleString override added to spec in v2 (2015)
- ECMA-402 String.prototype.toLocale{Lower,Upper}Case overrides added to spec in v2 (2015)
- Deduplicated NumberFormat option declarations in es2020.bigint
- Updated docblocks for many override methods
- Updated docblocks for native (parameter-less) toLocaleString methods
- clean up supportedLocalesOf()
- add NumberFormat V3 options to es2023
- resolved {minimum,maximum}FractionDigits may be omitted (as of es2023; mirrors ResolvedNumberFormatOptions)
- Locale no longer extends LocaleOptions but is defined separately - Locale properties may be undefined, but are never absent - LocaleOptions properties may be undefined - Create a LocaleConstructor interface for consistency
Doesn't add clarity, and gets optimised away by TS, so the associated docblock is not visible.
- Move es2015 primitive prototype toLocaleString overrides to es2015.core - Add ReadonlyArray overrides - Update TypedArray.prototype.toLocaleString overrides - Array.prototype.toLocaleString `options` type should not be narrowed
- Add missing numberingSystem option - Correctly type RelativeTimeFormatOptions - Deduplicate RelativeTimeFormatUnit - Add constructor interface
- move definition to es2021.intl - rename option types - separate es2022 changes, including union registry for DisplayNamesOptionsType - correctly type DisplayNamesOptions and ResolvedDisplayNamesOptions - add constructor interface
- rename option types - ListFormatPart interface - add constructor interface
- add union types - Segments#containing() may return undefined - add constructor interface
- convert remaining uses of `string | string[]` to `string | readonly string[]` for consistency - `locales` argument to IntlChildConstructor.supportedLocalesOf() is optional
Move tests for DisplayNames, supportedValuesOf Remove duplicate DisplayNamesOptionsLanguageDisplay definition
Renegade334
commented
Jun 11, 2024
Comment on lines
+53
to
+58
| interface DisplayNamesOptionsTypeRegistry { | ||
| language: "language"; | ||
| region: "region"; | ||
| script: "script"; | ||
| currency: "currency"; | ||
| } |
There was a problem hiding this comment.
- es2021: language, region, script, currency
- es2022: calendar, dateTimeField
Comment on lines
+4540
to
+4543
| interface DateTimeFormatOptionsTimeZoneNameRegistry { | ||
| short: "short"; | ||
| long: "long"; | ||
| } |
There was a problem hiding this comment.
- es5: short, long
- es2022: shortOffset, longOffset, shortGeneric, longGeneric
|
@typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: slate Package: react-native-sortable-list Package: pdfmake Package: x-ray Package: react-native-autocomplete-input Package: react-native-signature-capture Package: react-native-text-input-mask Package: tuya-panel-kit Package: react-native-modalbox Package: react-native-table-component Package: d3-quadtree/v2 Package: lodash/v3 Package: d3-quadtree Package: lodash Package: loadware Package: mergerino Package: mithril Package: react-native-htmlview Package: react-native-calendars Package: react-native-ad-manager Package: react-native-material-ripple Package: react-primitives Package: react-native-button Package: d3-quadtree/v1 |
|
@jakebailey Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…ype" This reverts commit 31d8a82.
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
Renegade334
commented
Jun 11, 2024
| * @param locales Passed as the `locales` parameter to each array element's `toLocaleString` method. | ||
| * @param options Passed as the `options` parameter to each array element's `toLocaleString` method. | ||
| */ | ||
| toLocaleString(locales?: string | readonly string[], options?: object): string; |
There was a problem hiding this comment.
See notes towards the bottom of the PR description for discussion regarding the type of options here.
I'm ambivalent towards the change, but thought it would be worth proposing.
|
@typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
perf
top400
|
We definitely do not want to be deleting types from lib.d.ts. |
|
Keep as deprecated exports, then? |
|
Yes, or keep using (if it makes sense). |
sandersn
requested review from
andrewbranch and
sandersn
and removed request for
andrewbranch
Renegade334
commented
Jun 13, 2024
| * | ||
| * See [MDN - Intl - locales argument](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl#locales_argument). | ||
| */ | ||
| type UnicodeBCP47LocaleIdentifier = string; |
There was a problem hiding this comment.
review: either revert or add deprecated type export.
While this type doesn't do any harm, primitive aliasing feels like a paradigm that should either be used everywhere in the library, or not used at all. This is the only case of it throughout lib.d.ts.
A very similar change was made to the library previously (https://github.com/microsoft/TypeScript/pull/52996/files#r1408503669) and didn't seem to cause too much consternation.
|
I don't know if this is related, but since TypeScript 5.5, we have some issue with changes in Number.ToLocaleString(). // Defined once to avoid copy pasting this everywhere.
const percentOptions = {
minimumFractionDigits: 2,
maximumFractionDigits: 2,
style: "percent",
};
let output1 = number1.toLocaleString(undefined, percentOptions);
let output2 = number2.toLocaleString(undefined, percentOptions);
let output3 = number3.toLocaleString(undefined, percentOptions);Typescript will shout about this, because "percent" is narrowed to string instead of the subset of valid style values. const percentOptions = {
minimumFractionDigits: 2,
maximumFractionDigits: 2,
style: "percent" as keyof Intl.NumberFormatOptionsStyleRegistry,
};Sorry if this is the wrong place to mention this, but I wasn't sure if opening a new issue was the way to go. |
|
@wartab: This was intentionally changed previously to make the enum-style string option types consistent. For your example, using |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recent work on the Intl library definitions revealed a lot of outstanding inaccuracies and inconsistencies, and so I've taken a deep dive through ECMA-402 and done some significant housekeeping.
This is a big PR, and I've itemised the changes for ease of review. The majority are straightforward and uncontroversial updates to bring things in line with the spec, and it seems sensible just to have them as one PR.
A couple of proposed changes have breaking potential or are related to the typing meta, and are marked 🚩 to indicate that they may need discussion.
Library-wide
Typings
{ConstructorName}Options{PropertyName}naming convention. This practice started to be used here and there in later lib.intl updates, but is now consistent throughout.keyof R, which works fine for validation, but isn't very helpful in tsserver output.type Intl.NumberFormatOptionsStyle = keyof NumberFormatOptionsStyleRegistry– so what are the valid options?R[keyof R], which ensures that they're resolved to a union type.type Intl.NumberFormatOptionsStyle = "decimal" | "percent" | "currency"– much more accessible.WeakKeyetc.){ConstructorName}{PropertyName}to{ConstructorName}Options{PropertyName}. These are exported types, so in theory this is breaking if anyone is importing those specific union types from the library; however, the maintenance and DX benefits of a universal namespace convention seem to support this change. (There are no resulting errors in the user test suite.)UnicodeBCP47LanguageTag, which is simply a primitive type alias tostring. This gets optimised away by the compiler, and serves no useful purpose; there are no other examples of "primitive alias as usage descriptor" elsewhere in the library.Documentation
Tests
Intl
Intl.supportedValuesOffrom es2022 to es2023.Intl.getCanonicalLocalesto es2020.prototypeproperties of Intl constructors.Intl.Collator
Intl.supportedValuesOf(), if runtime validation is desired.collationoption from es5 to es2021.caseFirstandnumericonly exist if they are supported by the implementation and are absent if not, hence these are typed as optional.Intl.DateTimeFormat
bigintfrom argument types in format functions, as explicitly not supported.ResolvedDateTimeFormatOptionsproperty types in es5.hourCycleoption from es2020 to es2018.dayPeriod,dateStyle,timeStyleoptions from es2020 to es2021.timeZoneName, with values{short,long}Offsetand{short,long}Genericmoved to es2022.relatedYearandyearNameto es2020.Intl.DisplayNames
DisplayNamesOptionsproperties may beundefined.DisplayNamesOptionsType, with valuescalendaranddateTimeFieldmoved to es2022.languageDisplayoption to es2022.DisplayNameConstructorinterface.Intl.ListFormat
ListFormatPartinterface.ListFormat{PropertyName}toListFormatOptions{PropertyName}.ListFormatConstructorinterface.Intl.Locale
Localeprototype no longer being typed as a child interface ofLocaleOptions.LocaleConstructorinterface.LocaleOptionsproperties may beundefined.Localeproperties as required/readonly.caseFirstandnumericonly exist if they are supported by the implementation and are absent if not, hence these are typed as optional.Intl.NumberFormat
bigintfrom the original definition of theformatToPartsmethod in es2018 (prior to the language feature's existence), and added a new override to es2020.bigint.Intl.PluralRules
selectRangemethod to es2023.ResolvedPluralRulesOptions,{minimum,maximum}FractionDigitsmay be omitted from es2023 onwards, and these are therefore typed as optional throughout. This mirrors the equivalent change toResolvedNumberFormatOptionsin Intl.NumberFormat: Add latest options, fix previous library discrepancies #56902.new.Intl.RelativeTimeFormat
numberingSystemoption.RelativeTimeFormat{PropertyName}toRelativeTimeFormatOptions{PropertyName}.Intl.Segmenter
Segments#containingmay returnundefinedif the specified index does not exist.SegmenterConstructorinterface.Locale methods of primitive prototypes
Array.prototype.toLocaleStringandTypedArray.prototype.toLocaleStringunder es2020.array, which were omitted from Add missing parameters from Array.toLocaleString on ES2015 libs #57679.String.prototype.toLocale{Lower,Upper}Caseto es2015.core.localesargument to all first-generation overrides acceptsreadonly string[], as per Make Intl locales arrays readonly #56513.NumberFormatOptionsin the definition ofBigInt.prototype.toLocaleStringin es2020.bigint, and remove the duplicate NumberFormat definitions.toLocaleStringmethod ofBigInt64ArrayandBigUint64Arrayprototypes updated to the second-generation override signature.optionsparameter ofArray.prototype.toLocaleString.toLocaleStringwill be builtin number-like/date-like objects, whosetoLocaleStringmethod will pass its parameters to either NumberFormat or DateTimeFormat.Array.prototype.toLocaleStringdoesn't specify this; it will pass its parameters to any object'stoLocaleStringmethod, including user-defined methods. All that ECMA-262 asks is that those parameters not be used for anything other than the standardtoLocaleStringparameter pattern.toLocaleStringimplementations – providing they obey the standardised function signature.object. However, this comes at the expense of not type-checking any option properties, even for arrays of builtin elements which were covered just fine by the previous behaviour.T, but this leads to issues with infinite recursion, so doesn't look like it's an option.localesparameter of first- and second-generation overrides should be required or optional when an es5 placeholder already exists.