Quote all property accesses on types with index types. #494
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| goog.module('test_files.quote_props.quote');var module = module || {id: 'test_files/quote_props/quote.js'};/** | ||
| * @fileoverview added by tsickle | ||
| * @suppress {checkTypes} checked by tsc | ||
| */ | ||
|
|
||
| /** | ||
| * @record | ||
| */ | ||
| function Quoted() { } | ||
| let /** @type {!Quoted} */ quoted = {}; | ||
| console.log(quoted['hello']); | ||
| quoted['hello'] = 1; | ||
| quoted['hello'] = 1; | ||
| /** | ||
| * @record | ||
| * @extends {Quoted} | ||
| */ | ||
| function QuotedMixed() { } | ||
| /** @type {number} */ | ||
| QuotedMixed.prototype.foo; | ||
| let /** @type {!QuotedMixed} */ quotedMixed = { foo: 1, 'invalid-identifier': 2 }; | ||
| console.log(quotedMixed.foo); | ||
| quotedMixed.foo = 1; | ||
| // Should be converted to non-quoted access. | ||
| quotedMixed.foo = 1; | ||
| // Must not be converted to non-quoted access, as it's not valid JS. | ||
| quotedMixed['invalid-identifier'] = 1; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // silence warnings about redeclaring vars. | ||
| export {}; | ||
|
|
||
| interface Quoted { | ||
| [k: string]: number; | ||
| } | ||
| let quoted: Quoted = {}; | ||
|
|
||
| console.log(quoted.hello); | ||
| quoted.hello = 1; | ||
| quoted['hello'] = 1; | ||
|
|
||
| interface QuotedMixed extends Quoted { | ||
| // Assume that foo should be renamed, as it is explicitly declared. | ||
| // It's unclear whether it's the right thing to do, user code might | ||
|
There was a problem hiding this comment. in the past we've thought that tsickle should produce a diagnostic for this case, which would be treated as a build error in tsc-wrapped. There was a problem hiding this comment. So the reasoning here is that if you end up with a type that has a property declaration and a string index type, and you do There's a separate question as to whether we want to disallow mixing string index types and properties in one type, as Closure does. We might want to, but that'll certainly bring some compatibility problems. It's also not generally possible without revisiting every type constructing construct in the program, as you note with the intersection type. We should think about that, but it's a later decision. Does that make sense? |
||
| // access this field in a mixed fashion. | ||
| foo: number; | ||
| 'invalid-identifier': number; | ||
| } | ||
| let quotedMixed: QuotedMixed = {foo: 1, 'invalid-identifier': 2}; | ||
| console.log(quotedMixed.foo); | ||
|
|
||
| quotedMixed.foo = 1; | ||
| // Should be converted to non-quoted access. | ||
| quotedMixed['foo'] = 1; | ||
| // Must not be converted to non-quoted access, as it's not valid JS. | ||
| quotedMixed['invalid-identifier'] = 1; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is trouble, right? could we somehow warn/lint about this kind of computation in a property name?