Switch to application/x-www-form-urlencoded encoding in rewriteURIForGet.ts

[DrewML]

Hey there 👋

Change

When useGETForQueries is enabled, application/x-www-form-urlencoded encoding is now used when serializing query parameters (query/operationName/variables) instead of URL percent encoding via encodeURIComponent.

Why

When shoving a larger query into the URL, there is a risk of exceeding URL size limits anywhere past where TLS terminates. For example, I'm currently contributing to a platform that uses Fastly for caching, which has a cap of 4kb.

The biggest wins here come from space characters being replaced with + instead of %20. apollo-client currently serializes ASTs to pretty-formatted queries (at runtime) before sending them over the network, so there's a significant amount of white-space that gets encoded.

Alternative Solution

Users of apollo-client could alternatively switch to persisted queries, but this is a significantly larger change for existing applications that are just attempting to increase cache hit rates.

Implementation Notes

1. This change depends on the URL and URLSearchParams constructors, along with URLSearchParams.prototype.set. I believe these are supported in all target browsers except IE11. This is likely a breaking change for anyone that would need to add polyfills.

   It's possible to work around this by manually implementing application/x-www-form-urlencoded serialization (or at least a subset of it).

2. The https://www.ignored-base used as a base in the URL constructor is pretty hacky looking, but I figured (since there's now a dependency on URLSearchParams) we could fix some of the assumptions left in code comments (current code assumes a well-formed URI is always passed in). Happy to undo this or welcome suggestions for improvements

Checklist:

• [ ] If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• [ ] Make sure all of the significant new logic is covered by tests

[hwillson]

Thanks for this @DrewML - very interesting proposal. Switching the encoding type could be breaking for some applications, so this isn't a change we can introduce right away. That being said, we could consider allowing people to override the default encoding, just like we allow people to switch to GET via useGETForQueries. This would let us roll this change out sooner (but puts us on the hook to support another config option). Food for thought. We'll review this a bit more internally, but if you or anyone else in the community is interested in rebasing against main, that would be great. Thanks!

[alessbell]

Hi @DrewML 👋 Thanks again for opening this. I'm doing some housekeeping and just moved your original issue into our feature requests repository.

As @hwillson said above, I think this is definitely an interesting idea for us to consider for the next major. Since this PR is approaching 3 years old and 4.0 is not being actively planned yet, I'll close it for now but we'll revisit it when the time comes. Thanks!