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

feat(core): Allow GET URL limit to be ignored using force option #2692

Merged
merged 1 commit into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/few-worms-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/core': patch
---

Allow URL limit for GET requests to be bypassed using `preferGetMethod: 'force'` rather than the default `true` or `'within-url-limit'` value.
22 changes: 11 additions & 11 deletions docs/api/core.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ It accepts several options on creation.

`@urql/core` also exposes `createClient()` that is just a convenient alternative to calling `new Client()`.

| Input | Type | Description |
| ----------------- | ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `exchanges` | `Exchange[]` | An array of `Exchange`s that the client should use instead of the list of `defaultExchanges` |
| `url` | `string` | The GraphQL API URL as used by `fetchExchange` |
| `fetchOptions` | `RequestInit \| () => RequestInit` | Additional `fetchOptions` that `fetch` in `fetchExchange` should use to make a request |
| `fetch` | `typeof fetch` | An alternative implementation of `fetch` that will be used by the `fetchExchange` instead of `window.fetch` |
| `suspense` | `?boolean` | Activates the experimental React suspense mode, which can be used during server-side rendering to prefetch data |
| `requestPolicy` | `?RequestPolicy` | Changes the default request policy that will be used. By default, this will be `cache-first`. |
| `preferGetMethod` | `?boolean` | This is picked up by the `fetchExchange` and will force all queries (not mutations) to be sent using the HTTP GET method instead of POST. |
| `maskTypename` | `?boolean` | Enables the `Client` to automatically apply the `maskTypename` utility to all `data` on [`OperationResult`s](#operationresult). This makes the `__typename` properties non-enumerable. |
| Input | Type | Description |
| ----------------- | ------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `exchanges` | `Exchange[]` | An array of `Exchange`s that the client should use instead of the list of `defaultExchanges` |
| `url` | `string` | The GraphQL API URL as used by `fetchExchange` |
| `fetchOptions` | `RequestInit \| () => RequestInit` | Additional `fetchOptions` that `fetch` in `fetchExchange` should use to make a request |
| `fetch` | `typeof fetch` | An alternative implementation of `fetch` that will be used by the `fetchExchange` instead of `window.fetch` |
| `suspense` | `?boolean` | Activates the experimental React suspense mode, which can be used during server-side rendering to prefetch data |
| `requestPolicy` | `?RequestPolicy` | Changes the default request policy that will be used. By default, this will be `cache-first`. |
| `preferGetMethod` | `?boolean \| 'force' \| 'within-url-limit'` | This is picked up by the `fetchExchange` and will force all queries (not mutations) to be sent using the HTTP GET method instead of POST if the length of the resulting URL doesn't exceed 2048 characters. When `'force'` is passed a GET request is always sent regardless of how long the resulting URL is. |
| `maskTypename` | `?boolean` | Enables the `Client` to automatically apply the `maskTypename` utility to all `data` on [`OperationResult`s](#operationresult). This makes the `__typename` properties non-enumerable. |

### client.executeQuery

Expand Down Expand Up @@ -204,7 +204,7 @@ properties you'll likely see some options that exist on the `Client` as well.
| `fetchOptions` | `?RequestInit \| (() => RequestInit)` | Additional `fetchOptions` that `fetch` in `fetchExchange` should use to make a request. |
| `fetch` | `typeof fetch` | An alternative implementation of `fetch` that will be used by the `fetchExchange` instead of `window.fetch` |
| `requestPolicy` | `RequestPolicy` | An optional [request policy](../basics/document-caching.md#request-policies) that should be used specifying the cache strategy. |
| `url` | `string` | The GraphQL endpoint, when using GET you should use absolute url's |
| `url` | `string` | The GraphQL endpoint, when using GET you should use absolute url's |
| `meta` | `?OperationDebugMeta` | Metadata that is only available in development for devtools. |
| `suspense` | `?boolean` | Whether suspense is enabled. |
| `preferGetMethod` | `?boolean` | Instructs the `fetchExchange` to use HTTP GET for queries. |
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/internal/fetchOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const makeFetchURL = (
body?: FetchBody
): string => {
const useGETMethod =
operation.kind === 'query' && !!operation.context.preferGetMethod;
operation.kind === 'query' && operation.context.preferGetMethod;
if (!useGETMethod || !body) return operation.context.url;

const url = new URL(operation.context.url);
Expand All @@ -40,7 +40,7 @@ export const makeFetchURL = (
search.set('extensions', stringifyVariables(body.extensions));

const finalUrl = url.toString();
if (finalUrl.length > 2047) {
if (finalUrl.length > 2047 && useGETMethod !== 'force') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to allow people to specify their own limit or are we just going to leave it to the escape-hatch?

Copy link
Member Author

@kitten kitten Sep 21, 2022

Choose a reason for hiding this comment

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

I don't really see personally why someone would have a different limit than the defacto IETF limit on the length of URLs, which also happens to be one that all browsers align on 🤔
I think the main reason we're adding this is if someone knows their entire stack can support variable length URLs, in which case, worst case, they're opting into HTTP stack errors, which are very observable. Most likely, I'd expect to see this in backend-only applications where likely no limit really applies if someone knows their stack supports longer URLs

operation.context.preferGetMethod = false;
return operation.context.url;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ export interface OperationContext {
url: string;
meta?: OperationDebugMeta;
suspense?: boolean;
preferGetMethod?: boolean;
/** Instructs fetch exchanges to use a GET request.
* When true or 'within-url-limit' is passed, GET won't be used when the resulting URL exceeds a length of 2048. */
preferGetMethod?: boolean | 'force' | 'within-url-limit';
}

/** A [query]{@link Query} or [mutation]{@link Mutation} with additional metadata for use during transmission. */
Expand Down