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

Type checker works on union types but it's buggy. #41043

Closed
Arian94 opened this issue Oct 11, 2020 · 11 comments
Closed

Type checker works on union types but it's buggy. #41043

Arian94 opened this issue Oct 11, 2020 · 11 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Arian94
Copy link

Arian94 commented Oct 11, 2020

We define a type called "Row". we then define a variable called "row" which is an instance of the Row type. The bug is the value of the row variable. The type of the key "iAmNumber" is number, but we wrote the key as "iAmString" which is a string and the type checker validates it as a correct type of value. This is the bug. More detail commented on the code below.

TypeScript Version: 4.0.3

Search Terms:

Code

type Row =
    | { [iAmNumber: number]: number; }
    | { iAmString: number; b: number; }
    | { iAmString: number; b: number; c: number; }
    | { iAmString: number; b: number; c: number; d: number };

let obj: Row = {
    iAmString: 45 // The bug is here. The 'obj' has ONE property and it's iAmString which is a string itself. 
                  // The Row type has an iAmNumber key with the type number.
                  // That is where the issue is. The type checker should either tell us to change the key (iAmString) to a number (eg 58) 
                  // or ask us to add the property 'b'.
    // b: 457,
};

console.log(obj);

Expected behavior:
Scenario 1: ERROR: Type '{ iAmString: number; }' is not assignable to type '{ [iAmNumber: number]: number; }'.
Correction: { 58: 45; }
Scenario 2: ERROR: Type '{ iAmString: number; }' is missing the following properties from type '{ iAmString: number; b: number; c: number; d: number; }': b, c, d.
Correction: { iAmString: 45; b: 34; }
Actual behavior:
No error is published. [LOG]: { "iAmString": 45 }
Playground Link:
https://www.typescriptlang.org/play?#code/C4TwDgpgBASg9gdygXgFBQ1APlA3lAbQEsBBAWwDkBXMgIwgCcAuKAOxvoYF0X27GA3FAC+6TDnykyAZWAMirAOa8OgqLRX8GQ0Zmx4oU2fKWbOQjW1XaoAYzNrd4g0bkLlVrRYc37n81AAJj4iAqioADYQwFBwtABWLPBIyHhiGK4mHgAsAKxQAPQFUAAqABbQtFSKhgDOUBUMEAB0pRVQAORx8R0NAIb1APIUAKJQYAxwkAygUH2sgYbAHfWZ7lAIZUS2ZXVzULVuSku1EBEAZq3l0MlQoJD99fOG5NRaUADWECAbRMC7-2g92gfE4zXSekhUIwRTafRiRHqm0YQPaiNqVGgiKu7WBdgqti+DAOZTgVAiiwgf0adzOESgVHqwDgUCarD6ZCx5GM62ZL0o1lixIGHwZTJZfUCi0B40m01mHVoHXBelhljyAHYADSoYRhVC2OCsWpwKLNCJwRQACm6AEoBEA
Related Issues:

@MartinJohns
Copy link
Contributor

Duplicate of #20863. Excess property checks are not performed for union types, that's why you don't get an error on the assignment to { [iAmNumber: number]: number; }. Note that indexer key types are always strings

@Arian94
Copy link
Author

Arian94 commented Oct 11, 2020

Duplicate of #20863. Excess property checks are not performed for union types, that's why you don't get an error on the assignment to { [iAmNumber: number]: number; }. Note that indexer key types are always strings

Not sure about what you say. Excess property checks actually work in union types. If you write a wrong key like iAmAnotherStr instead of what I wrote iAmString, it gives you an error.

The other thing, if indexer key types are always strings, in a case that there's only one
type Row = { [iAmNumber: number]: number; } defined in as a type, if I write a string as a key like:
let row: Row = { randomString: 45; }
it throws an error and says that your key must be of type number. So, I don't think indexer key types would always be strings.

@MartinJohns
Copy link
Contributor

Not sure about what you say. Excess property checks actually work in union types. If you write a wrong key like iAmAnotherStr instead of what I wrote iAmString, it gives you an error.

That does surprise me. Excess property checks generelly don't work on union types (at least without a discriminant, see the linked issue). I guess the compiler gets very confused here.

The other thing, if indexer key types are always strings, in a case that there's only one
type Row = { [iAmNumber: number]: number; } defined in as a type, if I write a string as a key like:
let row: Row = { randomString: 45; }
it throws an error and says that your key must be of type number. So, I don't think indexer key types would always be strings.

And that error is the excess property check.
But nothing prevents you from assigning { [key: string]: number } to your type:

type NumberKey = { [iAmNumber: number]: number; } 
type StringKey = { [iAmString: string]: number }

let stringKey: StringKey = { randomString: 45 }
let numberKey: NumberKey = stringKey; // Works just fine.

In JavaScript properties are always strings.

@Arian94
Copy link
Author

Arian94 commented Oct 11, 2020

Not sure about what you say. Excess property checks actually work in union types. If you write a wrong key like iAmAnotherStr instead of what I wrote iAmString, it gives you an error.

That does surprise me. Excess property checks generelly don't work on union types (at least without a discriminant, see the linked issue). I guess the compiler gets very confused here.

The other thing, if indexer key types are always strings, in a case that there's only one
type Row = { [iAmNumber: number]: number; } defined in as a type, if I write a string as a key like:
let row: Row = { randomString: 45; }
it throws an error and says that your key must be of type number. So, I don't think indexer key types would always be strings.

And that error is the excess property check.
But nothing prevents you from assigning { [key: string]: number } to your type:

type NumberKey = { [iAmNumber: number]: number; } 
type StringKey = { [iAmString: string]: number }

let stringKey: StringKey = { randomString: 45 }
let numberKey: NumberKey = stringKey; // Works just fine.

In JavaScript properties are always strings.

let numberKey: NumberKey = stringKey; // Works just fine. This line works fine because you used an intermediate variable which is stringKey. So, TS won't check anything. But if you write something like:
let numberKey: NumberKey = {randomString: 32}; // this won't work.

@Arian94 Arian94 changed the title Type checker doesn't work. Type checker works on union types but it's buggy. Oct 11, 2020
@Qwertiy
Copy link

Qwertiy commented Oct 11, 2020

If you want some property to determine one of the union types, it should be included into all candidates.

Here is the fix: playground

type Row =
    | { iAmString: never; [iAmNumber: number]: number; }
    | { iAmString: number; b: number; }
    | { iAmString: number; b: number; c: number; }
    | { iAmString: number; b: number; c: number; d: number };

@Arian94 Arian94 closed this as completed Oct 11, 2020
@Arian94 Arian94 reopened this Oct 11, 2020
@Arian94
Copy link
Author

Arian94 commented Oct 11, 2020

If you want some property to determine one of the union types, it should be included into all candidates.

Here is the fix: playground

type Row =
    | { iAmString: never; [iAmNumber: number]: number; }
    | { iAmString: number; b: number; }
    | { iAmString: number; b: number; c: number; }
    | { iAmString: number; b: number; c: number; d: number };

Well, I don't think so. Now I can't write something like:
let obj: Row = { 7854: 45 };
What I want is simple: I want 4 cases exactly as below:
1- let obj: Row = { 7854: 45 // a number (NOT string) as a key };
2- let obj: Row = { iAmString: 45, b: 30 };
3- let obj: Row = { iAmString: 45, b: 30, c: 596 };
4- let obj: Row = { iAmString: 45, b: 30, c: 596, d: 1002 };
With the code you provided, case 1 won't work.

@jgbpercy
Copy link

Making iAmString optional on the first type in the union seems like it might work? i.e:

type Row =
    | { iAmString?: never; [iAmNumber: number]: number; }
    | { iAmString: number; b: number; }
    | { iAmString: number; b: number; c: number; }
    | { iAmString: number; b: number; c: number; d: number };


const row: Row = { 235: 63 };
const row2: Row = { iAmString: 532, b: 64 };

Or I might be missing some reason this is also bad :)

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Oct 12, 2020
@RyanCavanaugh
Copy link
Member

All of the examples so far represent intended behavior; this is probably better suited for Stack Overflow or other support-type forums.

@Arian94
Copy link
Author

Arian94 commented Oct 12, 2020

Making iAmString optional on the first type in the union seems like it might work? i.e:

type Row =
    | { iAmString?: never; [iAmNumber: number]: number; }
    | { iAmString: number; b: number; }
    | { iAmString: number; b: number; c: number; }
    | { iAmString: number; b: number; c: number; d: number };


const row: Row = { 235: 63 };
const row2: Row = { iAmString: 532, b: 64 };

Or I might be missing some reason this is also bad :)

It's much better but the thing is that the code below is allowed too (which shouldn't be):
let obj: Row = { 234: 34, b: 45 }; // could write c and d instead of b as well

@Arian94
Copy link
Author

Arian94 commented Oct 12, 2020

All of the examples so far represent intended behavior; this is probably better suited for Stack Overflow or other support-type forums.

None of the examples represent the intended behavior. Don't get away from it.
It may not be exactly labeled as 'bug' but it is definitely doing it wrong. There is no tricking, dummy-ing etc. on the compiler, TSLint (or whatever the name is).
I just defined a union type and I want it to act exactly as expected but it's not. So to me it's a bug. None of the solutions above do the thing that is supposed to do.
Four types defined in a union-way. First one is with a key of type number, and the rest are all keys of type string. The values are all numbers.
The type checker cannot recognize which to select when a variable is defined based on the type. It's so simple. It can be fooled so easy.

@RyanCavanaugh
Copy link
Member

All of the examples so far represent intended behavior; this is probably better suited for Stack Overflow or other support-type forums.

@microsoft microsoft locked as off-topic and limited conversation to collaborators Oct 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants