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
Member

JoviDeCroock commented Feb 3, 2020

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

@JoviDeCroock JoviDeCroock requested a review from kitten Feb 3, 2020
JoviDeCroock added 2 commits Feb 3, 2020
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
src/exchanges/fetch.ts Outdated Show resolved Hide resolved
JoviDeCroock added 4 commits Feb 3, 2020
@JoviDeCroock JoviDeCroock marked this pull request as ready for review Feb 3, 2020
@@ -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$));

This comment has been minimized.

Copy link
@kitten

kitten Feb 3, 2020

Member

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 added 3 commits Feb 3, 2020
@kitten
kitten approved these changes Feb 3, 2020
@JoviDeCroock JoviDeCroock merged commit 14a31ed into master Feb 3, 2020
2 checks passed
2 checks passed
Pika CI Package preview is ready!
Details
Travis CI - Pull Request Build Passed
Details
@JoviDeCroock JoviDeCroock deleted the get-http-calls branch Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.