(core/next) Only restore uncached operation data into ssr-cache

[zachasme]

Summary

We are attempting to improve our nextjs SSG strategy. We recently figured out how to statically generate pages for public resources while allowing non-public resources to not result in a temporary 404 while being fetched on the client (see discussion #1771).

The final issue we are facing is that the statically generated data hydrates client cache, due to #1602. While this preferable for most pages, it is a problem for our resource listings, where the statically generated version only includes public data, which then updates locally when a user has access to additional resources.

As far as I can tell, what happens is that when the cache has been updated with resources visible to the user, and they navigate back to the listing page, the cache is overwritten with the statically generated data (and then updated again when the client query is fetched).

Set of changes

I've changed the ssr.restoreData function to only merge operations into the cache that have not been cached yet.

I'm not entirely sure if this should be fixed in the ssrExchange or I should be focusing on next-urql, please let me know your thoughts on how best to handle this.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 08e7c06

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

[JoviDeCroock]

Hmm, this kinda feels weird to me. Generally the same key in the cache should have the same result across different pages, I'm curious how and why this would deviate.

This would be a breaking change if we were to go for this.

[zachasme]

Maybe it would make more sense to put this in
https://github.com/FormidableLabs/urql/blob/891a17a0a22e3b4d5b301a779330336efc79d1b0/packages/next-urql/src/with-urql-client.ts#L58-L60
so it only affects the next integration?

[JoviDeCroock]

Considering your specific issue wouldn't it be better to make a distinction between public and private resources from a schema perspective? Then the SSG'd data could be considered partial and fetch the private data in the background.

[kitten]

where the statically generated version only includes public data, which then updates locally when a user has access to additional resources.

If the SSR data is present and restored synchronously on mount, and more data then comes in later, how would they ever interfere, i.e. in your case the more up-to-date runtime data would override it. All data in the ssrExchange is only accessible once, meaning that the first query actually takes the data off the cache and then it's in your cacheExchange. Any subsequent results would override that.

As far as I can tell, what happens is that when the cache has been updated with resources visible to the user, and they navigate back to the listing page, the cache is overwritten with the statically generated data (and then updated again when the client query is fetched).

This can only happen if the SSR'd data is never accessed, which means that there's a discrepancy between what's in the SSR cache due to what's rendered on the server and what's on the client. The reason why I'm bringing this up is that obviously SSR != SSG in a lot of cases. If you're manually populating the ssrExchange's cache with unrelated data then the risk is that the app you're rendering never uses this data.

[zachasme]

Thanks for the feedback, I'll try and explain further, hopefully there is a simpler way to implement my use-case.

where the statically generated version only includes public data, which then updates locally when a user has access to additional resources.

If the SSR data is present and restored synchronously on mount, and more data then comes in later, how would they ever interfere, i.e. in your case the more up-to-date runtime data would override it. All data in the ssrExchange is only accessible once, meaning that the first query actually takes the data off the cache and then it's in your cacheExchange. Any subsequent results would override that.

We are using getStaticProps, and the SSG data is indeed restored and later overridden by runtime data. This is as intended.

As far as I can tell, what happens is that when the cache has been updated with resources visible to the user, and they navigate back to the listing page, the cache is overwritten with the statically generated data (and then updated again when the client query is fetched).

This can only happen if the SSR'd data is never accessed, which means that there's a discrepancy between what's in the SSR cache due to what's rendered on the server and what's on the client. The reason why I'm bringing this up is that obviously SSR != SSG in a lot of cases. If you're manually populating the ssrExchange's cache with unrelated data then the risk is that the app you're rendering never uses this data.

I'm not sure I follow. We are only populating the ssrExchange's cache with data from getStaticProps, which for 90% of our users is the same as the runtime data. But for the last 10% they will have slightly different lists of visible resources. So we use the cache-and-network request policy, such that they get a fast static response if they land directly on a resource listing page, which will then be updated shortly after when the runtime data overrides the SSG data in the cache.

This is all as intended. The problem arises when the user navigates around the application and then later returns to the resource listing page. At this point the cache has a bunch of runtime data (including that of the listing page query), and I would expect this runtime data to be used when returning to the listing page. Instead the SSG data is briefly shown and then shortly afterwards overriden with the runtime data (as the query is performed again due to the cache-and-network policy).

I assumed this was due to #1602 which Add[s a] check for when a client-side transition introduces new data to hydrate into the SSG page.].

@JoviDeCroock we do consider fields related to the authenticated user partial, but mostly fields such as viewerHasLiked etc. Would it be possible to use partial results for a list of resources where some users will have additional items?

Thanks for the help!

[kitten]

Ok, gotcha, I believe what's happening here is specific to Next SSG as when we switch the page, we may inject data from Next again into the ssrExchange that is then used when the cacheExchange doesn't have a result for it.

So if I don't misunderstand this, your intention is to not use the SSG data on navigation? Because I'm not convinced then that this PR will work. It may also break existing non-SSG use cases of next-urql, so I'm thinking what we really want here is more control over when data is used "past" the initial runtime page and when we don't

[zachasme]

Yes I think you are right. However, my intention is to still use SSG on first navigation (when the cacheExchange doesn't have a result for it), but not on subsequent visits to the same route (when the cache does have a result for it).

What are your thoughts on putting the logic in

https://github.com/FormidableLabs/urql/blob/891a17a0a22e3b4d5b301a779330336efc79d1b0/packages/next-urql/src/with-urql-client.ts#L58-L60

such that server state is only merged when no result is already present in the cache? Something like

} else if (ssr && typeof window !== 'undefined') {
  // Only restore data for uncached operations
  const cached = ssr.extractData();
  const uncached = {};
  Object.entries(urqlServerState).forEach(([key, value]) => {
    if (false === key in cached) {
      uncached[key] = value;
    }
  });
  ssr.restoreData(uncached);
}

[kitten]

Ok, actually, I talked about this with @JoviDeCroock and we did find that it's an even more specific case than stated above. So, we realised what the implementation you have here is trying to do. Unfortunately, it isn't effective as it still needs another line in invalidate to change, since there we delete properties and hence can't check for their presence later. So instead, what we can do is mark invalidated keys in the ssrExchange explicitly as null and use that to check for previously invalidated results.

Hence, I've added a test case and rewrote this a little: #1776

[zachasme]

Fantastic! Thanks for helping out and for a great library 🥇

[kitten]

Superseded by #1776

[kitten]

Fix is shipped in @urql/[email protected]