TypeScript Version: 2.1.1
Code
class PartialPolicySearchIten {
public displayValue: string;
public oriNumber: string;
public parentName: string;
public sysNumber: string;
public systemCode: number;
public systemName: string;
}
Expected behavior:
class PartialPolicySearchIten {
constructor() {
this.displayValue = undefined;
this.oriNumber = undefined;
this.parentName = undefined;
this.sysNumber = undefined;
this.systemCode = undefined;
this. systemName = undefined;
}
}
Actual behavior:
class PartialPolicySearchIten { }
The problem comes when using Reflection it can't find the properties. I would expected that the properties exists as i'm redefining them. Otherwise their very little point is declaring properties of a class.
You need to explicitly initialise them as undefined to have the effect you want.
Input
class ImplicitlyUndefinedProperties {
public foo: string;
public bar: string;
}
class ExplicitlyUndefinedProperties {
public foo: string = undefined;
public bar: string = undefined;
}
Output
var ImplicitlyUndefinedProperties = (function () {
function ImplicitlyUndefinedProperties() {
}
return ImplicitlyUndefinedProperties;
}());
var ExplicitlyUndefinedProperties = (function () {
function ExplicitlyUndefinedProperties() {
this.foo = undefined;
this.bar = undefined;
}
return ExplicitlyUndefinedProperties;
}());
Therefore I think that this is not an issue but is intended behaviour.
While this is the original/current behavior, the current ES proposal seems to actually imply that a property will indeed be defined & set to undefined
even without an initializer.
https://tc39.github.io/proposal-class-public-fields/#initialize-public-static-fields
The proposal is now Stage 3 with this behavior. We need to discuss whether or not to take a break for this
(Public) class fields are enabled by default in Chrome 72 with the initializer semantics outlined above. This part of the spec is extremely unlikely to change. Current topics of debate are just how private fields should work AFAICT.
Can we please revisit and get a decision here? While the Babel transform is not officially supported, they would want to wait for a decision by the TypeScript team before implementing any fixes: https://github.com/babel/babel/issues/9105
The major arguments for keeping the current semantics to me are:
In favor of switching:
Any input would be much appreciated ☺️❤️
- "re-typing" a class field in a derived class is easier.\
For type-only information something like this could work:
class Foo {
public x: string; // initialized to undefined
declare public y: string; // type-only
}
@nicolo-ribaudo not sure how it's possible, tried it on ts playground and got declare' modifier cannot appear on a class element
.
@kutomer I think what @nicolo-ribaudo wanted to express is that the declare
keyword could be used as _new_ syntax, i.e. it does not work yet and would be a breaking change to TypeScript. 🙂
Yeah exactly
Also worth noting here that moving to spec compliance is a loud breaking change, because the existing types end up being wrong with strictNullChecks: true
:
class Foo {
public x: string; // initialized to undefined
}
☝️ would be wrong; the type would need to be:
class Foo {
public x?: string; // initialized to undefined
}
This will be a non-trivial breaking change to a lot of codebases. (It intersects with definite strictPropertyInitialization
a bit as well.)
Case in point, this breaks angularjs (tested with 1.5.11) bindings to components:
~~~~
class Controller {
public foo: () => string;
}
const component: ng.IComponentOptions = {
controller: Controller,
controllerAs: '$ctrl',
bindings: {
foo: '&',
}
}
~~~~
Works with tsc (angularjs initializes $ctrl.foo) but fails with @babel/typescript ($ctrl.foo is initialized as undefined and angularjs does not overwrite it).
@vbraun It's an AngularJS bug already fixed in 1.6.
Previously AngularJS sets binding properties even before the constructor being invoked, making it totally incompatible with ES class (only work when compiled to ES5, which doesn't really keep class semantics).
In 1.5, you'll need to manually set the option to false
, as patch version is considered non-breaking.
I suggested the own
keyword in ^^^, which would use the new (set-property-to-undefined) behaviour. Might not work here, but thought to mention it.
@chriskrycho this doesn't have to be a breaking change. Right now the following code is considered valid (unless strictPropertyInitialization
, or more generally strict
, is true), even if strictNullChecks
is true:
class Foo {
public x: string;
public getX(): string {
return x;
}
}
But this can clearly cause runtime issues because getX
will return undefined
. It is no worse if x
is declared and assigned to undefined
automatically, since this is (approximately) the existing behaviour, with the same possible bugs.
I suggest declaring the property and setting it to undefined
if not specified in a constructor (to match the spec), but introduce no new warnings or errors. The existing strict
checks are already sufficient, and anybody who has those turned off is already facing the same possible runtime bugs.
You’re partly correct, but there are ways that people with strictNullChecks
but not strict
or strictPropertyInitialization
can be broken by this, precisely it’s breaking in terms of compiler output, not only in terms of type-checking. If you have a way that a property is known to be initialized outside the constructor (e.g. component arguments, which may be checked via debug assertions or the like), the behavior can change with this. We saw these breaking changes in practice across many applications when we changed to using TS only for type checking and Babel (which is spec compliant) for actual compilation in Ember apps and addons.
That said, deferring it to strictPropertyInitialization
or strict
doesn’t seem unreasonable, since it’s impossible for this not to be a breaking change in some way. (It should, however, be code-mod-able given a satisfactory design.)
Could I propose that only fields whose types include undefined
be initialized as such? It seems to me that initial example, where the fields were just removed, ought to be disallowed -- if the value is non-optional but also never set in a constructor, then that should just be a type error.
So this should be fine:
class Foo {
public x?: string; // initialized to undefined
}
and this should be an error:
class Foo {
public x: string; // TypeError -- x can't be undefined
}
Any news on this? This kind of behavior is as important as common and standard. Would be great to fix it.
What is the use case for this one?
undefined
is the spiritual equivalent of the lack of a property, thus:
type LolRly = { prop?: bool }
const A: LolRly = {}
const B: LolRly = { prop: undefined }
console.log(typeof A.prop) // undefined
console.log(typeof B.prop) // undefined
console.log(JSON.stringify(A)) // {}
console.log(JSON.stringify(B)) // {}
If you want to do some dirty, 5-minute life hacks with iterating over an object properties (and I can smell from a mile, that it's exactly what you're trying to do), then null
is for that to represent an existing property with en empty (NULLABLE) value.
Even if JS/TS can produce and allows the existence of an object with a property initialised with undefined
value, it should be treated as if it wasn't there in the first place.
@IgorSzymanski the difference, however, is observable, and it's important for it to be correct.
@ljharb I ask again for the use case.
Also, if you do not initialise and optional property, why do you expect it to be initialised with undefined? If you do not initialise an optional property, the expected behaviour is it's not there, uninitialised shouldn't be initialised, and that's exactly what happens.
Also, the OP example is horrible.
If you define a property as non-optional number
, it should be nothing else, but a number
. No part of the code says it can be undefined
.
Anything using object spread or Object.assign
will care about it being a present, own property, even if set to undefined
. undefined
is not simply "not defined", in JS, there exists "absent", "present but set to undefined", and "present but set to non-undefined".
Then initialise your properties explicitly. This is a breaking change, it breaks the logic behind explicit property initialisation and, once again, I do not see any use case where it should matter.
In OOP, you shouldn't really ask for properties of a class instance, because it breaks its encapsulation.
In FP, you wouldn't even use a class for this, and therefore you would have to initialise the property on your own.
This change is nothing but an attempt to encourage a bad programming style.
I also do not see, why you would want to run a class instance against Object.assign
or object spread. Mind to elaborate?
Give me a valid example of why the lack of implicit property initialisation in a class instance is an issue.
@IgorSzymanski this continual moving of the goalposts isn't indicative of a good-faith conversation. If you don't think it's important to you, that's fine, but no one is required to justify spec compliance as a valid goal
The reason I initially came across this issue, and wrote my comment above, is that I hit a case where an error was caused by this surprising logic. Unfortunately I cannot remember the exact example, but any similar example would be equivalent: if one writes
class Foo {
public x?: string;
}
and doesn't initialize the property, there should be _some_ sort of warning.
Otherwise
const foo = Foo()
console.log(foo.x); // typechecks, gives 'undefined'
console.log(foo.hasOwnProperty('x')); // gives 'false'
leads to confusion. For instance, sometimes one uses hasOwnProperty
to implement a typeguard:
function isFoo(foo: Foo | Bar): foo is Foo {
return foo.hasOwnProperty('x')
}
which fails if Foo
is defined as above, even though it might be an actual instance of Foo
.
Most helpful comment
(Public) class fields are enabled by default in Chrome 72 with the initializer semantics outlined above. This part of the spec is extremely unlikely to change. Current topics of debate are just how private fields should work AFAICT.
Can we please revisit and get a decision here? While the Babel transform is not officially supported, they would want to wait for a decision by the TypeScript team before implementing any fixes: https://github.com/babel/babel/issues/9105
The major arguments for keeping the current semantics to me are:
In favor of switching:
Any input would be much appreciated ☺️❤️