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 types: Better typing for exactOptionalPropertyTypes needed #45652

Closed
HolgerJeromin opened this issue Aug 31, 2021 · 9 comments
Closed

Intl types: Better typing for exactOptionalPropertyTypes needed #45652

HolgerJeromin opened this issue Aug 31, 2021 · 9 comments
Assignees
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript

Comments

@HolgerJeromin
Copy link
Contributor

Bug Report

I did not find a "planning" issue for related issues.
All delivered types should be safe to use with the new Typescript option.

🕗 Version & Regression Information

  • I was unable to test this on prior versions because this is a new option

⏯ Playground Link

// @exactOptionalPropertyTypes
const IntlOptions: Intl.DateTimeFormatOptions = {
    timeZone: undefined
};

Workbench Repro

🙁 Actual behavior

Type 'undefined' is not assignable to type 'string'.(2322)

🙂 Expected behavior

Should compile because undefined is a valid property for that object.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/DateTimeFormat

copied from microsoft/TypeScript-DOM-lib-generator#1126

@orta
Copy link
Contributor

orta commented Aug 31, 2021

I'm half-half on this, undefined is the default for those values, yes, but it's not documented as something you should really pass in via that MDN page - can you think of a specific reason why we should add this?

@HolgerJeromin
Copy link
Contributor Author

HolgerJeromin commented Aug 31, 2021

My real world code is something like that:

let timeZone: string | undefined = getUserTimeZone();
let IntlOptions: Intl.DateTimeFormatOptions = {
     timeZone: timeZone
};

refactoring to

let timeZone: string | undefined = getUserTimeZone();
let IntlOptions: Intl.DateTimeFormatOptions = {};
if (timeZone) {
    IntlOptions.timeZone = timeZone;
}

to make TS happy feels clumsy.
My API returns "use client setting" explicit as undefined (for timezone and locale) to allow direct use in the modern browser APIs.

@orta
Copy link
Contributor

orta commented Aug 31, 2021

Yeah, that seems reasonable to me

orta added a commit to orta/TypeScript that referenced this issue Aug 31, 2021
@orta orta added the Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript label Aug 31, 2021
@orta orta self-assigned this Aug 31, 2021
@tolysz
Copy link

tolysz commented Aug 31, 2021

I would love if the setting of undefined would simply delete the key in the object...i.e. set it to the default of timeZone?

@nmain
Copy link

nmain commented Aug 31, 2021

I would love if the setting of undefined would simply delete the key in the object...i.e. set it to the default of timeZone?

Seems like you just want exactOptionalPropertyTypes: false then.

@tolysz
Copy link

tolysz commented Sep 1, 2021

I would love if the setting of undefined would simply delete the key in the object...i.e. set it to the default of timeZone?

Seems like you just want exactOptionalPropertyTypes: false then.

Is it? I was under the impression that 2 != 3? (But it is more about category then arithmetics)

when: exactOptionalPropertyTypes: false

  1. timeZone can be not set
  2. timeZone: can be string
  3. timeZone can be undefined

exactOptionalPropertyTypes: true

  1. timeZone can be not set, but you can not get here back by yourself.
  2. timeZone: can be string
    3) timeZone can be undefined

I am asking for a 3rd way: where you can unset the value or it holds value of specific type... so we can still use the ? optional bit on the type but in this stricter way...
We might need some other annotation like it can not be unset maybe a +?

@orta
Copy link
Contributor

orta commented Sep 1, 2021

Maybe I'm also confused, but point is seems to be what this issue is about. The change would allow all three options by adding | undefined in the .d.ts which you can see in orta@689bf6f

@fatcerberus
Copy link

fatcerberus commented Sep 2, 2021

@tolysz

// exactOptionalPropertyTypes: true
foo.optional = undefined;  // error
delete foo.optional;  // all good

orta pushed a commit that referenced this issue Sep 8, 2021
* Import of Intl.Locale from #39664

* Handle updating es2020.intl and add es2021 for new DateTimeFormatOptions options - re: #39664

* Extends DateTimeFormatOptions for new Intl APIs - re: #45420

* Handle migrating Intl.NumberFormat.formatToParts to es2018 (keeping esnext.intl around)

* Adds Intl.DisplayNames to es2020 - re: #44022

* Remove attributes added in es2021 from es2020 - re: #42944

* Add a reference to es2021 in the command line parser

* Adds some docs about the lib files

* Baselines

* Allow undefined in Intl inputs to allow for ergonomic usage of exactOptionalPropertyTypes - see #45652

* Adds some tests covering the APIs

* Apply suggestions from code review

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>

* Handle PR feedback

* More review improvements

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
@orta
Copy link
Contributor

orta commented Sep 8, 2021

This is included with #45647 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants