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

Implement type narrowing for optional chaining operator on unions #56440

Closed
6 tasks done
meirg opened this issue Nov 17, 2023 · 7 comments
Closed
6 tasks done

Implement type narrowing for optional chaining operator on unions #56440

meirg opened this issue Nov 17, 2023 · 7 comments
Labels
Duplicate An existing issue was already created

Comments

@meirg
Copy link

meirg commented Nov 17, 2023

πŸ” Search Terms

"type narrowing", "optional chaining operator", "unions"

βœ… Viability Checklist

⭐ Suggestion

Infer a union object's type following the optional chaining operator

πŸ“ƒ Motivating Example

Say I have the result of an API call that returns some result or an error like this:

const res: {
  someResult: {
    foo: 'bar'
  }
} | { error: string } = await getApiResponse();

If I want to use the value of foo if it exists, it would be quicker to use optional chaining rather than an if statement to determine the type of response. Eg.
const foo = res.someResult?.foo ?? 'no result';
as opposed to

let foo: string;
if('someResult' in res)
  foo = res.someResult.foo;
else
  foo = 'no result';

or
const foo = 'someResult' in res ? res.someResult.foo : 'no result';

πŸ’» Use Cases

  1. What do you want to use this for? Writing code quicker
  2. What shortcomings exist with current approaches? They are more wordy
  3. What workarounds are you using in the meantime? Either the 'in' operator and re-typing the variable name or just adding ' | any' to the type union if I'm in a hurry

Further examples:
https://stackoverflow.com/questions/58974640/typescript-property-does-not-exist-on-union-type#comment120750095_58974714

@jcalz
Copy link
Contributor

jcalz commented Nov 17, 2023

That's just a union, not a discriminated union. The suggestion to allow optional chaining to narrow unions has been raised before and declined: #33736, #56264, etc.

@fatcerberus
Copy link

fatcerberus commented Nov 17, 2023

Want to see something fun?

// not a type error:
function doTheThing(): { someResult: { foo: 'bar' } } | { error: string } {
  const err = { error: "fooey", someResult: { foo: 42 } };
  return err;
}

// prints a number, not a string.
// worse, it’s not properly recognized as an error!
const res = doTheThing();
console.log('someResult' in res ? res.someResult.foo : `err: ${res.error}`);

Types are not exact (see #12936) and in checks are already unsound for that very reason. I doubt the TS devs will want to add an even easier way to fire this footgun.

@meirg meirg changed the title Implement type narrowing for optional chaining operator on discriminated unions Implement type narrowing for optional chaining operator on unions Nov 17, 2023
@meirg
Copy link
Author

meirg commented Nov 17, 2023

That's just a union, not a discriminated union.

Ah, thanks, I corrected that.

The suggestion to allow optional chaining to narrow unions has been raised before and declined: #33736, #56264, etc.

That is unfortunate. I didn't think to look in closed issues as this seems equivalent to using 'in', just shorter. Perhaps they will reconsider

Want to see something fun?

// not a type error:
const err = { error: "fooey", someResult: { foo: 42 } };
const res: { someResult: { foo: 'bar' } } | { error: string } = err;

// prints a number, not a string.
// worse, it’s not properly recognized as an error!
console.log('someResult' in res ? res.someResult.foo : `err: ${res.error}`);

Types are not exact (see #12936) and in checks are already unsound for that very reason. I doubt the TS devs will want to add an even easier way to fire this footgun.

Even so, it's no worse than 'in', just quicker. Coders need to account for the various scenarios either way.

@fatcerberus
Copy link

It's indeed no worse than in in terms of potential damage, but the added convenience makes it more likely someone will write such code without thinking (people have already posted bugs about in being unsound, not realizing the unsoundness is intentional). It's better if you have to take the safety off the footgun first :)

@fatcerberus
Copy link

I didn't think to look in closed issues as this seems equivalent to using 'in', just shorter. Perhaps they will reconsider

That seems unlikely. See #33736 (comment), in particular the last paragraph where the equivalence to in is openly acknowledged:

You could argue this is also true for the in operator--and you would be right. See discussion in #10485. But this doesn't change the fact that we don't want this behavior for the very common operation of property access using . or ?..

@jcalz
Copy link
Contributor

jcalz commented Nov 17, 2023

Perhaps they will reconsider

It's hard to imagine anything has changed since #56264 was filed less than a month ago.

@andrewbranch andrewbranch added the Duplicate An existing issue was already created label Nov 17, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants