dcramer / peated

https://peated.com
Apache License 2.0
64 stars 13 forks source link

Optimize loaders to share Query cache #171

Closed dcramer closed 5 months ago

dcramer commented 5 months ago

This migrates away from direct tRPC bindings in loaders, and instead injects a query client. It does this to ensure the cache is utilized as best as possible.

The DX is intended to be lower-boilerplate:

export const { loader, clientLoader } = makeIsomorphicLoader(
  async ({ request, context: { queryUtils } }) => {
    return {
      bottleList: await queryUtils.bottleList.ensureData(),
    };
  },
);

export default function BottleList() {
  const { bottleList } = useLoaderData<typeof loader>();

  return (
    <Layout rightSidebar={<FilterSidebar />}>
      <QueryBoundary>
        <Content bottleList={bottleList} />
      </QueryBoundary>
    </Layout>
  );
}

What this does under the hood is the following:

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.08%. Comparing base (d27b869) to head (14bba97).

:white_check_mark: All tests successful. No failed tests found.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #171 +/- ## ========================================== + Coverage 78.07% 78.08% +0.01% ========================================== Files 200 200 Lines 14751 14759 +8 Branches 1241 1244 +3 ========================================== + Hits 11517 11525 +8 Misses 3234 3234 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dcramer commented 5 months ago

actually, we need to run dehydrate on it, which isnt awesome..

maybe theres a way to just hack this in...

dcramer commented 5 months ago

Ok so high level here's the issue:

Today I've managed to get it so the same query can be used server and client side. Subsequent navigations on the client can re-use the same cache. Generally speaking, its a good spot to be in.

(Caveat, the cache might not be busting between server-routing due to the singleton, and that is not a desirable default)

The challenge:

I cant see how to achieve the above using the existing hydration helpers. To do that we'd need - at the root level - to implement a shared dehydration boundary. That's ok, except the way loaders work they are component specific. That means while even though we can pass dehydratedState in every single clientLoader through our abstraction, we wont be able to actually rehydrate it.

dcramer commented 5 months ago

Found a workaround for hydration of state but now it makes the implementation far more tightly constrained. Realistically we'd need a context processor for clientLoader to avoid this.

dcramer commented 5 months ago
export function shouldRevalidate() {
  return false;
}

Appears to fix the root loader happening every single navigation entry. Not sure if its going to cause any other issues..