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

Destructuring function param doesn't work with union types #5461

Closed
atabel opened this issue Dec 4, 2017 · 1 comment
Closed

Destructuring function param doesn't work with union types #5461

atabel opened this issue Dec 4, 2017 · 1 comment

Comments

@atabel
Copy link
Contributor

atabel commented Dec 4, 2017

Here is an example with a redux reducer:
This code:

type FooAction = { type: "FOO", payload: Object };
type BarAction = { type: "BAR", payload: Object };

type Action = FooAction | BarAction;

function reducer(state, { type, payload }: Action) {
  switch (type) {
    case "FOO": return { ...state, value: payload };
    case "BAR": return { ...state, value: payload };
    default:
      return state;
  }
}

Produces this error:

6: function reducer(state, { type, payload }: Action) {
                             ^ string literal `BAR`. Expected string literal `FOO`, got `BAR` instead
6: function reducer(state, { type, payload }: Action) {
                             ^ string literal `FOO`
6: function reducer(state, { type, payload }: Action) {
                             ^ string literal `FOO`. Expected string literal `BAR`, got `FOO` instead
6: function reducer(state, { type, payload }: Action) {
                             ^ string literal `BAR`
8:     case "FOO": return { ...state, value: payload };
            ^ string literal `FOO`. This === check always fails because `FOO` is not the same string as `BAR`
6: function reducer(state, { type, payload }: Action) {
                             ^ string literal `BAR`

see in Try Flow

But, if I move destructuring to the function body instead of function declaration:

function reducer(state, action: Action) {
  const { type, payload } = action
  switch (type) {
    case "FOO": return { ...state, value: payload };
    case "BAR": return { ...state, value: payload };
    default:
      return state;
  }
}
no errors!

see in Try FLow

@kristiehowboutdat
Copy link

kristiehowboutdat commented Jun 1, 2018

Here is another example using Flow 0.73.0: Try Flow

Filed #6408

facebook-github-bot pushed a commit that referenced this issue Jul 15, 2019
Summary:
Bindings introduced by destructuring a type annotation should behave as
annotations themselves. That is, `p` in `var {p}: {p: T}` should constrain any
subsequent write to be a subtype of `T`.

This logic works today by wrapping the binding in an AnnotT using BecomeT
internally, which works for most cases. However, since BecomeT performs
unification, it is necessary that its lower bound be resolve once and exactly
once.

The once-and-exactly-once invariant is broken by union-like types when combined
with the destructuring operation. Consider the following example:

```
var {p}: {p:string}|{p:number} = ...
```

When evaluating this variable declaration, we emit the following constraints:

1. {p:string}|{p:number} -> GetPropT ('p', T)
2. T -> BecomeT T'
3. P = T'

Constraint (1) is processed by splitting the lower bound, which turns into two
separate constraints `string -> T` and `number -> T`. Since `T` resolves twice,
the `BecomeT` constraint misbehaves and we see a spurious error that `number` is
not compatible with `string`.

This diff deals with the above by handling union-like types specially in the
`DestructuringT` use type.

Fixes #183
Fixes #2198
Fixes #2220
Fixes #4077
Fixes #4270
Fixes #5461
Fixes #5745
Fixes #6408

Reviewed By: panagosg7

Differential Revision: D15457398

fbshipit-source-id: 22c9aba1e1df475c73b36a92bdf7ff5cf57504a6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants