Skip to content

Object key type should be forced to be a string (index signature parameter) #44577

New issue

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

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

Already on GitHub? # to your account

Closed
rosenbergd opened this issue Jun 14, 2021 · 14 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@rosenbergd
Copy link

Bug Report

πŸ”Ž Search Terms

Object key type, index signature parameter type

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I believe it should act differently

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

const a: {[prop: number]: number} = {};

a[1] = 123;

for (const i in a) {
    console.log(a, a[i], typeof i);
}

πŸ™ Actual behavior

I'm allowed to set the prop key to type number, but it is actually a string when tested

πŸ™‚ Expected behavior

It should not allow me to use number as the type for the key properties as it automatically converts to string anyways which could create confusion

@MartinJohns
Copy link
Contributor

This is intentional and has been the behavior for... ever? Changing this would be a massive breaking change.

@rosenbergd
Copy link
Author

rosenbergd commented Jun 14, 2021

I'm aware that this has been the way it was forever, hence:

This is the behavior in every version I tried, and I believe it should act differently

I understand this will be a breaking change, I don't expect this to be changed in a minor version bump, but it's a dangerous design decision since it sets the wrong expectation for the type of the properties, especially in a language where types are supposed to be more controllable.

I can see that looping through the properties shows the property as a string and trying to compare using === will indicate that it will never match since it's a string, but something like + can cause unexpected bugs and there are other situations where this might cause potentially difficult to find bugs.

@jcalz
Copy link
Contributor

jcalz commented Jun 14, 2021

Sure, but it's not a bug. It's a design decision you don't agree with. You might consider changing this from a bug report to a feature suggestion. Perhaps #43041 is close to what you're asking for, and also see #32047.

@RyanCavanaugh
Copy link
Member

I don't understand what's being suggested here. Where specifically do you want an error, or do you just disagree with the existence of numeric index signatures in general?

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Jun 14, 2021
@rosenbergd
Copy link
Author

I disagree with the ability to set index type to number for an object type when in reality, once the type of the index is checked, it's actually a string. Since that's the behaviour of JS, I don't expect TS to make number indexes on object types a possibility, so I think it would make more sense to set expectations correctly and not allow the code sample I gave to work (since it's actually a lie).

const a: {[prop: number]: number} = {};

a[1] = 123;

for (const i in a) {
    console.log(a, a[i], typeof i);
}

The output of the console.log is:

{
  "1": 123
},  123,  "string"

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 14, 2021
@rosenbergd
Copy link
Author

rosenbergd commented Jun 14, 2021

I'll change this to a feature request, should I edit the original post, add a new comment or completely start a new issue or remove this and just upvote issue #43041?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 14, 2021

This is the intended behavior. Numeric index signatures mean that when you index an object by a key (which, yes, are always strings) which was produced from the coercion of a number, a particular property type is produced. It doesn't imply that a different storage format is being used at runtime.

We are not accepting feature requests to remove language constructs.

@rosenbergd
Copy link
Author

rosenbergd commented Jun 14, 2021

That's great in theory, but due to the way types work in the rest of the language, it isn't immediately obvious. There's also this:
keyof typeof a which returns a number type (which makes sense when considering you have to use a number type for the index, but since the index itself becomes a string, it gives the wrong impression).

It would be nicer to have a more concise way to specify a type as a string representation of a number which would make it more understandable than specifying the type as a number when it's a string and it would probably give people that are looking for such a feature a bit more control and consistency.

I was aware of this for a while, but I've seen people make this mistake and it's easy to understand why, it's simply not concise enough.

This would also be a great way to allow this:

const a: {[prop: number]: number} = {};

a[1] = 123;
a['2'] = 321; // this currently doesn't work

@RyanCavanaugh RyanCavanaugh removed the Needs More Info The issue still hasn't been fully clarified label Jun 14, 2021
@RyanCavanaugh
Copy link
Member

The type system is largely constructed of such artificial distinctions - at runtime there's no difference between an empty string[] and and empty number[], yet the type semantics of them are different. We need some concept of a numeric key to meaningfully describe how arrays work, which is numeric index signatures.

keyof typeof a is a fully-consistent description of what types x may have for the expression a[x].

@rosenbergd
Copy link
Author

I'm well aware that at runtime the types don't truly matter since you could save whatever type you want into whatever variable. The point remains that number is not a fitting type for this and a better type might need to be created to address this.

Some kind of numeric string type should be created or at least a type that can represent this in a more accurate way.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@rosenbergd
Copy link
Author

rosenbergd commented Jun 17, 2021

So should I make a new feature request to create a new type for this purpose instead of using number?
@RyanCavanaugh

@RyanCavanaugh
Copy link
Member

@rosenbergd if you like; please keep backcompat in mind if you want it to be seriously considered

@rosenbergd
Copy link
Author

Just linking this to the new issue created
#44649

# for free to join this conversation on GitHub. Already have an account? # to comment
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