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

Conversation

kitten
Copy link
Member

@kitten kitten commented Sep 21, 2022

Resolve #2680

Summary

This updates preferGetMethod to allow for one more mode, which ignores the URL size limit of 2048 characters.

The option now, apart from boolean flags, also accepts preferGetMethod: 'within-url-limit' and preferGetMethod: 'force'. This is the proposed method to allow fetch exchanges to bypass the defined limit and always send a GET request for queries.

The default remains unchanged at preferGetMethod: true | 'within-url-limit'

Set of changes

  • Update types to include new options for preferGetMethod
  • Handle preferGetMethod: 'force'

@kitten kitten requested a review from JoviDeCroock September 21, 2022 14:05
@changeset-bot
Copy link

changeset-bot bot commented Sep 21, 2022

🦋 Changeset detected

Latest commit: 517cac7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@urql/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -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

@JoviDeCroock JoviDeCroock merged commit b9ca5d3 into main Sep 30, 2022
@JoviDeCroock JoviDeCroock deleted the feat/allow-ignoring-url-limit branch September 30, 2022 07:44
@urql-ci urql-ci mentioned this pull request Sep 30, 2022
@urql-ci urql-ci mentioned this pull request Oct 22, 2022
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.

RFC: Configurable url-size limit
2 participants