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

implement conversion for useHTTPGetForQueries #519

Merged
merged 10 commits into from
Feb 3, 2020
Merged

Conversation

JoviDeCroock
Copy link
Collaborator

The only uncertainty (no documentation anywhere) is how to use fragments for this.

@JoviDeCroock JoviDeCroock requested a review from kitten February 3, 2020 09:54
src/exchanges/fetch.ts Outdated Show resolved Hide resolved
src/exchanges/fetch.ts Outdated Show resolved Hide resolved
src/exchanges/fetch.ts Outdated Show resolved Hide resolved
src/exchanges/fetch.ts Outdated Show resolved Hide resolved
@JoviDeCroock JoviDeCroock marked this pull request as ready for review February 3, 2020 11:28
@@ -29,7 +29,7 @@ export const fetchExchange: Exchange = ({ forward }) => {
filter(op => op.operationName === 'teardown' && op.key === key)
);

return pipe(createFetchSource(operation), takeUntil(teardown$));
return pipe(createFetchSource(operation, operation.operationName === 'query' && client.preferGetMethod), takeUntil(teardown$));
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn’t ever have to read from the client directly. Let’s move this to the operation context.

The reason for that has generally always been that the client’s option are opaque and separate from the operations and how they’re created and are dealt with in the exchange pipeline. So any option on the context can be seen, set, and manipulated by other exchanges. The same doesn’t go for the client.

To be fair, we should probably type the Client on the exchange using an interface that only contains the method we’d like to be exposed on the exchanges.

@JoviDeCroock JoviDeCroock merged commit 14a31ed into master Feb 3, 2020
@JoviDeCroock JoviDeCroock deleted the get-http-calls branch February 3, 2020 20:21
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.

3 participants