Flow: [bug] Looses the type of a disjoint union inside an arrow function

Created on 22 Nov 2016 · 4Comments · Source: facebook/flow

First off, thanks for the amazing tool. Flow has been an incredible time-saver for me and I appreciate all the collective effort that went into it.

I'd like to report a bug that's best demonstrated by code:

Exhibit A:

This code is supposed to be working but it's not. I guess what might be happening is, the action type information somehow gets lost inside the map arrow function.

type User = { isFollowing: bool };

type Action = 
  { type: 'FOLLOW_USER', idx: number } | 
  { type: 'SOMETHING' };

function usersReducer(state: Array<User>, action: Action): Array<User> {
  switch (action.type) {
    case 'FOLLOW_USER': {
      const isFirst = action.idx === 0; // not giving an error
      const suggestions = state.map((person, idx) => {
        if (idx !== action.idx) return person; // BOOM
        return { ...person, isFollowing: !person.isFollowing };
      });
      return suggestions;
    }
    default: return state;
  }
}

12:         if (idx !== action.idx) return person; // BOOM
                               ^ property `idx`. Property not found in
12:         if (idx !== action.idx) return person; // BOOM
                        ^ object type

Exhibit B:

However, if I capture a field from action outside the arrow function, then use it in it, things seem to work.

type User = { isFollowing: bool };

type Action = 
  { type: 'FOLLOW_USER', idx: number } | 
  { type: 'SOMETHING' };

function usersReducer(state: Array<User>, action: Action): Array<User> {
  switch (action.type) {
    case 'FOLLOW_USER': {
      const isFirst = action.idx === 0; // not giving an error
      const actionIdx = action.idx; // capture the value
      const suggestions = state.map((person, idx) => {
        if (idx !== actionIdx) return person; // no longer booms
        return { ...person, isFollowing: !person.isFollowing };
      });
      return suggestions;
    }
    default: return state;
  }
}

No errors!

Exhibit C:

Finally, this demonstrates type information is not lost if Action is an object type, instead of being a disjoint union.

type User = { isFollowing: bool };

type Action = 
  { type: 'FOLLOW_USER', idx: number };

function usersReducer(state: Array<User>, action: Action): Array<User> {
  switch (action.type) {
    case 'FOLLOW_USER': {
      const isFirst = action.idx === 0; // not giving an error
      const suggestions = state.map((person, idx) => {
        if (idx !== action.idx) return person; // no longer booms
        return { ...person, isFollowing: !person.isFollowing };
      });
      return suggestions;
    }
    default: return state;
  }
}

No errors!

This is a bug because in the case A, there is enough information to conclude which branch of the union is being referred inside the case statement, and there are no new facts inside the arrow function that would change that.

not a bug refinements

Source

goshakkk

Most helpful comment

Another way to fix the error is to set the experimental.const_params=true option in the [options] section of .flowconfig

gcanti on 22 Nov 2016

👍3

All 4 comments

Narrowing on a variable is undone inside a closure that captures that variable because Flow can't prove that the variable won't change before the closure is called.

One workaround is to capture a const variable instead:

type User = { isFollowing: bool };

type Action = 
  { type: 'FOLLOW_USER', idx: number } | 
  { type: 'SOMETHING' };

function usersReducer(state: Array<User>, action_: Action): Array<User> {
  const action = action_;
  switch (action.type) {
    case 'FOLLOW_USER': {
      const isFirst = action.idx === 0; // not giving an error
      const suggestions = state.map((person, idx) => {
        if (idx !== action.idx) return person; // okay now
        return { ...person, isFollowing: !person.isFollowing };
      });
      return suggestions;
    }
    default: return state;
  }
}

jesseschalken on 22 Nov 2016

@jesseschalken wow, thanks for the fast reply! That could work... but it still feels like a hack, much like capturing idx before the closure.

I'm not sure if it's tricky to implement this, but maybe Flow could look for argument reassignments, and if none exist, assume it's proven it won't change?

goshakkk on 22 Nov 2016

👍1

@goshakkk That sounds reasonable to me, but you'll have to wait for a Flow dev to say for certain and whether it's achievable in Flow's design.

jesseschalken on 22 Nov 2016

Another way to fix the error is to set the experimental.const_params=true option in the [options] section of .flowconfig