Various ideas to discuss

[Gejsi]

Is your feature request related to a problem? Please describe.
First of all, I'd like to say that I really like this project and appreciate all the work that has been done by the community.
I'm opening this issue to discuss some features that I think could be added (I don't want to spam the issues section).

Starting from the minor adjustments

1. Dismantle Prisma client on teardown, this can be easily done by adding one line:

// src/pages/api/trpc/[trpc].ts
[...]
import { prisma } from '../../../server/db/client'
export default createNextApiHandler({
  router: appRouter,
  createContext,
  teardown: () => prisma.$disconnect(),
})

2. Loggers are really useful during dev: both tRPC and prisma offer some kind of logging. I noticed that @theobr already opened an issue about the former (Include tRPC client 'loggerLink' for dev debugging #296). For Prisma, a possible approach may be:

export const prisma =
  global.prisma ||
  new PrismaClient({
    log:
      env.NODE_ENV === 'development' ? ['query', 'error', 'warn'] : ['error'],
  })

// unsure about this if statement,
// can be improved for the testing env?
if (env.NODE_ENV !== 'production') {
  // prisma middleware to show elapsed time
  prisma.$use(async (params, next) => {
    const before = Date.now()
    const result = await next(params)
    const after = Date.now()
    console.log(
      `Query ${params.model}.${params.action} took ${after - before}ms`
    )

    return result
  })

  global.prisma = prisma
}

Major features

3. Currently all projects scaffolded by ct3a use a session-based strategy for auth.
   Do you guys think that having a jwt-based strategy as a option could benefit the project? For example, a possible cli step:

- next-auth(database-strategy)
- next-auth(jwt-strategy)
- tRPC
- tailwind
- prisma

4. Add support for Per-page layouts:

export type NextPageWithLayout = NextPage & {
  getLayout?: (page: ReactElement) => ReactNode
}

type AppPropsWithLayout = AppProps & {
  Component: NextPageWithLayout
}

const MyApp = (({ Component, pageProps }: AppPropsWithLayout) => {
  const getLayout =
    Component.getLayout ?? ((page) => <MainLayout>{page}</MainLayout>)

  return getLayout(<Component {...pageProps} />)
}) as AppType

// at EOF
export default withTRPC<AppRouter>({})(MyApp)

5. context.ts can be improved, because currently the req/res object are being passed around for all procedures. PR chore: fix context #186 already fixes this issue but can be improved even further (after the merge maybe). The main problem is in the fact that createContext is called for each request, so getServerSession is called everytime we call the API and even endpoints that don't need to be authenticated will have to wait for the fetching of the user session: this can be solved by passing a function down to all procedures, as such:

// this code changes a bit of stuff from the PR opened by KATT
interface CreateContextOptions {
  getUserSession: () => Promise<Session | null>
}

export async function createInnerContext(opts: CreateContextOptions) {
  return {
    getUserSession: opts.getUserSession,
    prisma,
  }
}

export async function createContext(opts: CreateNextContextOptions) {
  const getUserSession = async () =>
    await getServerSession(opts.req, opts.res, authOptions)

  return await createInnerContext({
    getUserSession,
  })
}

type Context = inferAsyncReturnType<typeof createContext>

// Creates a normal tRPC router
export const createRouter = () => router<Context>()

// Creates a tRPC router that asserts all queries and mutations are from an authorized user.
// Will throw an unauthorized error if a user is not signed in.
export const createProtectedRouter = () =>
  createRouter().middleware(async ({ ctx, next }) => {
    const session = await ctx.getUserSession()
    if (!session) throw new TRPCError({ code: 'UNAUTHORIZED' })

    return next({
      ctx: {
        ...ctx,
        session, // session is known to be non-null and will be passed to other procedures
      },
    })
  })

Final considerations

I know I have shown a lot of stuff in this single issue (maybe a discussion section would be more approriate!),
but feel free to discuss (and reject) all the points: in particular, point 3 may need a lot of code to be implemented.
If help is needed, I can happily contribute to the project and open PRs.

[FlurryNight]

+1, Agree with all, but I've the same opinion for point 3, not feeling that it should come on the cli

[juliusmarminge]

1 and 2 looks fine. Haven't used 1 myself but seems reasonable if it's a 1-liner.

Not a fan of 3 tbh. Implementing the installer would be a pain if we should support selecting both since basically every file will conflict and we would need duplicates for all of them.

4 seems like a user-issue and will soon be irrelevant since Next is changing their router and layout, not sure how far away that is but it is in the works

5a (req/res) looks good, definitely should get rid of passing req/res around.

5b (session cb) Not sure if there is any performance gains in sending a callback function instead of the session? Is it really that heavy?

[Gejsi]

Not a fan of 3 tbh. Implementing the installer would be a pain if we should support selecting both since basically every file will conflict and we would need duplicates for all of them.

I was thinking about having the option to select one or another (not both). But, I understand that it may be somewhat painful to implement.

4 seems like a user-issue and will soon be irrelevant since Next is changing their router and layout, not sure how far away that is but it is in the works.

I wouldn't consider it a user-issue, because it removes some boilerplate that the dev must write just to have per-page layouts.
It's not that big of a deal, so I guess it's fine not having it since Next is changing their approach.
However, am I the only one that needed it?

5b (session cb) Not sure if there is any performance gains in sending a callback function instead of the session? Is it really that heavy?

To prove that it can hit performances, suppose you have a procedure that simply fetches some public posts that everyone (even non-logged in users) can see. The db queries would look something like:

get session -> get user
get public posts

Why would I need to get the user session if I just want to retrieve some public posts?
Through my approach, you could have something simple like:

get public posts

But still have the possibility to have auth-protected procedures (through the session cb inside createProtectedRouter):

get session -> get user
get private posts

[juliusmarminge]

To prove that it can hit performances, suppose you have a procedure that simply fetches some public posts that everyone (even non-logged in users) can see. The db queries would look something like:

get session -> get user
get public posts

That's not correct though (right?? please correct me if I'm wrong). unstable_getServerSession only grabs the session cookie, it doesn't query the db for the user-object. you have to get the user-object yourself:

export const userRouter = t.router({
  me: authedProcedure.query(async ({ ctx }) => {
    // from cookie
    const user = ctx.session.user;

    // from db
    const dbUser = await ctx.prisma.user.findFirst({
      where: {
        id: user.id,
      },
    });

    return dbUser;
  }),
});

so I don't think the gains are any significant

[Gejsi]

You are absolutely right.
I've seen those db queries while trying out some stuff with Planetscale: a prisma query as simple as

return await ctx.prisma.post.findMany({
  where: { authorId: ctx.session?.user?.id },
})

causes these queries: select Session -> select User -> select Post.
I assumed the first "user session" fetches were caused by the createContext but your argument is correct and those db queries may be an effect of the MySQL-prisma model interaction.
(If anyone has already encountered this problem, how did you solve it? Is it a problem?).

Back to the other points, should I open a PR for 1 and 2? And is your opinion still the same for points 3 and 4 @juliusmarminge?

[juliusmarminge]

Back to the other points, should I open a PR for 1 and 2? And is your opinion still the same for points 3 and 4

Yes! Please file a PR for 1 and 2. I vote no for 3 and 4 but there may be others who thinks otherwise.

[juliusmarminge]

If anyone has already encountered this problem, how did you solve it? Is it a problem?

Unsure what you mean by this. The return-statement you are referring to should be a single SQL query with joins if I'm not mistaken.

[c-ehrlich]

I agree about no for 3 and 4, because adding those things is no different than it would be in any other project/template.

[FlurryNight]

I agree about no for 3 and 4, because adding those things is no different than it would be in any other project/template.

Same

[jlalmes]

Dismantle Prisma client on teardown, this can be easily done by adding one line:

This change should not be made. The teardown function actually runs on each request after res.end which means it has no guarantee to actually work in lambdas. Additionally, it's recommended not to directly call prisma.$disconnect in NextJS (cc. prisma/prisma#5890 (comment)).

[juliusmarminge]

Dismantle Prisma client on teardown, this can be easily done by adding one line:

This change should not be made. The teardown function actually runs on each request after res.end which means it has no guarantee to actually work in lambdas. Additionally, it's recommended not to directly call prisma.$disconnect in NextJS (cc. prisma/prisma#5890 (comment)).

Nice catch. Wanna make a PR reverting the change?

[Guria]

I came here trying to solve typing burden with getLayout approach. Not voting for having it in by default but it would be much appreciated if anyone can share a snippet. I have failed to combine typings of api.withTRPC, next-auth session and custom geLayout property on Component.

4 seems like a user-issue and will soon be irrelevant since Next is changing their router and layout, not sure how far away that is but it is in the works

Documentation strongly avoids from moving in that direction since appDir is currently in beta.

[Escapado]

I came here trying to solve typing burden with getLayout approach. Not voting for having it in by default but it would be much appreciated if anyone can share a snippet. I have failed to combine typings of api.withTRPC, next-auth session and custom geLayout property on Component.

If you are willing to typecast the getLayout function you can do the following:

import { type AppType } from "next/app";
import { type Session } from "next-auth";
import { SessionProvider } from "next-auth/react";
import { api } from "~/utils/api";
import { type NextPage } from "next";

export type NextPageWithLayout = NextPage & {
  getLayout?: (page: React.ReactElement) => React.ReactElement;
};

const MainApp: AppType<{ session: Session | null }> = ({
  Component,
  pageProps: { session, ...pageProps },
}) => {
  const getLayout =
    (Component as NextPageWithLayout).getLayout ?? ((page) => page);
  return (
    <SessionProvider session={session}>
      {getLayout(<Component {...pageProps} />)}
    </SessionProvider>
  );
};

export default api.withTRPC(MainApp);

This avoids most of the typing problems and is close to the implementation of the next docs (except NextPageWithLayout now returns a React.ReactElement).

[AndKenneth]

I've opened #1363 to discuss point 4.