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

Add QueryOptions and QueryBaseOptions interfaces #3569

Merged
merged 4 commits into from
Jun 11, 2018
Merged

Conversation

hwillson
Copy link
Member

ApolloClient.query is currently configured to accept WatchQueryOptions, which means the docs show people that passing things like pollingInterval into ApolloClient.query is supported (when in reality it's only supported when using ApolloClient.watchQuery). These changes add a new QueryOptions interface, and pull some of the common options out into a parent QueryBaseOptions interface. ApolloClient.query has been updated to use the QueryOptions interface, which should help clear up a bit of confusion.

Fixes #3395.

`ApolloClient.query` is currently configured to accept
`WatchQueryOptions`, which means the docs show people that
passing things like `pollingInterval` into `ApolloClient.query`
is supported (when in reality it's only supported when using
`ApolloClient.watchQuery`). These changes add a new
`QueryOptions` interface, and pull some of the common options
out into a parent `QueryBaseOptions` interface.
`ApolloClient.query` has been updated to use the `QueryOptions`
interface, which should help clear up a bit of confusion.

Fixes #3395.
@apollo-cla
Copy link

apollo-cla commented Jun 11, 2018

Warnings
⚠️

There are library changes, but not tests. That's OK as long as you're refactoring existing code

Generated by 🚫 dangerJS

@@ -672,17 +674,6 @@ export class QueryManager<TStore> {
throw new Error('returnPartialData option only supported on watchQuery.');
}

if ((options as any).pollInterval) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hwillson it would probably be good to still throw here for people not using typescript

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was on the fence on this one (I removed it just before committing). My logic was that once the option is properly removed from the docs, the number of people trying to use it should be greatly reduced. Then if it is provided by accident, it will just be ignored. But I guess it doesn't hurt to be extra communicative 📣. I'll add it back - thanks @jbaxleyiii!

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.

3 participants