Compiler should allow assignment of new property in empty object

[yuit]

TypeScript Version: built from master

Code

var range;
range = {} 
range.newProp = 10;  // error

var obj;
obj = {
   propx: 10
}
obj.propy = 20;  // error

Expected behavior:
No error here range.newProp = 10; or obj.propy = 20;

Actual behavior:
Error TS2339 Property 'newProp' does not exist .... or Property 'propy' does not exist ...

[yortus]

Would this change stop the compiler from identifying typos? e.g. what if obj.propy is a typo?

What would the rules be for when a new property can be added, and when the 'property X does not exist' error would still be raised?

[RyanCavanaugh]

• For unannotated uninitialized (or initialized with null/undefined/empty arr/obj) variables
  • Keep this an error under NIA
• Without NIA, keep these as implicit any

[ahejlsberg]

We infer an implicit any type for let and var declarations of the following forms:

let x;
let y = null;
let z = undefined;

With #11263 we introduced control flow analysis for such implicit any variables. That's generally a good thing and certainly will detect issues that we previous didn't find, but it is technically a breaking change. For example:

let x;
x = {};
x.foo = 42;  // Error, {} has no 'foo' property

The code above used to compile because x had type any and nothing was checked. We now control flow analyze the type of x, and following the {} assignment we know that x has no foo property. We may at some point attempt to evolve the type of x when the detect this initialization pattern, but that is out of scope for 2.1.

Some older code bases that compile without --noImplicitAny rely on the loose behavior and currently require an explicit any annotation to be added before they'll compile with 2.1. If we think this is an acceptable break, we can leave things as they are.

Alternatively, we could revert back to the old behavior when --noImplicitAny is disabled (the default state), and only perform control flow analysis when --noImplicitAny is enabled. The reasoning here is that the above pattern is already an error with --noImplicitAny so nothing breaks if we start being stricter.

I'm curious what people think? I'm personally on the fence here.

[mhegazy]

One thing to note is for JS files the new behavior is more desirable. so either ways, we should keep it working in JS files.

[aluanhaddad]

As someone who generally avoids --noImplicitAny I think the new behavior is preferable. I would rather have the advantage of the evolving type analysis without having to enable the flag, even if it required me to place a few more any annotations in my code.

[ahejlsberg]

@aluanhaddad I'm curious why you avoid --noImplicitAny since it actually makes your code safer?

[aluanhaddad]

@ahejlsberg It's something I'm conflicted about.

The main reason why I avoid it is that I find that, while evolving a piece of code, some types are naturally in flux.

Since --noImplicitAny results in an error for unannotated declarations that can not at that time be inferred, the usual thing that people do is add an any annotation to "shut the type checker up". This would be fine if they went back later and made it more specific or if they removed the type annotation entirely because it became an inference site.

Unfortunately I find that the code is rarely revisited with this intent and so the any remains.

That left over any then makes the code less safe and, perhaps even more unfortunately, prevents inference from later coming into effect.

I am open to being persuaded otherwise 😃.

[aluanhaddad]

Here is something I see fairly often, basically verbatim.

First Pass

export let fields: any;

Second Pass

export let fields: any = {
  name: { match: /[a-zA-Z]+/g, value: '', valid: false },
  age: { match: /[0-9]+/, value: '', valid: false }
};

The second pass makes me very sad.

EDIT array -> object

[sandersn]

Let's talk about this at the design meeting. @ahejlsberg and I think leaving it as-is (keeping the new behaviour everywhere) is worthwhile. It is a break, but only for remarkably un-Typescript-like code, and it is easy to work around the break.

Besides, 100% of the people we surveyed want this feature even with --noImplicitAny off. :)

[mhegazy]

//cc: @DanielRosenwasser

[aluanhaddad]

@mhegazy Would it be possible to have this under a separate flag? I realize that it is important to be conservative in adding new flags, but I would really like to take advantage of this future without enabling --noImplicitAny.

Additionally I think many users who do not currently use this option could actually stand to benefit most from this feature. I believe that the fact that it will still be enabled for JavaScript files underscores my point.

[mhegazy]

We could revisit this in the future, assuming we have support for the different object construction pattern and a consistent error reporting experience.

[aluanhaddad]

@mhegazy thanks I appreciate it. Should I open an issue for it?

[aluanhaddad]

@sandersn

It is a break, but only for remarkably un-Typescript-like code

Would you mind defining this? I want to know if the code that I am writing is "un-Typescript-like" 😃

[sandersn]

Basically, writing code that makes it harder for the compiler to get the types right instead of easier.

A simple example is writing ES5-style classes instead of ES6 classes. They're basically equivalent, but the compiler is way better at understanding and giving errors on ES6 classes.

From the bug, I would expect to see

var range;
range = {} 
range.newProp = 10;  // error

Written as

var range = { newProp: 10 };

It's more succinct and easier for the compiler to analyse.

[aluanhaddad]

@sandersn Thank you for answering! I definitely prefer declarative forms such as those you mention. They are easier to reason about for humans as well for compilers.

I think what you are saying is that clean, declarative JavaScript is idiomatic TypeScript. That definitely aligns with my use of the language.