Warn when properties are accessed through ['literal']

[rkirov]

Typescript allows property access with square brackets and string literal to access regular properties (ref https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#4.13)

so the following is valid TS

class A {
  foo: string;
}

var a: A;
var b = a['foo'];

but this will fail spectacularly post minification with closure.

[evmar]

This feature is quite weird. If you create an intermediate variable (let foo = 'foo';) you cannot index into A with it (like A[foo]) -- it only allows string literals. This makes me wonder whether we can just rewrite A['foo'] into A.foo. I guess the former may occur for field names that aren't valid identifiers...

[mprobst]

Does it output a quoted access, or a non quoted? If it compiles into non
quoted, it'd actually be fine.

Evan Martin notifications@github.com schrieb am Fr., 18. März 2016, 09:23:

This feature is quite weird. If you create an intermediate variable (let
foo = 'foo';) you cannot index into A with it (like A[foo]) -- it only
allows string literals.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#23 (comment)

[evmar]

It outputs quoted.

[evmar]

Got a CL ready that checks this. I am pretty scared I'm accidentally warning on some other, legal thing, so I think I want to put this behind a temporary flag.

[evmar]

Another antipattern I spotted in the same vein:

let data: {[key: string]: string} = {
  foo: "bar",
};

I think you need to quote foo to prevent it from being renamed.

[evmar]

If you have an any, perhaps we should require to use quoted accessors?

[mprobst]

Depends on the expected use of any, I guess? But sounds reasonable, if you want actual properties, you can still cast to the actual type. We'd have to see what the impact on a large code base would be.

[rkirov]

Randomly stumbled on this bug, through our documentation. Current status is that this is handled by 'https://tsetse.info/property-renaming-safe'. In any case, we definitely don't want to add such checks into tsickle - tslint and/or tsetse is the right layer for checks.