Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS gets very confused when merging objects inside a generic function #57466

Closed
denis-migdal opened this issue Feb 21, 2024 · 23 comments
Closed
Labels
Unactionable There isn't something we can do with this issue

Comments

@denis-migdal
Copy link

denis-migdal commented Feb 21, 2024

πŸ”Ž Search Terms

none

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________

⏯ Playground Link

Playground Link

πŸ’» Code

function foo<Data extends Record<string, any>>(data: Data) {


    const BASE_DATA = {
        "DELETED": true
    };

    // any merging method...
    // https://stackoverflow.com/questions/49682569/typescript-merge-object-types
    type RET_TYPE = Omit<Data, "DELETED"> & typeof BASE_DATA;

    let result  = Object.assign({}, data, BASE_DATA);
    let result2 = Object.assign({}, BASE_DATA, data);
    let result3 = {} as RET_TYPE;

    type T1 = RET_TYPE["DELETED"]; // = RET_TYPE["DELETED"]...

    type T2 = typeof result.DELETED; // boolean....
    type T3 = typeof result2.DELETED; // boolean.... wrong...
    type T4 = typeof result3.DELETED; // boolean.... so you know it...
    type T5 = typeof result3["DELETED"]; // = {...}["DELETED"]

    type RET_TYPE2 = Omit< Record<string, any>, "DELETED"> & typeof BASE_DATA;
    type T6 = RET_TYPE2["DELETED"]; // boolean...

    let i3: T1 = true; // error
    let i1: T5 = true; // error
    let i2: T6 = true; // ok
}

πŸ™ Actual behavior

In this context, TS is somehow losing type informations when manipulating types with [""] (cf T1 and T5), when he knows the type as shown by T4. In other contexts, TS doesn't lose the type informations as shown by T6.

There is also an issue with Object.assign() where the returned type is wrong (cf T3, data can have a member called DELETED that will override BASE_DATA.DELETED in the Object.assign().)

πŸ™‚ Expected behavior

TS shouldn't lose type informations and shouldn't bully me like that.

T3 should be any, as data can have a member called DELETED that will override BASE_DATA.DELETED in the Object.assign().

Additional information about the issue

No response

@RyanCavanaugh
Copy link
Member

Object.assign is not 100% correctly representable in the type system so the resulting types may not be perfect. See #10727 for the needed operator.

TS shouldn't lose type informations and shouldn't bully me like that.

This isn't a defined behavior; please be specific.

You seem to be a little confused on syntax,

type T5 = typeof result3["DELETED"]; // = {...}["DELETED"]

is parsed as

type T5 = (typeof result3)["DELETED"]; // = {...}["DELETED"]

so is correctly different from typeof result3.deleted, which parses as typeof (result3.deleted)

@RyanCavanaugh RyanCavanaugh added the Unactionable There isn't something we can do with this issue label Feb 21, 2024
@denis-migdal
Copy link
Author

denis-migdal commented Feb 21, 2024

This isn't a defined behavior; please be specific.

Sorry, I'm a little tired as I lost a day because of it during a debug... after having already lost a week trying to fight with other TS typing issues...

So, if we have :

type Data = Record<string, any>;
type RET_TYPE2 = Omit<Data, "DELETED"> & {DELETED: boolean}; 

We do agree that no matter what, RET_TYPE2["DELETED"] is boolean.
And indeed, TS agrees with us, good.

As well, if we have :

function foo<Data extends  Record<string, any>>() {
    type RET_TYPE = Omit<Data, "DELETED"> & {DELETED: boolean}; 
}

No matter what, RET_TYPE["DELETED"] is boolean.
But, TS gets confused :

function foo<Data extends  Record<string, any>>() {
    type RET_TYPE = Omit<Data, "DELETED"> & {DELETED: boolean}; 
    
    let a: RET_TYPE["DELETED"] = false; // TS error
}
Type 'false' is not assignable to type 'Data["DELETED"] & boolean'.(2322)

Even though Data["DELETED"] IS omitted... and TS knows that this is a boolean, as if we do :

let o = {} as RET_TYPE
o.DELETED // <= TS knows this is a boolean.

We can also get another error message if we use a number instead of a boolean :

function faa<Data extends  Record<string, any>>() {
    type RET_TYPE = Omit<Data, "DELETED"> & {DELETED: number}; 
    
    let result = {} as RET_TYPE;

    type T5 = typeof result["DELETED"];
    let a: T5 = 32; // TS error
}
Type 'number' is not assignable to type 'T5'.
  Type 'number' is not assignable to type 'Data["DELETED"]'.
    'number' is assignable to the constraint of type 'Data["DELETED"]', but 'Data["DELETED"]' could be instantiated with a different subtype of constraint 'any'.(2322)

Note: using a type alias:

function foo<Data extends  Record<string, any>>() {
    type RET_TYPE = Omit<Data, "DELETED"> & {DELETE: boolean}; 
    
    type T1 = RET_TYPE["DELETE"]
    let a: T1 = false; // TS error
}

Gives less informations :

Type 'boolean' is not assignable to type 'T1'.(2322)

so [(typeof result3)["DELETED"];] is correctly different from typeof result3.DELETED, which parses as typeof (result3.DELETED)

I don't see why the two should be any different... shouldn't it be 2 ways to get to the same goal ?
Shouldn't RET_TYPE["DELETED"] === typeof ({} as RET_TYPE).DELETED be the same ?

@denis-migdal
Copy link
Author

I think I just found out a workaround... I think I understand why it works, but I'm not fully sure.
I'll have to check tomorrow if it works in the context of my code.

function foo<Data extends Record<string, any>>(t: { [K in keyof Data]: K extends "DELETE" ? never : Data[K] } ) {

    type BASE_DATA = {DELETE: boolean};

    type RET_TYPE = {
      [K in keyof (Data&BASE_DATA)]: (K extends keyof Data ? Data[K] : never) | (K extends keyof BASE_DATA ? BASE_DATA[K] : never) 
    }

    let a: RET_TYPE["DELETE"] = true;

    return {} as RET_TYPE;
}

{
  let f = foo({a: 32});
  f.DELETE // boolean
  f.a // number

  type Z = typeof f;
  type A = Z["DELETE"] // boolean
  type B = Z["a"]      // number
}
{
  let f = foo({a: 32, DELETE: 43});
  f.DELETE // does not exists
  f.a // does not exists
}

@RyanCavanaugh
Copy link
Member

Even though Data["DELETED"] IS omitted...

Omit is a userland type alias, not a primitive operation, so TS isn't capable of using it to do the kind of higher-order reasoning that you're doing here.

@denis-migdal
Copy link
Author

Omit is a userland type alias, not a primitive operation, so TS isn't capable of using it to do the kind of higher-order reasoning that you're doing here.

Thanks for your answer.
I'm not quite sure what would be the difference between a "userland type alias" and a "non-userland type alias".
Is there some documentation on it somewhere ?

I'll have to check things more in depth tomorrow.
Some workarounds seem to work with this simple case, but I believe it didn't initially work when I first tried them in my more complex code. I'll have to test it little by little to see if I missed something or if I have indeed something else going on.

Another version that seems to work:

function foo<Data extends  Record<string, any>>(t: Data) {

    type BASE = {DELETED: boolean};

    type RET_TYPE = {

      [K in keyof (Data&BASE)]: K extends keyof BASE ? BASE[K] : Data[K]

    } & {};

    let a: RET_TYPE["DELETED"] = true; // TS error

    return {} as RET_TYPE;
}

let f = foo({a: 32});

@MartinJohns
Copy link
Contributor

MartinJohns commented Feb 22, 2024

I'm not sure you understand what Omit<Data, "DELETED"> does. This is essentially a no-op, you can't omit a specific key from a record.

@denis-migdal
Copy link
Author

denis-migdal commented Feb 22, 2024

I'm not sure you understand what Omit<Data, "DELETED"> does. This is essentially a no-op, you can't omit a specific key from a record.

Shouldn't this behavior be documented ? or raising a warning when doing so ?

I'm even more confused now. Indeed, the following code works without the Omit :

type Data = Record<string, number>;
type RET_TYPE2 = Data & {DELETED: boolean};

let b: RET_TYPE2["DELETED"] = true;

But why ??? Shouldn't RET_TYPE2["DELETED"] be of type never ? (number & boolean).

And this one doesn't :

function foo<Data extends Record<string, number>>() {
    type RET_TYPE2 = Data & {DELETED: boolean};

    let b: RET_TYPE2["DELETED"] = true; // b not assignable to type `never`.
}

EDIT: Doesn't & have an issue when using it with records ?

{
    type A = { DELETED: boolean }
    type B = { DELETED: number }
    type C = A&B; // never
}
{
    type A = { DELETED: boolean }
    type B = Record<string, number>
    let b: B = { DELETED: 42 }
    type C = A&B; // ok, shouldn't it be never too ?
}

you can't omit a specific key from a record.

I guess the best workaround we have is :

type T = Record<string, any> & {DELETED?: never}

The only issue is that it can also take undefined as a value...

@denis-migdal
Copy link
Author

I guess the best workaround we have is :

type T = Record<string, any> & {DELETED?: never}

The only issue is that it can also take undefined as a value...

According to the documentation:

The never type represents the type of values that never occur.

Then shouldn't the following be correct in this case ?

type T = {
     DELETED: never // DELETED never occur, i.e. can't be used in type T.
}
let t: T = {} // TS error: missing key DELETED

Otherwise, shouldn't the documentation state something like the following instead ?

The never type represents a type whose constraints are never satisfied. It can be used to represent the type of values that should never occurs, but the variable still exists.

I also noticed that TS doesn't seem to have a way to express a facultative attribute that can't be undefined...

@MartinJohns
Copy link
Contributor

MartinJohns commented Feb 22, 2024

Shouldn't this behavior be documented ? or raising a warning when doing so ?

You can't document every single small pitfall. And when you look up how Omit<> is defined it should be understandable. Or if you check the resulting type of your intersection and see that the DELETED is gone. Besides that, issues count as documentation too, and I'm sure there are plenty of issues about this.

But why ??? Shouldn't RET_TYPE2["DELETED"] be of type never ? (number & boolean).

It's an intentional unsoundness to allow combining types with index signatures (that's what Record<> basically is) and incompatible typed known properties. Keep in mind that having a fully sound type system is not one of TypeScripts design goals.

What you want would basically require "negated types" (#4196), so you can represent a type "any key, except this one". This is currently not possible. There are many many issues about this use case.

I also noticed that TS doesn't seem to have a way to express a facultative attribute that can't be undefined...

I'm not sure what you mean with "facultative attribute". If you mean an optional property, then this is intentional, because reading a missing property results in undefined.

@denis-migdal
Copy link
Author

denis-migdal commented Feb 22, 2024

You can't document every single small pitfall.

The issue is that it seems there are a lot of pitfalls, that are hard to understand if we don't know exactly how TS is doing things behind the curtain... and I do not know if we have access to such documentation.

And when you look up how Omit<> is defined it should be understandable.

How so ?

type Omit<T, K extends string | number | symbol> = { [P in Exclude<keyof T, K>]: T[P]; }

How am I supposed to know that if keyof T is string, then Exclude will have no effects, simply by reading the Omit definition ?

It is maybe obvious for you as you seem to be a contributor, but for a simple user, this isn't.
It is even less obvious when you get a type error due to this, with an unreadable error message due to complex type manipulation...

Or if you check the resulting type of your intersection and see that the DELETED is gone.

?

But why ??? Shouldn't RET_TYPE2["DELETED"] be of type never ? (number & boolean).

It's an intentional unsoundness to allow combining types with index signatures (that's what Record<> basically is) and incompatible typed known properties.

But why doing something unsound, when you could simply add a e.g. Assign<X, Y> type utility to explicitly perform such dangerous operation ?
This unsound thing can lead to undetected bugs, as shown by the example I gave above.

I could understand the principle, but at the same time, when doing it inside a generic function, it doesn't work anymore...
There is no consistency in this behavior...

Keep in mind that having a fully sound type system is not one of TypeScripts design goals.

It is really hard to use when we get stuck for days due to undocumented unsoundness.

And how can we know if something unsound is intentional or is a bug ?
I submitted lot of issues due to this. Couldn't it be possible to have a library of known workarounds, pitfalls, etc. ?

I'm not sure what you mean with "facultative attribute". If you mean an optional property, then this is intentional, because reading a missing property results in undefined.

If, indeed, reading a missing property results in undefined, this isn't the same as having a property which value is undefined.

For me this is clearly a bug, and I opened an issue for that:
#57479

@RyanCavanaugh
Copy link
Member

@denis-migdal the documentation tries to explain concepts so that you can understand how things work at a high level, and assumes that the reader is capable of doing simple experiments if they are curious about edge cases. If you're new to a language I always recommend doing small little tests to understand it better, as well as reading all of the documentation (it's not that long).

@denis-migdal
Copy link
Author

@denis-migdal the documentation tries to explain concepts so that you can understand how things work at a high level,

For example, in the documentation for Exclude, nowhere it states that Exclude<string, "test"> would have no effects, and tsc generates no error for that. Idem for Omit<Record<string, number>, "test">. Idem, in the documentation for type intersections, it doesn't state that we should be cautious when using them with indexed type as it produces counter-intuitive results.

But, in order to use TS properly, we need to know that, or at least to be warned by tsc.

and assumes that the reader is capable of doing simple experiments if they are curious about edge cases.

But for that we need to know that theses are edge cases.

I got an error, which were unreadable. I spent hours trying to get a minimal reproducible example.
Only to discover that there are a strange behavior that manifests sometimes, and sometimes doesn't, for something that I though was trivial. A part of code that worked in the general case, but stopped working when put in a generic function.

@RyanCavanaugh
Copy link
Member

The docs say

Constructs a type by excluding from UnionType all union members that are assignable to ExcludedMember

string isn't a union. A union type looks like this: A | B.

@denis-migdal
Copy link
Author

denis-migdal commented Feb 22, 2024

Thanks for your answer.

string isn't a union. A union type looks like this: A | B.

Well, one may assume that a string can be viewed as an union of all possible string literals (mathematically speaking it is).
Of course, we all know that we can't explicitly write all possible string literals to define such type, but there are still ways to represent such unions without describing each of its elements one by one (maybe not compatible with TS internal stuff, but as users we don't know that).

It doesn't explicitly state, e.g. "UnionType, a type defined with the union operator |, /!\ will not work on types like string.".
This is obvious once I understand it, but this isn't obvious to understand. Now that I understand, it is obvious when I re-read this sentence, but it wasn't the first times I did.

For example:

Constructs a type by picking all properties from Type and then removing Keys (string literal or union of string literals).

It seems "properties" here means "explicitly defined properties" as opposed to the "indexed properties".
But this is a level of nuance you can't comprehend if you don't already have a big expertise on TS... and really taking the time to dissect it words by words while knowing all the exact terms of TS vocabulary.

And still, here I use Omit with a Keys that isn't a string literal nor an union of string literals... and it works:

type T = Omit<Record<"toto", number>, string>;
type Z = T["toto"] // TS error (expected)

I will have to re-re-read the full documentation, now that I understand more things, and that I have more times for it.
Couldn't some warnings be added with the most common mistakes or something to help comprehension of the doc nuances ?

@RyanCavanaugh
Copy link
Member

The docs never imply that strings are unions, nor are any unions in TypeScript infinite as would be required. I don't know how to write documentation in a way that fends off any incorrect assumptions the reader might come in with, unfortunately, and people come up with new incorrect assumptions on a daily basis. I think it'd be tiresome to read docs that spend the majority of their time disclaiming incorrect interpretations.

Couldn't some warnings be added

Of course; TypeScript has thousands of warnings. If you're doing something that doesn't generate a warning, it's probably because that thing is also plausibly some intended thing. Can you be more specific?

@denis-migdal
Copy link
Author

I think it'd be tiresome to read docs that spend the majority of their time disclaiming incorrect interpretations.

What about some spoilers (e.g. with details/summary HTML tags) : "Show some common misconceptions" ?
Or a page for common misconceptions and workarounds ? To put the knowledge dispersed in the git issues into a single page ?

I don't know, maybe there is something possible to do.

Of course; TypeScript has thousands of warnings. If you're doing something that doesn't generate a warning, it's probably because that thing is also plausibly some intended thing. Can you be more specific?

For example, when doing :

Exclude<string, "toto"|"titi"> : I understand now that this is theoretically correct, but it is very likely that this doesn't do want the user tries to do.
The rule could be to warn (with an opt-out flag) "when the ExcludedMembers contains literals of a type specified in UnionType", or could be restricted to "when UnionType is a type, and ExcludedMembers is literals of this type".

In the same way :

Omit< Record<string, any>, "toto"|"titi"> : "When Type has an index signature and Keys is compatible with the index signature key ?".

@denis-migdal
Copy link
Author

denis-migdal commented Feb 24, 2024

Mmm... it seems I do have something else going on.

In the example below :

  • when manipulating types, TS doesn't know that Z2["DELETED"] is a boolean. Which may cause issue when manipulating the type further (and makes error harder to understand).
  • when manipulating values, TS do know it.

I don't know TS internal, but I suggest the following behavior when having such situation :

  • for each already known properties (here DELETED) pre-compute its type.
  • pre-compute the type of all unknown properties (?).
  • when accessing a property from a type, first check if it is a known property to return its precomputed type, else, compute the type for the unknown property ?
function foo<T extends Record<string, unknown>>() {
	const events    = {DELETED: true}//super._events;
	const addEvents = {} as T; // simulate

	{
		type Z2 = {
			[K in keyof typeof events]: boolean
		}["DELETED"];
		const _z: Z2 = true;
	}
	{
		type Z2 = {
			[K in (keyof typeof events) | string]: boolean
		}["DELETED"];
		const _z: Z2 = true;
	}
	{
		type Z2 = {
			[K in (keyof typeof events) | (keyof typeof addEvents)]: boolean
		}["DELETED"];
		const _z: Z2 = true; // ok, but Z2 is {...}["DELETE"] instead of simply boolean
	}
	{
		type Z2 = {
			[K in keyof (typeof events & typeof addEvents)]: boolean
		}["DELETED"];
		const _z: Z2 = true; // ok, but Z2 is {...}["DELETE"] instead of simply boolean
	}
	{
		type Z2 = {
			[K in (keyof typeof events) | (keyof typeof addEvents)]: K extends keyof typeof events ? boolean: string
		}["DELETED"];
		const _z: Z2 = true; // ok, but Z2 is {...}["DELETE"] instead of simply boolean
	}
}

@MartinJohns
Copy link
Contributor

The compiler does not know what type T is, so it can't resolve the type involving typeof addEvents.

@denis-migdal
Copy link
Author

denis-migdal commented Feb 24, 2024

The compiler does not know what type T is, so it can't resolve the type involving typeof addEvents.

It doesn't matter, as no matter what type is really T, it is still possible to know the type of some properties.

Furthermore :

type T = (keyof typeof events) | (keyof typeof addEvents); // = "DELETED" | keyof T
type T = keyof (typeof events & typeof addEvents); // idem

So here the compiler at least knows that he can try to precompute the type for the already known "DELETED" property.

One of the main advantage, is that error messages will get waaay more clear in such situations.

EDIT: And this isn't true, as the compiler does knows it when values are manipulated.

@denis-migdal
Copy link
Author

denis-migdal commented Feb 24, 2024

For future reference, simply adding a // @ts-ignore inside the generic method seems to work and causes no issue outside the generic method... Not ideal, but as long as it works...

@MartinJohns
Copy link
Contributor

It doesn't matter, as no matter what type is really T, it is still possible to know the type of some properties.

And that doesn't matter. The compiler does not resolve types that contain unbound generic arguments. Besides that, T could be typed { DELETED: number }, resulting in never.

For future reference, simply adding a // @ts-ignore inside the generic method seems to work and causes no issue outside the generic method...

Yes, @ts-ignore can be used as an escape-hatch, when you either willingly want to ignore potential type issues or the compiler can't figure out the correctness of your code (due to design limitations).

@denis-migdal
Copy link
Author

denis-migdal commented Feb 24, 2024

And that doesn't matter. The compiler does not resolve types that contain unbound generic arguments.

Well, it should.
It'll be :

  • more performant ;
  • cleaner type printing in errors/hovering ;
  • remove some non-errors.

Besides that, T could be typed { DELETED: number }, resulting in never.

Nope.

The only way T could be an issue, is for the 4th case (keyof (U&T)), if typed { DELETED: never }.
For the other cases, no matter T, the answer will always be boolean.

Yes, @ts-ignore can be used as an escape-hatch, when you either willingly want to ignore potential type issues or the compiler can't figure out the correctness of your code (due to design limitations).

It'll be better if we could @ts-ignore a specific error to not ignore other errors that could occurs in the line...

@denis-migdal
Copy link
Author

denis-migdal commented Feb 24, 2024

This is an issue only in the case 4:

Playground Link

So I stand correct for case 3 and 5...

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Feb 25, 2024
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Unactionable There isn't something we can do with this issue
Projects
None yet
Development

No branches or pull requests

3 participants