Repository navigation
Inaccurate type for return value of Intl.RelativeTimeFormat.prototype.formatToParts #46245
Description
Bug Report
The types for Intl.RelativeTimeFormat.prototype.formatToParts in lib.e2020.intl.d.ts have a few issues.
-
According to the spec (and MDN docs), either singular or plural names for units are accepted as input, but only singular names are output. The current types assume that plural names can also be output, which is incorrect. The relevant step in the spec is Step 5 in https://tc39.es/ecma402/#sec-PartitionRelativeTimePattern, where plural inputs are converted to singular before being used in the output.
-
Also, there's a missed opportunity here to narrow the return type of this method, because for any input unit, the return unit will be the same unit, unless it's a plural in which case it's the plural unit converted to singular. (See https://tc39.es/ecma402/#sec-makepartslist.) So the return type could be a generic where
unitin the output depends onunitin the input. -
Finally, there's another missed opportunity to narrow the return type using the
typeproperty, because according to https://tc39.es/ecma402/#sec-FormatRelativeTimeToParts and https://tc39.es/ecma402/#sec-makepartslist, iftype === 'literal'thenunitwill be omitted, while iftypeis any other value, thenunitwill be present in the output.
I'd be happy to submit a PR to change these types. (If a PR would be welcome.)
Here's excerpts from the current relevant types in lib.e2020.intl.d.ts:
TypeScript/lib/lib.es2020.intl.d.ts
Lines 114 to 118 in cb158e7
| interface RelativeTimeFormatPart { | |
| type: string; | |
| value: string; | |
| unit?: RelativeTimeFormatUnit; | |
| } |
TypeScript/lib/lib.es2020.intl.d.ts
Lines 183 to 186 in cb158e7
| formatToParts( | |
| value: number, | |
| unit: RelativeTimeFormatUnit, | |
| ): RelativeTimeFormatPart[]; |
TypeScript/lib/lib.es2020.intl.d.ts
Lines 38 to 47 in cb158e7
| type RelativeTimeFormatUnit = | |
| | "year" | "years" | |
| | "quarter" | "quarters" | |
| | "month" | "months" | |
| | "week" | "weeks" | |
| | "day" | "days" | |
| | "hour" | "hours" | |
| | "minute" | "minutes" | |
| | "second" | "seconds" | |
| ; |
🔎 Search Terms
Intl
RelativeTimeFormat
formatToParts
plural
singular
🕗 Version & Regression Information
Introduced in #36084.
⏯ Playground Link
💻 Code
const rtf = new Intl.RelativeTimeFormat();
const parts = rtf.formatToParts(42, 'minutes');
// ResultUnit is the actual type returned per the ECMA-402 spec
type ResultUnit = "year" | "quarter" | "month" | "week" | "day" | "hour" | "minute" | "second";
const partsWithUnit = parts.filter(p => p.unit);
const unit: ResultUnit = partsWithUnit[0].unit!;
// ^^^^
// Type 'RelativeTimeFormatUnit' is not assignable to type 'ResultUnit'.
// Type '"years"' is not assignable to type 'ResultUnit'.(2322)🙁 Actual behavior
TS error 2322
🙂 Expected behavior
No error because the unit of the return type is always singular.
Activity
orta commentedon Oct 8, 2021
Cool, happy to have
SingularRelativeTimeUnitadded and switched out where needed 👍🏻I'm not too wild on making the functions generic to support some of the narrowing, we really want to keep the shipped libs as simple as possible because they get included in everyone's projects and historically a lot of the times we've been clever we've had to backtrack
justingrant commentedon Oct 11, 2021
OK, I'll send a PR soon.
I also noticed another Intl type-declaration issue: the input properties to the
optionsparam of theIntl.DateTimeFormatconstructor are narrowly typed string literals, e.g.localeMatcher?: "best fit" | "lookup".But the
ResolvedDateTimeFormatOptionstype, which should represent the same range of string literals, is typed more loosely asstring. Any idea why this is? Do TS lib types generally restrict inputs but are more forgiving for outputs in order to support later platforms that can output values that old TS lib declarations don't know about? Or is this a bug and they should match?Here's the types for reference:
TypeScript/src/lib/es5.d.ts
Lines 4392 to 4406 in 5d0d7ae
TypeScript/src/lib/es5.d.ts
Lines 4408 to 4423 in 5d0d7ae
OK, sounds reasonable, I won't include (2) in the PR. What about (3) above which is a discriminated union not a generic?
justingrant commentedon Oct 11, 2021
FWIW, newer updates to Intl are inconsistent with older libs' types. Older libs (excerpted in my earlier comment) use
string, but newer updates use string literal types forResolvedDateTimeFormatOptions.TypeScript/src/lib/es2021.intl.d.ts
Lines 3 to 18 in 61d2939
Any idea why they are different? Should they be different?
orta commentedon Oct 12, 2021
Generally we want to use a union of string literals when it makes sense which is most of the time, but it's can be hard to do that (which means falling back to
string.) For example if a new option for hour cycle is added ines2022we don't have a way in TypeScript to safely describe that it ishourCycle?: ES2011hourCycle | "h25"when you are in es2022.As seen in something like
TypeScript/src/lib/es2018.intl.d.ts
Lines 41 to 44 in 61d2939
Which is both correct... and incorrect.
The other is that these are contributions from a range of people, and while I gave them all big look over for 4.5, that doesn't make them all perfectly consistent and accurate (because I've barely used these APIs in production) - so we're OK to back breaking changes like
string->"h11" | "h12" | "h23" | "h24"so long as it has some history of been in for a while and isn't looking to drastically change in the futurejustingrant commentedon Oct 12, 2021
Makes sense. Would you welcome a PR to change
ResolvedDateTimeFormatOptionsto string literals? Or should it be left as onlystringproperties?Could you explain the problem a little more? Not sure I fully understand it. Thanks!
fix: update types for RTF.p.formatToParts() result
fix: update types for RTF.p.formatToParts() result
fix: update types for RTF.p.formatToParts() result (#46508)