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

Fix NetworkMiddleware flow type #1937

Merged
merged 3 commits into from
Jul 24, 2017
Merged

Conversation

META-DREAMER
Copy link
Contributor

@META-DREAMER META-DREAMER commented Jul 22, 2017

When adding middleware and afterware to the network interface, it should only accept either applyMiddleware or applyBatchMiddleware, but not both. The way to types were defined, it would accept both types of middleware. This is the error that the test case I added would give:

test/flow.js:33
 33: const middleware: MiddlewareInterface[] = [{
                       ^^^^^^^^^^^^^^^^^^^ property `applyMiddleware` of MiddlewareInterface. Property not found in
 97: export type NetworkMiddleware = Array<MiddlewareInterface | BatchMiddlewareInterface>;
                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^ BatchMiddlewareInterface. See: src/index.js.flow:97

test/flow.js:48
 48: const afterware: AfterwareInterface[] = [{
                      ^^^^^^^^^^^^^^^^^^ property `applyAfterware` of AfterwareInterface. Property not found in
 98: export type NetworkAfterware = Array<AfterwareInterface | BatchAfterwareInterface>;
                                                               ^^^^^^^^^^^^^^^^^^^^^^^ BatchAfterwareInterface. See: src/index.js.flow:98

@mention-bot
Copy link

@HammadJ, thanks for your PR! By analyzing the history of the files in this pull request, we identified @michalkvasnicak, @kamilkisiela and @tgriesser to be potential reviewers.

@apollo-cla
Copy link

apollo-cla commented Jul 22, 2017

Messages
📖

Please add your name and email to the AUTHORS file (optional)

📖

If this was a change that affects the external API, please update the docs and post a link to the PR in the discussion

Generated by 🚫 dangerJS

@jbaxleyiii
Copy link
Contributor

@HammadJ thank you for this! 👍 for improving flow types!

@jbaxleyiii jbaxleyiii merged commit 2b81172 into apollographql:master Jul 24, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants