It doesn't make much sense to forbid property access (o.unknown
) syntax on a type with a string index signature, but allow element access syntax (o['unknown']
).
Note that this should only apply to types with an explicit string index signature — element access is currently allowed on any type if you don't pass --noImplicitAny
, but this should not be true for property access.
Given
type Flags = {
aLongName: boolean;
anotherReallyLongOne: number;
someOtherOnes: boolean;
[other: string]: number | boolean;
}
Expected behavior:
function f(flags: Flags) {
flags.aLongName; // ok
flags.anUnknownName; // ok
flags.aLongname; // ok, but misspelled
flags['aLongName']; // ok
flags['anUnknownName']; // ok
flags['aLongname']; // ok, but misspelled
}
Actual behavior:
function f(flags: Flags) {
flags.aLongName; // ok
flags.anUnknownName; // error, unknown
flags.aLongname; // error, misspelled
flags['aLongName']; // ok
flags['anUnknownName']; // ok
flags['aLongname']; // ok, but misspelled
}
It doesn't make much sense to forbid property access (
o.unknown
) syntax on a type with a string index signature, but allow element access syntax (o['unknown']
).
It makes a lot of sense to me - dotted access is used when the programmer expects the property to have some particular meaning. A map (string-indexed type) is homogeneous, thus the expectation is wrong:
var stuff: { [key: string]: number };
for (var i = 0; i < stuff.length; i++) { // oops, this is nonsense - the "length" property
// doesn't have any special meaning on this object
console.log(stuff[i]);
}
I'm strongly against this change. I don't have data for modern JS usage but it used to be that when people used objects as hash tables they preferred indexing access. It might have been because of experience with more rigid languages but I believe the real reason was because it is a different logical use and indexing conveys it to the reader. Also, if this gets merged in we'll lose valuable type checking.
My plea is if you decide to go for it, please add one of your "favourite" flags to turn it off.
Edit: My point is that currently we trade some otherwise valid syntax for additional type safety. And that's good - yes, it is a convention but it is in line with the type signature. On the type level properties are declared naked and the value level counterpart is also naked dot access. Index signatures are declared inside brackets - you access them with brackets.
@gcnew, the proposal is only limited to types that have index signatures, and not all types. does this change your feelings about it?
@mhegazy No. The reason is twofold: objective - we'll lose some static typing (spelling mistakes / refactoring leftovers of "known" properties will no longer be hard checked) and subjective - it incentivises dot access for hashes, which adds burden on the reader.
Good suggestion. Will help in declaring types/interfaces for JSON structure which are received via API's
Agreed, this proposal make sense.
TLDR: this is an excellent change. The important thing here is that one should not have to resort to using the any
type to access a property.
@jeffreymorlan
It makes a lot of sense to me - dotted access is used when the programmer expects the property to have some particular meaning. A map (string-indexed type) is homogeneous, thus the expectation is wrong:
I vehemently disagree. They are often heterogeneous and that is what makes them useful.
A common example is a dynamic collection of form fields. Each value has an often dramatically different shape from the others, with completely unrelated properties such as options lists, regex validators, function validators, child fields and so on. Using index syntax to access this value has two purposes:
When writing JavaScript, I use the indexer syntax only when a key is not valid identifier. This was actually probably the most difficult part of adopting TypeScript for me, I got over at like 4 years ago but it is still annoying. Fundamentally it is far less readable to have to use the string syntax.
@gcnew
I don't have data for modern JS usage but it used to be that when people used objects as hash tables they preferred indexing access.
Even if this is true, I don't see much value in it. It reflects, as you point out, experience with statically typed languages like Java that are unable to express flexible patterns like heterogeneous collections in a typesafe fashion.
Map<string, object>
is only homogeneous in the most vacuous sense, it is in fact a static type offering no valuable type safety and a generic instantiation that has no place in a statically typed language that has union types.
it incentivises dot access for hashes, which adds burden on the reader.
I'm not sure what you mean by this. Are you saying that they should be using classes for standard types and ES2015 Maps for everything else? ECMAScript classes are not good enough. They will get better over time, but right now they are very very weak abstraction mechanism. On the other hand maps offer no first class syntax.
As Douglas Crockford said, JavaScript objects are like little hashtables and this is a great thing.
I strongly support this change.
@aluanhaddad
Are you saying that they should be using classes for standard types and ES2015 Maps for everything else?
No! All I'm saying is that currently MapLike
s are obvious and using dotted properties you are 100% safe. This suggestion blurs the line and chips away type safety. I'm a supporter of adding more guarantees, even if it means giving up some otherwise valid syntax.
The following stays the same:
type Coloured<T> = {
colour: 'red' | 'green' | 'blue',
value: T
}
declare const c: Coloured<string>;
c.color // Error: typo mistake
However this will change in a negative way:
type WithColour = {
colour: string,
[key: string]: string | undefined
}
declare const d: WithColour;
d.color // BAD! Currently an error, will no longer be
d['prop'] // it's obvious we venture into the unknown
d.prop // BAD: Does such a property exist? We'll no longer be sure :(
For me there are two distinct uses of JavaScript objects. One is for structured data - the properties are well know and enumerated in advance. The other is for Maps/Dictionaries/Hashes/you name it. This second one is completely different - we do dynamic lookup based on arbitrary keys for which a value might or might not exist. To be allowed to do this we provide an _indexing_ signature on the type. For me it's only natural to use this very same _indexing_ for access as well. It also conveys the idea that the object is a Map and there is a good chance a corresponding value is not being present.
PS: In a perfect world indexing signatures should always return T | undefined
, and definitions like
type WithColour = {
colour: string,
[key: string]: string | undefined
}
should be illegal. The above definition can easily be broken:
declare const d: WithColour;
const key: string = true ? 'colour' : '';
d[key] = undefined;
d.colour // says type `string` but the value is actually `undefined`..
@gcnew I understand why you feel that way, but it is cumbersome to be required to use subscripting syntax to access a "dynamic feeling js object".
The only alternative is resorting to any
which is much worse. Obviously it is just syntactic sugar, and not much at that, but it is very true to the nature of JavaScript.
That said, however, I appreciate the argument that the reduced safety might not be worth it, especially as type inference becomes more and more powerful, the need for dynamic dotted access becomes less and less of an issue.
I can see the argument on both, and feel both. Can we have a option to toggle that?
{
"compilerOptions": {
"allowDottedSyntaxForIndexes": true/false
}
}
EDIT: @gcnew actually already mentioned this in his first comment.
As @unional said, I can't imagine this behaviour without an option, since it is a breaking change...
@abenhamdine it's not a breaking change - it's actually more permissive. More code will work, however, it's not necessarily clear to me at this time whether everyone's on board with the idea.
Ah yes, you're right, it's definitely not a breaking change, sorry.
But the change breaks the expectations of some developpers, so in this way, an option would be a good thing to allow them to stay with the current behaviour.
Anyway, I think this change would be indeed very useful.
We discussed this in our design meeting today, and most of the team seemed in favor of moving forward.
Here's the PR again: #12671
Looking for reviewers!
We discussed this in our design meeting today, and most of the team seemed in favor of moving forward.
Why? I have yet to see anybody post any actual code that benefits from this change, that can't be written in a better way in TypeScript 2.1.
I've already demonstrated how this proposed change makes the type checker less effective at finding common errors and nobody seems to be able to concretely demonstrate the supposed upside. Even the original comment of this issue shows a misspelling going from a detected error to an undetected error!
There did used to be times when you'd want to use string-indexed types even when you know the set of valid properties, because variable indexing and mapping only worked on indexed types before, but the new keyof
and related type operators fix that.
I've thought about this a bit more, and I think there actually is a potential improvement to TypeScript along these lines. Basically, sometimes you want to declare something at an intermediate level of strong typing between a full property list { foo: boolean; bar: boolean; baz: boolean; ... }
, versus any
which gives no type safety at all. But the string-indexed type shouldn't be taken over and weakened for this purpose. What we need is a new kind of indexed type with its own syntax, maybe something like { *: T }
or { [key: any]: T }
.
This would express an object in which the set of keys may be known but the programmer can't or doesn't wish to declare them, only the value type. There actually have been a couple of times when I think this could have come in handy.
Here's an example of a function that returns the kind of object I'm thinking of:
function getPageParams() {
var ret: ??? = {};
if (location.hash) {
for (var kv of location.hash.slice(1).split('&')) {
var [k, v] = kv.split('=');
ret[k] = decodeURIComponent(v);
}
}
return ret;
}
The returned value has a type that currently can't be expressed properly. It's not a homogeneous { [key: string]: string }
map; it's an object with some fixed set of properties each with a particular meaning - but that set is dependent on something only the caller knows (the current page). Today I'd probably type it as any
, but that's unnecessarily weak since we at least know that the values must all be strings. { *: string }
would be perfect.
What we need is a new kind of indexed type with its own syntax, maybe something like
{ *: T }
I am not against something like this, but my suggestion to add a new signature type was met with a lot of groans. 馃槃
Would this change be able to also improve object prototype type safety with index signatures? I've run into a lot of people using objects like maps but unaware their code could error if they used the object prototype methods directly. E.g.
interface Foo {
[index: string]: string
}
const x: Foo = { hasOwnProperty: '' }
x.hasOwnProperty('bar') // Runtime error.
It'd be great for hasOwnProperty
in this situation to have a union of the times so the developer wouldn't be able to shoot themselves.
Would this change be able to also improve object prototype type safety with index signatures? I've run into a lot of people using objects like maps but unaware their code could error if they used the object prototype methods directly.
No. this is rather cosmetic change. whatever you could access using x["propName"]
before, after this change you should be able to access it as x.propName
. No changes to the types are involved.
Thanks @mhegazy, is there an existing issue for this or would you like me to create one?
Not that I can find.
I am not against something like this, but my suggestion to add a new signature type was met with a lot of groans.
IMO be conscious to overloading, that's a road to messiness. 馃嵒
That would be my counter argument for not introducing new signature type. 馃槣
@unional sage advice 馃槃
@gcnew @unional I think it should've be more something like this to match noImplicitAny:
{
"compilerOptions": {
"noImplicitDynamic": true/false (default false)
}
}
Edit: Actually, I don't see much difference between the two, after thinking about it. any keyword is pretty much the same as dynamic. That leads me to think that this should only apply when noImplicitAny is set to false. Maybe the option noImplicitAny should be more granular than an all or nothing (true/false).
Can this feature be disabled via a flag? For users that prefer type safety to the convenience it provides.
I agree with @bondz and @gcnew - I tend to use x["propName"]
when I know the property may not exist. I may even be using x[varStoringPropName]
since I'm using a variable to tell me which property to access. I know it's the same as using x.propName
but the former feels more like a "here be 馃悏 s" in my codebase and the latter is used 99% of the time since I know there's such a property.
I know it's only for certain types, and admittedly I don't use those types that often, but it does feel like something best put behind a flag if possible.
I'd rather see nameof
implemented 馃樅
As @abenhamdine said, this change breaks the expectation of developers, but I would not add Yet Another Flag (TM) though. The flag situation is bad enough as it is
To be honest, even if I don't support this change, having an object that has both named properties and an index signature is a mix of concerns in the way that @gcnew explained. You either have a dictionary-like data structure, where the keys are data, or you have a structured object, where each property is known in the code and specially handled. Having a hybrid is just a source of WTFs/min, regardless of how TypeScript decides to handle it.
I agree with @bondz and @gcnew - I tend to use x["propName"] when I know the property may not exist. I may even be using x[varStoringPropName] since I'm using a variable to tell me which property to access. I know it's the same as using x.propName but the former feels more like a "here be 馃悏 s" in my codebase and the latter is used 99% of the time since I know there's such a property.
Exactly, that's why it would have been great to distinguish the two differents cases on a per-object basis, and not in the whole code (without consideration of a global flag).
@use-strict I think the point of this addition is to be more idiomatic JavaScript to the native developer. This seems like a point where the dynamic nature of JavaScript is clashing with the type correctness that Typescript brings us.
@sandersn Instead of adding yet another flag, what do you think about changing the type of the noImplicitAny flag?
Option 1: A union type of boolean | "StringIndexOnly" | "ObjectOnly".
Option 2: A union type of boolean | Array<FlagOption>
where FlagOption = "StringIndex" | "Object"
. At some point, more options can be added later, and true/false can be deprecated.
Definitions:
This way it would reduce the number of flags needed in the tsconfig.json file and allow developers to gradually pick their level of enforcement.
Please see this as well: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/15683
@DanielRosenwasser I feel like if the name of a property is known at compile time, then it can be hard coded into the type def/interface, and if it is not known at compile time, then it can be accessed via the indexer. Exactly what was said here: https://github.com/Microsoft/TypeScript/issues/12596#issuecomment-264949692. We're definitely losing type safety here. I really really really want to see a compiler option to disable this "extra permissiveness".
@massimocode Maybe they could instead add a per index signature explicit any only override like explicit [index: string]: any;
to let the author decide typing enforcement. TypeScript 2.3???
That's definitely one possible approach but it would also mean updating loads of different type definitions and also getting into arguments with people about whether or not their typings should be explicit or not. Also technically probably more difficult to implement.
For me, a simple compiler flag would be awesome. That way different teams/projects can use different settings to suit their coding styles/preferences.
@massimocode They seem to be pretty much against having explicit any with index signature because it balances some internal sense of equality. I think your link shows an excellent example of where it fails. In that example, do explicit properties like $broadcast lose type definition? Maybe instead implicit [index: string]: any;
would work better.
They allow operator overloads in languages like C# for implicit object assignment without boxing/unboxing issues. Syntax like that would make sense for any type of implicit conversion. In this case, it's implicit conversion to any.
@wizarrc Explicit properties still work fine. For example:
$rootScope.$on(false); // this gives an error
$rootScope.$one(false); // this does not give an error as it falls back to the indexer of type any
I personally think that the type definition is incorrect, but the following still applies:
get(...)
method, but having said that there's nothing in JavaScript that prevents this and from what I understand the point of TypeScript is to be able to apply strong typing to as many of the JavaScript usages as possible.Just stumbled accross this because I wondered why the jquery typings would allow something like $('#myid').fadeOutttt('slow')
which is not an error.
What if i have a interface like this
interface InterfaceSharedAmonyDifferentProjects
{
method1():void;
method2():void;
[index: string]: any;
}
How is type safety enforcement supposed to work if different developers work on this interface in different projects? E.g. dev 1 changes or removes some method signatures and dev 2 does not get compiler errors when he uses the new version of the interface in his project. That can be a nightmare to fix if lots of things have changed.
Or what about type definitions (e.g. jquery uses indexers)? When method signatures are changed, the user does not get type errors.
And what about people who write method calls without using autocompletion? Typo = error.
As said, type safetly of all jquery objects are now compromised. People are now forced to use autocompletion for jquery objects in order to avoid typos and cannot be as sure as before that their jquery code still works if jquery and its typings are updated. I am sure jquery is not the only library suffering here. You cannot simply force the jquery devs and all other library devs to not use indexers, this is simply not an option!
Please at least acknowledge a problem here.
Also, I think it is not conceptually "pure" for type safety to break completely once a [index: string]: any;
signature is added to an interface. The way of using explicit string property access was intuitively cleaner because it had a feel to it that you were doing things manually, so type safety was not expected. If dotted access should now be permitted, it should be done, but I think this is the wrong way. So in this sense it is a "breaking" change, although it does not break any existing code.
Why does jQuery have that index signature in the first place?
The return type of the indexer was originally HTMLElement but changed to any
by this PR: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/413
I think someone thought $("body")["0"]
was a valid use case years ago.
We should nuke that from orbit. string -> any
indexers are basically a free for all; this is probably hiding a lot of other bugs
Maybe the compiler should give a warning if an interface exposes members that are assignable to an indexer from the same interface in order to avoid such fatal problems in the future. Of course better solutions may exist :)
These kind of problems are easy to produce and hard to find (sort of like empty catch blocks), so I think just removing the any indexer from jquery is a neccessary fix, but not sufficient to solve the whole problem.
edit: this problem is rampant. just saw multiple string => any
indexers in https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/angular/index.d.ts
I believe this change made more evil then good. What I can't understand why it wasn't introduced via an compiler option, or at least an options for old behavior was provided. It's too nuclear change.
You may say "just remove index signature". But in our case it's not easy to do. We have a hierarchy of UI components with corresponding Options interfaces for them (a component's constructor accept an object of component's Options interface). But there're also mixins which provide shared logic for components. And sometimes there should be set options for a component with options for mixin and they are not part of component Options interface. So base component class has indexing signature to allow extensibility.
But now we lost typing for component Options at all.
I guess that it's unlikely that the feature will be rolled back but please provide an option to revert pre-2.2 behavior.
@evil-shrike Can you provide code examples of how your components have been affected by this change? Consider opening a separate issue referencing this thread so that it can be properly triaged.
Not sure if this is a bug or intended.
I have a type with index signature:
interface SafeNull {
(...args: any[]): SafeNull
[key: string]: SafeNull
}
safeNull.toUpperCase() // Ok
"".toUpperCase() // Ok
let y: string | SafeNull = Math.random() < 0.5 ? "" : safeNull
y.toUpperCase() // Error
// Property 'toUpperCase' does not exist on type 'string | SafeNull'.
// Property 'toUpperCase' does not exist on type 'SafeNull'.
@Ebuall that's a bug, I think. I opened #21464 to track it.
What I am missing is something like:
type dotKeys = "key.1" | "key.2" | "complex.key.defined.here" | "simple.key";
interface HasKeysWithDot{
[x: dotKeys]: string;
}
or even better
type PropertyKeys = "normal" | "key.test" | "design.long.test" | "with.number.1";
interface DefinedKeys<T> {
[x:T]: string;
}
let objectWithDotPropertyKeys: DefinedKeys<PropertyKeys> = {
"normal": "test",
"key.test": "test2",
"design.long.test": "test3"
}
Assigning object with dot keys is possible approach in javascript. There should be some typings for this.
Most helpful comment
I can see the argument on both, and feel both. Can we have a option to toggle that?
EDIT: @gcnew actually already mentioned this in his first comment.