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

v0.16.0 no longer distinguishes between "{number}" and number object keys #34

Open
conorbrandon opened this issue Jun 11, 2023 · 3 comments

Comments

@conorbrandon
Copy link

First off, excellent work on this package!

Unfortunately, the upgrade to v0.16.0 seems to no longer be able to detect when an object has a number property vs. a "stringified" "{number}" property. The following test correctly failed with v0.15.0, but passes with v0.16.0:

type T = {
  1: string;
  "2": string;
};

expectTypeOf<T>().toEqualTypeOf<{
  1: string;
  2: string; // on 0.15.0, this property had to be "2" (a string, not a number)
}>();

Typescript itself doesn't really distinguish between these two cases, so I'd understand why this change was made, but I found the nuance helpful. For instance, this is perfectly legal:

declare const r: Record<number, string>;
r['-1'] = "";
@BenceSzalai
Copy link

BenceSzalai commented Sep 12, 2023

Passing is the right behaviour afaics.

Consider how this is valid in both TypeScript and Runtime:

type T = {
  1: string;
  "2": string;
  3: string;
  "4": string;
};
const t: T = {
  1: 'foo',
  2: 'foo',
  "3": 'foo',
  "4": 'foo',
}

That means number keys and numeric string keys are totally interchangeable
Or to be more precise this is valid in both TypeScript and Runtime:

type R = { 1: string }
type S = { '1': string }

function foo(r: R, s: S) {
  r = s
  s = r
}

Meaning assigning objects with number keys to object with matching numeric-string keys and vice-versa is totally fine.

There is no meaningful difference between string and number keys except for two caveats:
1.) non-numeric strings cannot be used where keys are declared to be a number
2.) negative numbers cannot be used as keys together with the . notation for syntax reasons, so they can only be represented as strings or should be used with [] notation, whereas positive numbers can be represented as both numbers and strings in all cases.

None of these two indicate a case for your test example to show an error. Or am I missing the point?

Don't want to hijack your issue, but what I find however interesting is why the third line passes here:

expectTypeOf<{ 1: string   }>().toMatchTypeOf<Record<number, string>>()
expectTypeOf<{ '1': string }>().toMatchTypeOf<Record<number, string>>()
expectTypeOf<{ 'a': string }>().toMatchTypeOf<Record<number, string>>()

While this related usecase has an error:

const bar: Record<number, string> = { 'a': 'foo' } // TS2322: Type  `{ a: string; }`  is not assignable to type `Record<number, string>`

It doesn't make much sense. However this is probably a different issue, as these all seem to pass too:

expectTypeOf<Record<string, string>>().toMatchTypeOf<Record<number, string>>() // should be error
expectTypeOf<Record<number, string>>().toMatchTypeOf<Record<string, string>>() // ok, since all number keys are cast to string anyway
expectTypeOf<Record<symbol, string>>().toMatchTypeOf<Record<string, string>>() // should be error
expectTypeOf<Record<symbol, string>>().toMatchTypeOf<Record<number, string>>() // should be error
expectTypeOf<Record<string, string>>().toMatchTypeOf<Record<symbol, string>>() // should be error
expectTypeOf<Record<number, string>>().toMatchTypeOf<Record<symbol, string>>() // should be error
expectTypeOf<Record<symbol, string>>().toMatchTypeOf<Record<number, string>>() // should be error
expectTypeOf<Record<symbol, string>>().toMatchTypeOf<Record<string, string>>() // should be error

But Record<> types are not even promised in the documentation to work.

@mmkal
Copy link
Owner

mmkal commented Sep 30, 2023

I can see both arguments. To be honest, distinguishing between number and string keys doesn't feel like the most critical thing, but a theme I'm seeing from recent requests is that there are a few different levels of strictness that people want. I don't think we'll be able to make toEqualTypeOf/toMatchTypeOf all things to all people, but I'll take this into consideration when resolving issues like #33 and similar ones about v0.16.0.

@mmkal
Copy link
Owner

mmkal commented Sep 30, 2023

Also note that if you want to test specifically for this, you could do something like this:

const x = {[1]: 'one', '2': 'two'}
expectTypeOf<keyof typeof x>().exclude<string>().toEqualTypeOf<1>()

But I imagine the reason for this issue is that you don't want to have to know to test for this specifically.

conorbrandon added a commit to conorbrandon/ts-dynamodb that referenced this issue Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants