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
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/client.ts
Expand Up @@ -66,6 +66,8 @@ export interface ClientOptions {
suspense?: boolean;
/** The default request policy for requests. */
requestPolicy?: RequestPolicy;
/** Use HTTP GET for queries. */
preferGetMethod?: boolean;
}

interface ActiveOperations {
Expand All @@ -82,6 +84,7 @@ export class Client {
fetchOptions?: RequestInit | (() => RequestInit);
exchange: Exchange;
suspense: boolean;
preferGetMethod: boolean;
requestPolicy: RequestPolicy;

// These are internals to be used to keep track of operations
Expand All @@ -99,6 +102,7 @@ export class Client {
this.fetch = opts.fetch;
this.suspense = !!opts.suspense;
this.requestPolicy = opts.requestPolicy || 'cache-first';
this.preferGetMethod = !!opts.preferGetMethod;

// This subject forms the input of operations; executeOperation may be
// called to dispatch a new operation on the subject
Expand Down
38 changes: 37 additions & 1 deletion src/exchanges/fetch.test.ts
Expand Up @@ -2,7 +2,8 @@ import { empty, fromValue, pipe, Source, subscribe, toPromise } from 'wonka';
import { Client } from '../client';
import { queryOperation } from '../test-utils';
import { OperationResult, OperationType } from '../types';
import { fetchExchange } from './fetch';
import { fetchExchange, convertToGet } from './fetch';
import gql from 'graphql-tag';

const fetch = (global as any).fetch as jest.Mock;
const abort = jest.fn();
Expand Down Expand Up @@ -151,3 +152,38 @@ describe('on teardown', () => {
expect(abort).toHaveBeenCalledTimes(0);
});
});

describe('convert for GET', () => {
it('should do a basic conversion', () => {
const query = `query ($id: ID!) { node(id: $id) { id } }`;
const variables = { id: 2 };
expect(convertToGet('http://localhost:3000', { query, variables })).toBe(
`http://localhost:3000?query=${encodeURIComponent(
query
)}&variables=${encodeURIComponent(JSON.stringify(variables))}`
);
});

it('should do a basic conversion with fragments', () => {
const nodeFragment = gql`
fragment nodeFragment on Node {
id
}
`;
const variables = { id: 2 };
const query = gql`
query($id: ID!) {
node(id: $id) {
...nodeFragment
}
}
${nodeFragment}
`;

expect(convertToGet('http://localhost:3000', { query, variables })).toBe(
`http://localhost:3000?query=${encodeURIComponent(
query
)}&variables=${encodeURIComponent(JSON.stringify(variables))}`
);
});
});
26 changes: 21 additions & 5 deletions src/exchanges/fetch.ts
Expand Up @@ -11,7 +11,7 @@ interface Body {
}

/** A default exchange for fetching GraphQL requests. */
export const fetchExchange: Exchange = ({ forward }) => {
export const fetchExchange: Exchange = ({ client, forward }) => {
const isOperationFetchable = (operation: Operation) => {
const { operationName } = operation;
return operationName === 'query' || operationName === 'mutation';
Expand All @@ -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.

})
);

Expand All @@ -53,7 +53,7 @@ const getOperationName = (query: DocumentNode): string | null => {
return node !== undefined && node.name ? node.name.value : null;
};

const createFetchSource = (operation: Operation) => {
const createFetchSource = (operation: Operation, shouldUseGet: boolean) => {
if (
process.env.NODE_ENV !== 'production' &&
operation.operationName === 'subscription'
Expand Down Expand Up @@ -88,9 +88,9 @@ const createFetchSource = (operation: Operation) => {
}

const fetchOptions = {
body: JSON.stringify(body),
method: 'POST',
...extraOptions,
body: shouldUseGet ? undefined : JSON.stringify(body),
method: shouldUseGet ? 'GET' : 'POST',
headers: {
'content-type': 'application/json',
...extraOptions.headers,
Expand All @@ -99,6 +99,10 @@ const createFetchSource = (operation: Operation) => {
abortController !== undefined ? abortController.signal : undefined,
};

if (shouldUseGet) {
operation.context.url = convertToGet(operation.context.url, body);
}

let ended = false;

Promise.resolve()
Expand Down Expand Up @@ -146,3 +150,15 @@ const executeFetch = (
}
});
};

export const convertToGet = (uri: string, body: Body): string => {
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
const queryParams: string[] = [`query=${encodeURIComponent(body.query)}`];

if (body.variables) {
queryParams.push(
`variables=${encodeURIComponent(JSON.stringify(body.variables))}`
);
}

return uri + '?' + queryParams.join('&');
};