bug: Incorrect type inference with discriminated unions.

[virtuallyunknown]

bug: Incorrect type inference with discriminated unions.

[bradennapier]

Beautifully done. Without your code base or spending the time necessary to interact back and forth and determine your exact needs I attempted to give you ways to consider the problem from differing point of views.

I admit, I assumed I failed as I wrote it at 5 am on Christmas morning when it caught my eyes in an email.

Examined, considered, Replied, solved like a true engineer. I do sometimes forget engineers often understand directness which most do not.. :)

Superjson fixed your issue with making the codebase operate, I had no doubt of that. But it's a level of force that imho rendered your entire use case an anti pattern without having to know what it was. Not to say SJ doesn't RARELY have some use. RARELY.

We can always improve code... and indeed I see places it could become more precise... but what MATTERS if your problem is solved to your code base... not to perfect precision. Just like in math or physics or any other activity. So one could even argue becoming more precise (and therefore often less flexible or more likely to be difficult to refactor), can be the WRONG path.

A good engineer identifies where scope needs precision or flexibility.

I actually wrote a very similar solution in a PR to the application but I got too tired trying to consider all their needs for application as I was attempting to solve it in all cases through inference (similar to trpc in many ways), lol.

In typescript, you will be amazed what can be done purely in the type checks to reduce a runtime problem to null or near-null as long as you apply it at the right place with the right precision.

It looks to me you accomplished this, congrats! Happy holidays.

[microdev1]

As part of tRPC's type inference, a serialization step is performed to ensure correct type post JSON.parse(JSON.stringify()).
JSON doesn't have undefined fields so they are converted to optional in the inferred type.
The generated type is correct. In your case, value is any which extends undefined and hence the conversion.

[Nick-Lucas]

Adding a transformer is always worth it with trpc, nobody is here for ultra-performance, it’s the rich features that matter

See if it resolves the problem and let us know, it’s definitely the more beaten track!

[bradennapier]

So I wrote out a huge reply already but felt it gave off a bit of an aggressive demeanor which was not my intent.

I do want to directly and strongly point out that the any type being part of any of your runtime code is a sign of issues you should address. There are many ways to fix it. If the 3rd party library is actually typed this way then it prob a sign something is amiss as it should prob be using generics or similar.

The use of generics in my example would be strictly optional - you will notice I actually don't use them in the code ... they are there to show how you could better define types to be flexible to your needs

You could fix this on your own ofc

This way you actually retain strong typing.

I would personally prefer the added ability to infer in unknown cases like this so simple type checks are always valid and easy to do. The extending of the base type here is unnecessary but useful to provide a direct reference of what all other types should conform to.

I would NOT use superjson to fix this specific issue and believe quite strongly this represents an error in runtime code or handling -- add some checks to make this safe or dont use trpc as its a waste of your tech debt and effort - you are not getting the benefits and it is very very slow in comparison to a direct server (trpc is VERY useful as a type utility not as a server/client only)

Although in my own code i'd also never even type this loosely! Add stronger types to your runtime and you will not regret the added few lines of code to make it safe

There are literally probably 50 ways you could go about fixing this which DOES NOT enter the runtime realm and only affects type level... as above.

If you actually NEEED to have undefined and cant change it to null or a serializable friendly value then you could also force the type although this would NEVER pass a code review for any decent codebase i would hope - it is unsafe and defeats the purpose of trpc AND typescript itself.

Lastly - upon reviewing the lib itself - what this lib is doing largely is going to be pointless to type / use trpc for -- you might as well bypass it by double serializing it or casting it... so a lot of this depends on why you need to push diffs like this over the wire blindly. Double serializing sounds wasteful but no point in trying to validate or type these values and you'll have to do the same runtime checks on the client for it to be safe in any way

To the OP:
Might I suggest you check your email and read my original , although slightly more direct reply? I didn't mean it to be so, which is why I edited, but I feel it was the better response that you should read and think about!

[bradennapier]

On performance, performance is over appreciated in many code bases but fatally under appreciated in all code bases that get it wrong.

To simplify, your performance is directly determined by your own code base and needs... and many developers consider performance more important than ever needed and waste tons of time implementing better performance (which is expensive laboriously), that being said - once you get it wrong it becomes fatal quickly. Luckily that SHOULD show itself quickly if you pay attention. In some form or another that makes no sense for the first 3-4 hours you spend on it ;)

That also doesn't mean the more performant solution isn't the more elegant/simple. solution.

[virtuallyunknown]

Just to further clarify.

The return type of the normal myFunction function is:

{ type: "CREATE"; value: any; } |
{ type: "REMOVE"; oldValue: any; } |
{ type: "CHANGE"; value: any; oldValue: any; }

But the return type of the procedure client.test.query() changes it to:

{ type: "CREATE"; value?: any; } |
{ type: "REMOVE"; oldValue?: any; } |
{ type: "CHANGE"; value?: any; oldValue?: any; }

So after it goes through the trpc "meatgrinder" it changes the return type and thus I get errors.

[virtuallyunknown]

As part of tRPC's type inference, a serialization step is performed to ensure correct type post JSON.parse(JSON.stringify()). JSON doesn't have undefined fields so they are converted to optional in the inferred type. The generated type is correct. In your case, value is any which extends undefined and hence the conversion.

Well, that is unfortunate for me, but I have to agree the explanation makes total sense. May I ask if there any common/accepted way to deal with this problem?

For example, do I need to use a "data transformer" such as superjson (which seems to support undefined values), or is there any other alternative approach which doesn't operate a runtime? My issue is extra tricky too because the Difference type is coming from a 3rd party library so I have little control over it's types, and even if I did, in real life the value property could be a plethora of different things so typing that would be a challenge.

I would actually be happy to add superjson, but I wonder what the performance impact would be compared to not using it.

Well, I will do some tests tomorrow, but any possible insight would be appreciated. Cheers and merry Christmas!

[virtuallyunknown]

Okay, so there is quite a lot to unpack here, so let me try to respond as short as possible without wasting too much of your time.

I've read both the replies including the deleted one, and none of them come as aggressive, they came off as direct which is actually something I appreciate, so no offense taken whatsoever.

Next, I have tried adding superjson and it immediately solved the issue, both in compile and runtime. Perhaps I should have tried it before opening the issue, but I only figured out what the core of the problem is after your assistance, so it would have taken me hours and hours to figure it out on my own. I am yet to decide on whether or not this what I will end up using.

A short bit on performance. I try to be "performance-aware" in every bit of code I write. This is the first project I am using trpc with, and it's just for a developer tool I'm going to use, and I won't be serving millions of requests, the only client is me or other developers so "blazing fast" performance is not critical, it's not going to run in production environments. This project also serves as a nice trpc learning experience, but none of this is to say I won't be using trpc for other projects in the future, which is why I asked for more details about performance.

As for the main issue in hand, I completely agree that using any is a bad practice to be avoided, it also became clear that it's the main culprit here, not fault of trpc.

I have checked your examples, and I came up with my own solution. The problem I have is that the Difference type comes from a 3rd party library so I can't modify the ReturnType of it's functions, so I made a wrapper function.

import type { Difference } from 'microdiff';

type Diff<T extends Difference = Difference> = T extends T
    ? { [K in keyof T]:
        K extends 'value'
        ? string
        : K extends 'oldValue'
        ? string
        : T[K]
    }
    : never;

export function diffWrapper(difference: Difference[]): Diff[] {
    return difference.map(diff => {
        return ({
            ...diff,
            ...'value' in diff && { value: typeof diff.value === 'undefined' ? '' : String(diff.value) },
            ...'oldValue' in diff && { oldValue: typeof diff.oldValue === 'undefined' ? '' : String(diff.oldValue) },
        })
    })
}

After reading your replies it occurred to me that even if trpc inferred the procedure return type as any (instead of optional property), it would still be kind of an issue that I would have to deal with on the client side. And my use case here is that I have a bunch of objects in a database, I need to perform some comparison and find the differences on the server side, then send them to the client for inspection. But for this inspection, I only need a basic preview so I can see what changed, this is why I can get away with converting everything to string.

type BaseRule = {
    type: "problem" | "suggestion" | "layout" | null;
    name: string;
    description: string;
    url: string;
    library: "eslint" | "@typescript-eslint/eslint-plugin" | "eslint-plugin-react" | "eslint-plugin-unicorn" | "@stylistic/eslint-plugin";
    fixable: "code" | "whitespace" | null;
    deprecated: boolean;
    recommended: string | boolean;
    hasSuggestions: boolean;
    extendsBaseRule: string | boolean;
    requiresTypeChecking: boolean;
    replacedBy: string[];
    schema: (any[] | Record<string, any>) & (any[] | Record<string, any> | undefined);
}

In theory I could probably hack the type system even further to get better inference and send the real value to client rather than string, then do the processing there, but to put this on a web page I would need a string representation anyway, so for my use case this method is easier.

Anyway, I don't want to write walls of text, but I really appreciate your comments. It helped me understand what happens behind the scenes, and it's rare to see library developers provide such in-depth responses, so thank you very much for that.

I will also close the issue because obviously this is not a bug. Cheers!

[virtuallyunknown]

Cheers for the follow up, and yeah, once again I completely agree with everything you said (starting to notice a pattern here haha).

I do have the tendency to prematurely-optimize stuff that first needs to be made working instead, not the other way around. Recently I even started learning go because I was fascinated on how fast things can run, and wrote a small cli tool as my first project to get my hands dirty with it.

But holy guacamole, it made me realize and appreciate the value of the JS/TS ecosystem, because if this took me ~1 week to code, I could have done the equivalent tool in a single afternoon with javascript. Of course, I regret absolutely nothing because learning new things is what keeps the developer interested.

Well, I don't have anything more interesting to add, so I wish you all happy holidays :)

[github-actions]

This issue has been locked because we are very unlikely to see comments on closed issues. If you are running into a similar issue, please create a new issue. Thank you.