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

Fluent query API #3447

Merged
merged 6 commits into from
Oct 28, 2021
Merged

Fluent query API #3447

merged 6 commits into from
Oct 28, 2021

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Oct 26, 2021

This PR serves as a RFC for a new ApolloCall type to make the API more fluent. It's hopefully the last big API-breaking change now that #3301 is done. An ApolloCall is a simple wrapper around a (ApolloRequest + ApolloClient). It can be configured, is self contained and contains everything needed to execute the query and return a response.

It's slightly more verbose if no customization is done (having to add the extra .execute()) but it's a lot less verbose as soon as any customization is required.

I personally like how it makes tests easier to write but the more feedback we get, the better.

Current code:

val request = ApolloRequest.Builder(HeroQuery()).fetchPolicy(FetchPolicy.NetworkOnly).build()
val response = apolloClient.query(request)

Fluent (new) code:

val response = apolloClient.query(HeroQuery())
        .fetchPolicy(FetchPolicy.NetworkOnly)
        .execute()

This API is also closer to what was done in 2.x and plays nicer with RxJava:

// Before
val response = Rx2Client(apolloClient).query(query).blockingGet()

// After
val response = apolloClient.query(query).rxSingle().blockingGet()

On the drawbacks side, it's one more type (ApolloCall) to maintain that needs to implement HasMutableExecutionContext. It should cause any performance regression but it's more code and increases the API surface. I've thought of making ApolloRequest internal to reduce the API surface a bit but it's still used as part of the ApolloInterceptor API so it's not really possible.

Let us know what you think!

@BoD
Copy link
Contributor

BoD commented Oct 26, 2021

I like it!

It's slightly more verbose if no customization is done (having to add the extra .execute()) but it's a lot less verbose as soon as any customization is required.

Indeed we can see in the tests that the custom cases look a lot nicer! For the simple case, we could add a executeQuery fun on ApolloClient which does .query().execute().

  • Pros:
    • shorter
    • if most users would end up making this extension anyway (but not sure that's fact), let's have it in the lib
  • Cons:
    • now you have 2 ways to query ;)
    • one more API
    • probably useful only in simple projects?

@martinbonnin
Copy link
Contributor Author

we could add a executeQuery fun on ApolloClient

Let's see the feedback we have until committing to more API? It's always easier to add than remove...

@martinbonnin martinbonnin merged commit 1c703f6 into dev-3.x Oct 28, 2021
@martinbonnin martinbonnin deleted the apollo-call branch October 28, 2021 16:08
@martinbonnin martinbonnin changed the title [RFC] Fluent query API Fluent query API Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants