create-t3-app icon indicating copy to clipboard operation
create-t3-app copied to clipboard

feat: Bug or feature? Server-side calls are not being cached

Open Sid-Turner-Ellis opened this issue 10 months ago • 1 comments

Is your feature request related to a problem? Please describe.

Not sure if this is a bug or a missing feature, but one of the key patterns with the app router is to call fetch in multiple components and have those requests cached on a per-request level (https://nextjs.org/docs/app/building-your-application/data-fetching/patterns#fetching-data-where-its-needed).

Right now, calling the same query in multiple RSC will cause multiple requests.

Describe the solution you'd like to see

Ideally, all procedures would make use of reacts Cache function

Describe alternate solutions

Right now, i'm having to wrap the rTRPC calls in react's Cache function:

api.post.hello = cache(() => api.post.hello())

Additional information

No response

Sid-Turner-Ellis avatar Apr 21 '24 11:04 Sid-Turner-Ellis

Right now, i'm having to wrap the rTRPC calls in react's Cache function: api.post.hello = cache(() => api.post.hello())

A solution to make every trpc procedure wrapped with the react's cache would be to map its leaves with some utility like:

import {cache} from "react"
// ...
export const trpc = mapLeaves(router, cache);

The mapLeaves would recursively map every leaf (the procedures) of the structure (the router) by applying the mapper (cache() function) on each of them.

yairopro avatar May 13 '24 12:05 yairopro

You can open this as a feature request in trpc

juliusmarminge avatar Jul 05 '24 08:07 juliusmarminge

Hi @juliusmarminge Sorry, but I think you miss the point. The cache we are talking about is per server-side rendering. Which means that for each server-side rendering, the cache is not shared between them. Setting a cache inside the TRPC level will share the cached data across multiple rendering (since rendering is outside the TRPC scope). And that will be a problem: what we try to render for a user might be completely different for another one.

yairopro avatar Jul 07 '24 14:07 yairopro

@yairopro I think @juliusmarminge is mainly saying this is out of scope of the planned work for CT3A, and that if this is a feature you want support for, it makes more sense to ask in the trpc project instead.

Wundero avatar Jul 07 '24 15:07 Wundero

Hi @Wundero

this is out of scope of the planned work for CT3A

Ok. I don't want to bother.

it makes more sense to ask in the trpc project instead

No, it makes less sense (with all respect) because TRPC is not related to the "react rendering", nor even to react at all. While this cache feature is all about caching TRPC calls for during an SSR. That's why I still think it should be at the link between NextJS and TRPC (this repo).

But anyway, if it's out of the planned work, I might just have to fork I guess.

yairopro avatar Jul 07 '24 15:07 yairopro

No, it makes less sense (with all respect) because TRPC is not related to the "react rendering", nor even to react at all. While this cache feature is all about caching TRPC calls for during an SSR. That's why I still think it should be at the link between NextJS and TRPC (this repo).

TRPC is actually quite ingrained into the react/nextjs ecosystem. There has been much work done on their side to support the nextjs app router, among other nextjs-specific behaviors. A feature like this is well within their scope of work, and while they are certainly swamped as an OSS project, I think it is something users could benefit from if it is implemented correctly. Additionally, this repo is not the only repo which uses trpc and nextjs together, so it makes sense to me to implement this closer to tprc's side of things anyways.

I am not really sure the best route is to implement the feature on our side either, since it likely would best be served from the trpc server-side query client, rather than the router itself. For example:

const getQueryClient = cache(createQueryClient);
const caller = createCaller(createContext, {
  // This is not valid code, just an example
  doReactCache(path, input, ctx) {
    if (path === 'path.to.cache') {
     return true;
    }
    if (path === 'path.to.check' && ctx.session?.user) {
      return true;
    }
    return false;
  }
});

This way, procedures could be cached for a response in their entirety. Unfortunately, this isn't really supported externally, though you could probably patch trpc to add this yourself (this patch is untested and may not work):

Patch for @trpc/server
diff --git a/dist/unstable-core-do-not-import/router.js b/dist/unstable-core-do-not-import/router.js
index d41e8b7b3e74589e39d67ac359f23520f93fa6ff..fb29654d8ecbe71bc76370f68f8f75013fe05082 100644
--- a/dist/unstable-core-do-not-import/router.js
+++ b/dist/unstable-core-do-not-import/router.js
@@ -5,6 +5,7 @@ var formatter = require('./error/formatter.js');
 var TRPCError = require('./error/TRPCError.js');
 var transformer = require('./transformer.js');
 var utils = require('./utils.js');
+var react = require('react');
 
 function isRouter(procedureOrRouter) {
     return procedureOrRouter._def && 'router' in procedureOrRouter._def;
@@ -65,8 +66,9 @@ const emptyRouter = {
                 if (procedures[newPath]) {
                     throw new Error(`Duplicate key: ${newPath}`);
                 }
-                procedures[newPath] = item;
-                aggregate[key] = item;
+                const cached = react.cache(item);
+                procedures[newPath] = cached;
+                aggregate[key] = cached;
             }
             return aggregate;
         }
diff --git a/dist/unstable-core-do-not-import/router.mjs b/dist/unstable-core-do-not-import/router.mjs
index c1b29d1dfb470af9c6526b018c3e20ea96cb1743..b1c53c3ce350e99c1895e4aca44b9e03aeddaab7 100644
--- a/dist/unstable-core-do-not-import/router.mjs
+++ b/dist/unstable-core-do-not-import/router.mjs
@@ -3,6 +3,7 @@ import { defaultFormatter } from './error/formatter.mjs';
 import { TRPCError, getTRPCErrorFromUnknown } from './error/TRPCError.mjs';
 import { defaultTransformer } from './transformer.mjs';
 import { mergeWithoutOverrides, omitPrototype, isFunction } from './utils.mjs';
+import { cache } from 'react';
 
 function isRouter(procedureOrRouter) {
     return procedureOrRouter._def && 'router' in procedureOrRouter._def;
@@ -63,8 +64,9 @@ const emptyRouter = {
                 if (procedures[newPath]) {
                     throw new Error(`Duplicate key: ${newPath}`);
                 }
-                procedures[newPath] = item;
-                aggregate[key] = item;
+                const cached = cache(item);
+                procedures[newPath] = cached;
+                aggregate[key] = cached;
             }
             return aggregate;
         }

Wundero avatar Jul 07 '24 15:07 Wundero

@trpc/react-query/rsc is an entrypoint with helpers for server component integration with React Query. I think (and have made some initial attempts at implementing) it makes perfect sense to have this be part of that (hence why I asked you to open this there)

juliusmarminge avatar Jul 07 '24 16:07 juliusmarminge