urql-graphql/urql

Data constant inside ssrExchange is incorrectly initialised and used

Closed this issue · 3 comments

Describe the bug

Inside SSR exchange (/packages/core/src/exchanges/ssr.ts) the exposed ssrExchange is a function which initialises the data constant each time the exchange is created (i.e. each time ssrExchange is called). However, inside the ssrExchange function, there is the definition of the exchange itself (the ssr constant) which will always use the very original instance of the data constant. This means, that if I call the ssrExchange e.g. 5 times, the data constant will be created 5 times, but inside the ssr function, the first reference will be used.

The repro stackblitz contains a minimal example together with thoroughly explained repro steps in README.md.

Reproduction

https://stackblitz.com/edit/github-is7frs?file=README.md

Urql version

"@urql/core": "^5.0.5"
"urql": "^4.1.0"
"next-urql": "^5.0.2",

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct

So, this doesn't check out to me, and this seems like a usage issue rather than a bug to me. All seems as intended. Since the reproduction here doesn't use the ssrExchange it's more of a demonstration rather than a reproduction — but I also fail to see what you mean in there.

Since I'm not quite sure what your use-case / desired behaviour / problem is, let me take a step back and illustrate how we think the ssrExchange should be used ✌️

There's basically two modes the ssrExchange can be in controlled by isClient:

  • in client-mode, we read results and erase them as we go, essentially "replaying" them internally
  • in server-mode, we append results

The ssrExchange is unique in that, like some other exchanges, it's a factory that creates an Exchange. It's more unique even in it having methods to imperatively extractData and restoreData, essentially being an equivalent to server-mode and client-mode "in reverse".

It's expected that on the client-side the instance of ssrExchange is stable. That means that you might want to instantiate it as a singleton and then re-use it, since, for the lifetime of the application on the client you'd use the same one.

In other words, on the client, the ssrExchange:

  • rehydrates data (either via the input param or .restoreData)
  • replays data as the app rehydrates (so it matches the server-rendered element structure)
  • then it becomes "inert", since it deletes prior results from const data, i.e. the rehydration cache

However, on the server-side it's expected that you create an ssrExchange once per request and don't share it between users. That's why const data is not global. This is so that the data is for the critical part of the app that renders. In other words, since we have a Client per request and an ssrExchange per request, the data is specific to that request and doesn't leak across.

As per the above, on the server, the ssrExchange:

  • is created per request and not shared
  • after the request .extractData() is called, which is data passed to the client-side
  • the client-side uses it as per the above

Hence, const data cannot live lower in scope (in the Exchange), since this would make it impossible to extract the data, and it cannot live higher in scope, since that would leak data globally, across requests.
const data is also not initialized in the Exchange since you are allowed to create multiple Clients (e.g. for component islands rendering)

This is probably clearer to see in the legacy next-urql code: https://github.com/urql-graphql/urql/blob/next-urql%405.0.2/packages/next-urql/src/with-urql-client.ts

Does that make sense? I'm curious where you're diverging with this in terms of what you're trying to build.
Sorry for the long reply! ❤️

Thanks for your lengthy explanation. I was midway writing another lengthy reply, but it turns out that following the approach from the github repo you shared was enough. Unfortunately, our case is too complex to use withUrqlClient and our custom implementation was not completely correct. However, what you have shared has helped us fix it and now it seems alright. I will close this issue.

Thanks again for your help.

Alright, glad that cleared it up! I'm sorry this isn't in the docs. We've had a bit of trouble and indecision of where to go next with the docs since they've been under some conditions and are still tied to formidable.com (now: commerce.nearform.com).