fix(server): fix regression introduced by #5017

[jussisaurio]

Closes #5037

🎯 Changes

There were two bug reports about 10.43.3, the cause of which can be traced back to PR #5017 (which fixed another inference bug... seeing a pattern here)

I think in both bug reports the cause has something to do with using generics to construct either tRPC instances or tRPC middlewares using a type parameter that is not constrained to object, which causes TS to infer incompatible types for middleware builders after Overwrite was changed to only do the key-wise merge on types that extend object.

This PR basically reverts the changes in #5017 and replaces it with a check to see whether TWith (the type that overwrites the target type) extends a JS primitive, and if so, wholly replaces it. This achieves the intended end result of #5017 while (hopefully) fixing the issues introduced in 10.43.3 and reported in #5037.

This PR includes a test with which I was able to reproduce a compile error similar to the one(s) reported in #5037, and which was fixed with the added changes.

✅ Checklist

• I have followed the steps listed in the Contributing guide.
• If necessary, I have added documentation related to the changes made.
• I have added or updated the tests related to the changes made.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
next-prisma-starter	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 11:21am
og-image	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 11:21am
trpc-next-app-dir	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 11:21am

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
www	⬜️ Ignored (Inspect)	Visit Preview		Nov 17, 2023 11:21am

[jussisaurio]

Alternative simpler implementation that also works (works i.e. does not break tests):

/**
 * @internal
 * Overwrite properties in `TType` with properties in `TWith`
 * Only overwrites properties when the type to be overwritten
 * is an object. Otherwise it will just use the type from `TWith`.
 */
export type Overwrite<TType, TWith> = TWith extends any
  ? TType extends object
    ? {
        [K in keyof TType | keyof TWith]: K extends keyof TWith
          ? TWith[K]
          : K extends keyof TType
          ? TType[K]
          : never;
      }
    : TWith
  : never;

I am not exactly sure why this is 🤷

[KATT]

cc @jussisaurio could you have a look at the conflicts? 🙏

[jussisaurio]

Feel a bit bad about these last few bugs (the one with input(z.string() getting mangled, plus #5037)... they were basically caused by fixing other bugs involving experimental_standaloneMiddleware which is an experimental feature. So apologies for the churn, hopefully the treadmill stops pretty soon.

As a general note, I find the current propagation of types in tRPC's middlewares really, really hard to reason about (and, consequently, to make changes to it) and I think @Nick-Lucas probably feels the same way as well. Wondering if for a future major version it might make sense to think about a full rewrite of the middleware system?

[Nick-Lucas]

I think @Nick-Lucas probably feels the same way as well. Wondering if for a future major version it might make sense to think about a full rewrite of the middleware system?

Yeah so I wrote this earlier in the year: https://github.com/trpc/trpc/pull/4239/files I then abandoned it because I was trying to handle all the corner cases and it just was not working out, for identical reasons to our t.middleware and standalone middleware causing problems

I ran into all sorts of problems with the chainability of two declared types. The problem is typescript's type system isn't really built for combining static definitions like A->B + A->C = A->C as those two aren't assignable to each other, and tricking those consistently into believing the formula is A->B + B->C = A->C is what we have to achieve to compile. So the fix I provided in the Issue by using a generic typed function is the most sound way to solve the problem because generics are designed to solve that chainability problem, if that makes sense? Anyway, Sentry likely weren't a cause, they just got caught up in the bad chaining.

So I don't think a re-write of middlewares will be a solution, as part of my problem implementing this was the complexity of types, and we might be able to have a better crack at this in v11 with the simplification work that's been done. But otherwise I suggest we promote the factory/higher-order-function approach instead of creating an API to generate a middleware, because I've found that behaves much more consistently

[jussisaurio]

Having a hard time with this... why are we expecting inferred outputs to contain symbols (which is not JSON serializable)? Plus, the input (which the query returns) doesn't contain the symbol, ctx does. And if you look at ctx inside the query, it's inferred correctly

[KATT]

I messed up the test, it actually works as intended, sorry :D