facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.07k stars 1.85k forks source link

Flow is not type checking this union correctly #4902

Open pfftdammitchris opened 7 years ago

pfftdammitchris commented 7 years ago

Hello all. I recently developed an excitement to type check my whole app after realizing how fun it is and now I am stuck with a problem of correctly type checking a redux action creator (dispatching). Also, I am a tiny bit over a year into programming now and will soon become a regular contributor to the flow community at github--which means flow will be my first community I will contribute to (regularly as well). Yay! It doesn't sound exciting to you readers but its exciting to me. I am growing a huge desire to dig really deep into facebook's flow type checker and help you all out soon in answering questions and fixing bugs as soon as I have dug in deep into flow's type system soon.

Anyways, Following the guide at here my typings are very similar. However, the error comes up in my IDE (atom) that the "object literal in incompatible with union: Actions | ThunkAction | PromiseAction | array type", but the types that I have defined look very exactly matched.

Target code:

export const getPosts = (collectionName: string): ThunkAction => {
  return (dispatch) => {
    dispatch({ type: JSREACTDAILY_FETCHING });
    const collection = db.collection(collectionName);
    client.login()
      .then(() =>
        collection.find({})
          .then((data: any) => dispatch({ type: JSREACTDAILY_FETCH_SUCCESS, posts: 'data' }))
          .catch((err: any) => dispatch({ type: JSREACTDAILY_FETCH_FAIL, posts: err }))
      ).catch(err => { throw err; });
  };
};

Typings:

export type FetchingPosts = {
  +type: 'JSREACTDAILY_FETCHING',
}
export type FetchPostsSuccess = {
  +type: 'JSREACTDAILY_FETCH_SUCCESS',
  +posts: any,
}
export type FetchPostsFailed = {
  +type: 'JSREACTDAILY_FETCH_FAIL',
  +posts: any,
}

export type Actions =
  | FetchingPosts
  | FetchPostsSuccess
  | FetchPostsFailed;

export type ThunkAction = (dispatch: Dispatch) => any;
export type PromiseAction = Promise<Actions>;
export type Dispatch = (action: Actions | ThunkAction | PromiseAction | Array<Actions>) => any;

In the IDE, this is what it looks like in the target code:

coffee

Here is what it looks like in the typings:

5555

In the screenshots it is exposing the words "object literal" and is pointing us directly towards the passed in objects. I assume it is the property "posts" that might be confusing flow since the first dispatcher isn't giving errors. It shouldn't be a problem since I have already negated any value checks with the "any" type for the posts, in the typings. What are your inputs?

edit: This is probably a bug from flow, isn't it?

pfftdammitchris commented 7 years ago

AT LAST! The errors have went away. I have not run my code, but the atom IDE has removed the errors with my new approach.

My new approach was to separate the object literals in the actual dispatchers and putting them into their own hash object, annotating them on declaring the object with their associated types.

Then, I proceed to dispatch in the original function using that new hash object.

Target code:

function actions(posts?: any) {
  const action: { fetchingPosts: FetchingPosts, fetchPostsSuccess: FetchPostsSuccess, fetchPostsFail: FetchPostsFailed } = {
    fetchingPosts: { type: JSREACTDAILY_FETCHING },
    fetchPostsSuccess: { type: JSREACTDAILY_FETCH_SUCCESS, posts },
    fetchPostsFail: { type: JSREACTDAILY_FETCH_FAIL, posts },
  };
  return { ...action };
}

// Return all posts
export const getPosts = (collectionName: string): ThunkAction => {
  return (dispatch) => {
    dispatch(actions().fetchingPosts);
    const collection = db.collection(collectionName);
    client.login()
      .then(() =>
        collection.find({})
          .then((data: any) => dispatch(actions(data).fetchPostsSuccess))
          .catch((err: any) => dispatch(actions(err).fetchPostsFail))
      ).catch(err => { throw err; });
  };
};

Typings (stayed the same)

export type FetchingPosts = {
  +type: 'JSREACTDAILY_FETCHING',
}
export type FetchPostsSuccess = {
  +type: 'JSREACTDAILY_FETCH_SUCCESS',
  +posts: any,
}
export type FetchPostsFailed = {
  +type: 'JSREACTDAILY_FETCH_FAIL',
  +posts: any,
}