trpc / trpc

🧙‍♀️ Move Fast and Break Nothing. End-to-end typesafe APIs made easy.
https://tRPC.io
MIT License
35k stars 1.25k forks source link

Add a way to access the `ctx` in the `input` parsers #1963

Open jrmyio opened 2 years ago

jrmyio commented 2 years ago

I want to transform a slug or id before passing it to the resolve function. Currently you cannot access the router ctx inside the input function which has the downside that you have to do transformations inside the resolve function (if the transformation requires the ctx object).

For example: You have a user profile page (profile/{userId}) that uses trpc to get the profile data of a user. You pass this userId to trpc and want to convert the userId into a user object by utilizing some kind of UserRepository that is available on your ctx object. The UserRepository is an instance of a service, and cannot be accessed without the ctx object as you do not allow singletons.

By allowing the input to access the ctx we can utilize Zod's transform to convert a slug or id before it is passed to the resolve function. This way we can keep our resolve functions as clean as possible and keep any sort of input validation & transformation inside our input function.

According to the docs we can currently pass a Zod, Yup or superstruct object as input. I am not aware how this type is checked internally but if they all provide an object instead of a function we could allow a function as input with the ctx as an argument.

The end-result could look like:

.query('profile', {
  input: ({ ctx }) => 
      yup.object({
          user: yup.number().required(),
      })
      .transform((userId) => ctx.userRepository.findById(userId))
      .refine((user) => !!user, {
           message: 'User could not be found.',
       }),

  resolve({ input }) {
    return {
      username: input.user.username,
    };
  },
});

T-71

Funding

Fund with Polar

jlalmes commented 2 years ago

Hi @jrmyio.

The input validation should only be used for parsing request input (e.g. check that userId is defined and is a number). Any data fetching or additional logic should be placed inside your resolve function, which has access to ctx.

.query('profile', {
  input: z.object({ userId: z.number() }),
  resolve: async ({ ctx, input }) => {
    const user = await ctx.userRepository.findById(input.userId);

    if (!user) {
      throw new TRPCError({ 
        code: 'NOT_FOUND',
        message: 'User could not be found.'
      })
    }

    return {
      username: user.username
    }
  }
)

Hope this makes sense!

jrmyio commented 2 years ago

Imo, a user Id that doesn't exist in the database can be considered bad input just like a non-number userId is.

Yes, in my example I am using z.transform() but there are other use-cases like using z.enum(). Let's say my trpc calls supports a locale parameter. The available locales can be accessed from my ctx. For example: ctx.locales = ['en', 'de', 'fr'];

In order to validate the input I need to check the ctx.locales so I can use z.enum(ctx.locales). This should clearly be a use-case where you do not want this check to be inside your resolve function.

So my questions would be:

jlalmes commented 2 years ago

Hi @jrmyio.

It looks like the use-case you mentioned above is best suited to the .middleware(...) utility. You can read more about it here: https://trpc.io/docs/middlewares#raw-input.

Let me know what you think!

jrmyio commented 2 years ago

I suspect middleware not to be on a per-route level. For a parameter called locale I can see how that would always be the same as this could be a reserved parameter for your application.

But let's say I want a messaging system. The message mutation would have a parameter called receiver, not user. There is no way you would catch all use-cases into a top level middleware nor would that be good practice.

But this also highlights another reason why it's so useful to have smart ctx aware inputs. One could create a UserInput similar to my original-post and then reuse that in the message mutation for the receiver.

I love the abstraction trpc is doing where there is clearly a separation between input and resolve, and I would love to keep it that way. I want my resolve functions not to deal with additional checks or transformations (note how many MVC frameworks also support input transformation before it hits the 'code' of the controller, example Symfony).

Zod seems to have the support for transformations, the types even change when using .transform(). This is really powerful, especially for the use-cases mentioned above.

KATT commented 2 years ago

I am tentatively positive to this suggestion, but we should not consider it until we have an alpha of the next major.

The next major's design would probably allow for creating a custom input parser that allowed this as the inputs will just be an abstraction on top of middlewares...

https://github.com/trpc/trpc/blob/0616ba5c94de9f6712f338b3278e8572f08641be/packages/server/src/router.ts#L302-L304

KATT commented 2 years ago

With the new major we could potentially add a new method called .inputCallback or similar that could take whatever input takes but as a callback instead. 🤔

jrmyio commented 2 years ago

With the new major we could potentially add a new method called .inputCallback or similar that could take whatever input takes but as a callback instead. 🤔

Is the input expecting a function in its default behavior? If not, wouldn't it be possible to overload input by allowing a function as the first parameter?

KATT commented 2 years ago

@sachinraja - a penny for your thoughts; do you think there's a way to support both?

We could change the default behavior of the parser callback from inputParser( input: unknown ) { } to input( opts: { rawInput: unknown; ctx: Context }) maybe?

KATT commented 2 years ago

So usage could maybe be something like

const router = t.router({
  hello: t.procedure.input(z.string()).query(() => '...'),
  // kinda as above, but with a callback and we'll lose how to infer difference between input/output (aka zod transformers)
  identicalHello: t.procedure
    .input(({ ctx, rawInput }) => z.string().parse(rawInput))
    .query(() => '...'),
});

It would be a breaking change for the callback functions, but not for the others.

sachinraja commented 2 years ago

I'm guessing most users would not like that new API but it would reduce some magic for us. I don't think there's a great solution here but we could introduce a new method like inputWithCtx or something like that.

mmkal commented 2 years ago

Could this be a rare case where a second arg would be helpful?

const router = t.router({
  hello: t.procedure.input(z.string()).query(() => '...'),
  identicalHello: t.procedure
    .input(rawInput, ({ ctx }) => z.string().parse(rawInput))
    .query(() => '...'),
});

Alternatively, if the input parser is called "late" (i.e. inside any middleware callback), potentially there could just be a recipe involving async_hooks and people could just do

const router = t.router({
  hello: t.procedure.input(z.string()).query(() => '...'),
  identicalHello: t.procedure
    .input(rawInput => {
      const ctx = summonCtxMagicallyFromAsyncLocalStorage()
      return ctx.whatever ? z.string().parse(rawInput) : z.number().parse(rawInput)
    })
    .query(() => '...'),
});
cayter commented 2 years ago

Bumped into the same use case where I find returning localised validation error from each procedure is much more cleaner. Otherwise, I would have to come up with lots of switch cases in error handling to localise the ZodError which is what I'm currently doing.

KATT commented 2 years ago

Alternatively, if the input parser is called "late" (i.e. inside any middleware callback), potentially there could just be a recipe involving async_hooks and people could just do [...]

It actually is part of the middleware chain, so this could work but seems very hard with the inference of the type being correct.

Bumped into the same use case where I find returning localised validation error from each procedure is much more cleaner. Otherwise, I would have to come up with lots of switch cases in error handling to localise the ZodError which is what I'm currently doing.

Thanks for providing the use-case & does seem very valid.


Unsure what an API would look like to support this. We'll try to address it somehow within the next month or so.

jrmyio commented 1 year ago

I have not upgraded to tRPC 10 yet so I am not sure if this is now supported somehow?

But I got another use case. What if the validation depends on the user's role?

For example in a delivery service, an admin would be allowed to pick any delivery date, whereas a normal user of the site would only be allowed to pick certain delivery dates with strict rules.

I need to access the ctx.req to grab the user session otherwise I would have to split up the validation into 2 parts which I'd rather not.

KATT commented 1 year ago

@jrmyio you might be able to use multiple input parsers for that: https://trpc.io/docs/procedures#multiple-input-parsers

jrmyio commented 1 year ago

@KATT that url doesn't seem to be valid anymore. Would still love this feature or a work-around even if it involves some fiddeling with types.

KATT commented 1 year ago

https://trpc.io/docs/server/procedures#reusable-base-procedures

jrmyio commented 1 year ago

https://trpc.io/docs/server/procedures#reusable-base-procedures

Thanks.

I looked a little bit more into the use() and middlware options and came up with the following:

// you can store this middleware in a shared package
export const ctxInput = <T extends z.ZodType, C extends object>(getZodType: (ctx: C) => T) =>
    experimental_standaloneMiddleware<{
        ctx: C;
        rawInput: unknown;
        meta: any;
    }>().create(async ({ next, ctx, rawInput }) => {
        try {
            return next({
                ctx: {
                    ctxInput: (await getZodType(ctx as C).parseAsync(rawInput)) as z.infer<T>,
                },
            });
        } catch (cause) {
            if (cause instanceof TRPCError) {
                throw cause;
            } else {
                throw new TRPCError({
                    code: 'BAD_REQUEST',
                    cause,
                });
            }
        }
    });

which can be used via:

.use(
    ctxInput(({ req }) =>
        z.object({
             // use req here...
        }),
    ),
)

.query(async ({ ctx: { req, res, ctxInput: input } }) => {}
KATT commented 10 months ago

I had a stab over at #5234 (note that it's a fork off next), but couldn't get it to work. Will put a pin in it for now, but if anyone wants a complex TS-challenge, feel free to have a stab. :)

KATT commented 10 months ago

See #5239 for a recipe on how to do this using AsyncLocalStorage as @mmkal mentioned before. Note that this won't work with zod transforms etc, so we should still try to add something to support this.

I don't need it myself so I'm unlikely to work on it myself, contributors are welcome.

lxia1220 commented 10 months ago

I'm trying to do this, but I need to discuss the API interface.

implementation will depends on how to handle the ParserCustomValidatorEsque.

Option 1, keep ParserCustomValidatorEsque It can be seen as simply an extension of ParserCustomValidatorEsque, but additional parse is required when accessing ctx. (btw I don't like that ctx is not first param 🤷‍♂️)

type ParserCallback = (input: unknown, opts?: TContext) => TInput

// only parser usage
.input(z.number())

// access ctx callback usage
.input(async (input, { ctx }) => {
    return z
        .number()
        .transform(async (id) => {
            return await getUserById(id);
        }).parse(input);
})

// ParserCustomValidatorEsque
.input((input) => {
    if (typeof input !== "string") {
        throw new Error();
    }
    return input as string
})

Option 2

type ParserCallback = ({ctx, input}) => Parser | TValue

// only parser usage
.input(z.number())

// access ctx callback usage
.input(async ({ ctx }) => {
    return z
        .number()
        .transform(async (id) => {
            return await getUserById(id);
        }));
})

// ParserCustomValidatorEsque
.input(({ ctx, input }) => {
    if (typeof input !== "string") {
            throw new Error();
        }
        return input as string
})

which could be better?

ochicf commented 10 months ago

@lxia1220 +1 for option 2. Signature is consistent with resolver/middlewares, and also is the most flexible because it covers parser, parser creator and validator usages.

KATT commented 10 months ago

I'm fine with option 2 but it'll break a bunch of adapters that work thanks to ParserCustomValidatorEsque - you'll notice it when running pnpm vitest validators.

IIRC, effect, runtimes, and others will break which isn't ideal, so it'd need a good story on how to work with those

ironexdev commented 2 months ago

I also have the use-case where I need to access context (specifically locale) in Input definition. This is my solution since experimental_standaloneMiddleware was deprecated.

src/trpc/routers/verification-tokens.ts

const vtCreateSchema = (locale: LocaleType) =>
  z.object({
    email: z.string().email(t_invalidEmail(locale)),
  })

export const verificationTokensRouter = createTRPCRouter({
  create: publicProcedure
    .input(
      (input: unknown) => input as z.infer<ReturnType<typeof vtCreateSchema>>,
    )
    .use(ctxInput(({ locale }) => vtCreateSchema(locale)))
    .mutation(async ({ ctx }) => {
    ...

src/trpc/trpc.ts

L-Mario564 commented 3 weeks ago

What's the current status of this issue? I've encountered a use-case in one of my projects where this feature would be great. I use the mutation/query callback to do additional validation with runtime values but it would be great to have this feature to separate validation logic from what the procedure is actually supposed to do.

I may add that I'm also down to contribute with a PR if necessary.