Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intl.DisplayNames options object type needs to be split #48218

Closed
tchetwin opened this issue Mar 11, 2022 · 2 comments · Fixed by #48262
Closed

Intl.DisplayNames options object type needs to be split #48218

tchetwin opened this issue Mar 11, 2022 · 2 comments · Fixed by #48262
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Milestone

Comments

@tchetwin
Copy link

Bug Report

🔎 Search Terms

DisplayNames, Intl, locale

🕗 Version & Regression Information

The behaviour was altered in TypeScript 4.6.2, but it was differently incorrect prior to that:

⏯ Playground Link

Playground link with relevant code

💻 Code

const displayNameOptions: Partial<Intl.DisplayNamesOptions> = {
    type: "language",
    // @ts-expect-error - this should *not* be valid. The Intl API seems to cope with excess properties
    locale: "en-GB",
};

const i = new Intl.DisplayNames(["tr"], displayNameOptions);

const resolvedOptions = i.resolvedOptions();
console.log(resolvedOptions); // prints an object containing `locale`, `style`, `type`, `fallback`, `languageDisplay`

// This should be valid
resolvedOptions.languageDisplay;

// @ts-expect-error - this should not be valid
const localMatcher = resolvedOptions.localeMatcher;
console.log(localMatcher); // prints `undefined`

🙁 Actual behavior

Type errors in the above code for valid patterns, no type errors for invalid patterns.

🙂 Expected behavior

Only valid type errors, accurate representation of the API

Notes

The DisplayNamesOptions interface:

interface DisplayNamesOptions {
locale: UnicodeBCP47LocaleIdentifier;
localeMatcher: RelativeTimeFormatLocaleMatcher;
style: RelativeTimeFormatStyle;
type: "language" | "region" | "script" | "currency";
fallback: "code" | "none";
}

...is currently shared by Intl.DisplayNames() constructor, and Intl.DisplayNames.prototype.resolvedOptions(). While the underlying objects share several properties, they are not a perfect match as the types suggest.

Per MDN:

ECMA-402 links:

Shoutout to @mkubilayk who spotted this during our TypeScript update!

Suggested Fix

These types are not compatible, suggesting they should be split into something like (bikeshed) DisplayNamesOptions and DisplayNamesResolvedOptions.

Probably a Good First Issue™

@tchetwin tchetwin changed the title Intl.DisplayName options object type needs to be split Intl.DisplayNames options object type needs to be split Mar 11, 2022
@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Mar 11, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 11, 2022
@tchetwin
Copy link
Author

CC: @gfyoung

gfyoung added a commit to forking-repos/TypeScript that referenced this issue Mar 14, 2022
@gfyoung
Copy link
Contributor

gfyoung commented Mar 14, 2022

Thanks @tchetwin for filing this: just opened a PR to update these.

gfyoung added a commit to forking-repos/TypeScript that referenced this issue Mar 20, 2022
gfyoung added a commit to forking-repos/TypeScript that referenced this issue Mar 20, 2022
gfyoung added a commit to forking-repos/TypeScript that referenced this issue Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants