Allow SSR exchange to use custom serializer

[Warxcell]

Summary

Allows SSR exchange to use custom serializer.

Set of changes

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 07f28a9e20ded16dfc4cd87dc55aaa4b39903d59

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[kitten]

This has been missing multiple things from our contributing guide

We at least need a changeset to be able to merge this, which is a changelog entry file that describes the change for release notes: https://github.com/urql-graphql/urql/blob/main/CONTRIBUTING.md#how-do-i-document-a-change-for-the-changelog

However, the PR also contains no description on why this change is important to you or why you decided to contribute this and no corresponding issue: https://github.com/urql-graphql/urql/blob/main/CONTRIBUTING.md#how-do-i-propose-changes

API additions without an explanation, even if they're straightforward look often without any reasons as to why they're needed like bloat. In this case, while the change is straightforward, exchanges are pluggable/interchangeable, i.e. you can make your own exchange that supports this behaviour separately from the core library, so we'd need a reason to know whether this would be needed by more users

[Warxcell]

The use-case is to be able to use more effective methods for serialize/deserialize. I would prefer not to copy the whole exchange just to change these 2 methods. Allowing configuration for these methods in exchange options is a big win with little cost.

[kitten]

The use-case is to be able to use more effective methods for serialize/deserialize

That's self-evident, but also imprecise. This is basically the XY Problem. I understand already that you want this change and what it does, but it's an API addition, so I'd love to understand what the motivation is.

Sorry for not clarifying this better. The "why" I'm after isn't "Why did you make this change?", as in describing the API and code change, but instead more like "Why did you need this?", so to go to the root of the change request,

• What would serialisation would you plug into these APIs?
• Why did you want to change from pure-JSON serialisation?
• What is the underlying problem you're trying to solve when swapping out serialisation?

I would prefer not to copy the whole exchange just to change these 2 methods.

The problem to us here is that that's a compelling and documented API surface. Exchanges are basically customisable extensions and we can't feasibly cover all use-cases with all exchanges all the time. The API of each exchange would balloon to tens of options. That's because exchanges are basically almost pure control flow. The same goes for this one. So, it's a balance, and hence we at least have to keep track of the changes we do want to make and why

[Warxcell]

• What would serialisation would you plug into these APIs?

https://github.com/banterfm/graphql-crunch

• Why did you want to change from pure-JSON serialisation?

Our tests showed that by using crunch rather than pure-JSON results in 40-50% size reduction of serialized result.

• What is the underlying problem you're trying to solve when swapping out serialisation?

Improve app performance, by reducing network bytes transferred by SSR server

[kitten]

Two small concerns here:

• If this is run on every SSR request the extra cost of running this may outweigh the benefits in many cases since JSON operations in most JS engines are very efficient if JS scalar parsing is skipped entirely in favour of raw string parsing (which is what we're doing in here)
• I'm a bit concerned that we'd also be applying this to result.extensions but haven't looked into this library enough to see if that's safe

Re. 1) The types do sufficiently enforce and encourage double serialisation which is desirable, so we can probably ignore this.

Re. 2) It may be nice to have a test here, specifically for graphql-crunch so we can keep it working?

Lastly, the changeset would be needed, but if push comes to shove I can add it myself to the PR, so no worries