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

Enable "exactOptionalPropertyTypes" for Typescript by default #11889

Closed
Tracked by #25162
andykais opened this issue Aug 31, 2021 · 8 comments
Closed
Tracked by #25162

Enable "exactOptionalPropertyTypes" for Typescript by default #11889

andykais opened this issue Aug 31, 2021 · 8 comments
Labels
suggestion suggestions for new features (yet to be agreed) tsc related to the TypeScript tsc compiler

Comments

@andykais
Copy link

In TypeScript 4.4, the option "exactOptionalPropertyTypes" was introduced https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#exact-optional-property-types. Now that #11678 is merged, I petition that we add this compiler flag. It ensures that optional field values

interface Foo {
  bar: string
  baz?: string
}

cannot be assigned as undefined. E.g.

// with exactOptionalPropertyTypes enabled this is invalid
const foo1: { bar: '', baz: undefined }

This is a good change for catching subtle errors with Object.key, object spreading, etc. There is likely code that is broken with this change though. I would say if this feels like a valuable flag to add, the sooner its added the less code that will be broken. I can of course still be disabled with a tsconfig.json for projects that rely on this behavior, but I imagine that will be a select fe

@andykais andykais changed the title Enable "exactOptionalPropertyTypes" for Typescript by default Enable "exactOptionalPropertyTypes" for Typescript by default Aug 31, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Aug 31, 2021

I think like the other "pendantic" flags, we shouldn't enable them. They will cause a lot of existing code to break. If they aren't part of strict, I don't feel we should enable them. There is good reason it isn't part of strict:

This flag is not part of the --strict family and needs to be turned on explicitly if you’d like this behavior. It also requires --strictNullChecks to be enabled as well. We’ll be making updates to DefinitelyTyped and other definitions to try to make the transition as straightforward as possible, but you may encounter some friction with this depending on how your code is structured.

@kitsonk kitsonk added suggestion suggestions for new features (yet to be agreed) tsc related to the TypeScript tsc compiler labels Aug 31, 2021
@andykais
Copy link
Author

andykais commented Sep 1, 2021

Fair enough. It sounds like theres already an established answer for "pedantic" flags. Should I close this issue?

@kitsonk
Copy link
Contributor

kitsonk commented Sep 1, 2021

Maybe someone else has an argument otherwise. No problem keeping it open for a while.

@Jamesernator
Copy link

This goes beyond just this particular option, but it would be nice if Deno supported applying options to only "own" code. i.e. I tend to prefer using noUncheckedIndexedAccess as when I've added it to a number of my codebases, the majority of errors produced were actual unidentified bugs.

But enabling these sort've options on library code tends to explode as these options aren't in super high usage, so libraries often don't consider them.

It would be a significant change, but if Deno did actually go for the most-strict options on all of these, then library authors would be encouraged to write code that is as correct as possible. Users wanting a less restrictive configuration wouldn't have to worry about breaking libraries as code working under these strict flags I'm pretty sure is always meant to compatible with the option being off.

@lucacasonato
Copy link
Member

We are not going to enable this by default. See #25162 for discussion.

@petamoriken
Copy link
Contributor

@lucacasonato @bartlomieju I made a big modification to enable this option in deno_std (denoland/std#5892), but in general users should only need to add | undefined if the type has an optional property e.g. jsr-core/unknownutil#132.

 interface Foo {
-  bar?: string;
+  bar?: string | undefined;
 }

It is recommended in the TSConfig documentation that this option be enabled. Could you please reconsider this?

@bartlomieju
Copy link
Member

@petamoriken I discussed this with @lucacasonato and we're worried that it will actually break a lot of existing code. We're not too keen on doing that at this point.

@petamoriken
Copy link
Contributor

petamoriken commented Sep 19, 2024

@bartlomieju Thank you for discussing this. I respect the decision!

The reason why this option is not included in the "strict" family, even though TypeScript recommends it, may be because we don't know how big a breaking change this option would be...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed) tsc related to the TypeScript tsc compiler
Projects
None yet
Development

No branches or pull requests

6 participants