microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101k stars 12.48k forks source link

Add support for prototype-based property default values #15607

Closed Fruneau closed 7 years ago

Fruneau commented 7 years ago

Currently when defining a class, the default values assigned to property are set by the constructor of the class:

class A {
    map: { [key: string]: string } = {};
}

Generates the following code:

class A {
    constructor() {
        this.map = {};
    }
}

This has its advantages: each instance of the class has its own instance of the field. However, it also has some surprising effects when the value is the result of a more complex expression, or in case we want the variable to refer to a shared instance. As a consequence, writing code that share an instance requires either to use a separate variable or manually patch the prototype after creating the class:

Using a separate variable:

const sharedMap: { [key: string]: string } = {};

class A {
    map = sharedMap;
}

Manually patching the prototype:

class A {
    map: { [key: string]: string };
}
A.prototype.map = {}

My proposal would be to add a new property modifier that would define a default value as shared between the instances.

class A {
    prototype map: { [key: string]: string } = {}
}

That code would be equivalent to the "Manual patching" case. The new keyword prototype could only be used on variables with a default value since it would only affect the storage of the default value.

kitsonk commented 7 years ago

That is very dangerous with non-primitive values. That is why things like this shouldn't be on the prototype.

In your particular example, each instance would be sharing the same map object.

const a1 = new A();
const a2 = new A();

a1.map['foo'] = 'bar';
console.log(a2.map['foo']); // would log 'bar'

The current implementation is pretty close to the semantics proposed by public fields in ECMAScript. There is no real good reason to change it and actually quite dangerous as demonstrated above.

Also, I re-read you actually wanted that... those would be called static properties and are already available.

Fruneau commented 7 years ago

@kitsonk I do understand that. That's why my proposal is purely additive and purely opt-in by the user. In some cases you really want to share an object between instances of the same class.

Static is not strictly equivalent to prototype variables since static are members of the constructor, not of the object, while properties of the prototype are actually accessible in the object.

class A {
     static map = "toto";
     prototype map = 42;
};

const a = new A();
console.log(a.map); // 42

Moreover, prototype variables would be overloadable by the instance.

a.map = 52; // valid since map remains a property of the class, even if its default value is not in the instance but in the prototype

console.log(a.map); // 52
kitsonk commented 7 years ago

Is there a compelling reason to break with these two TypeScript design goals?

6) Align with current and future ECMAScript proposals. 8) Avoid adding expression-level syntax.

It still leads to very confusing behaviour. For example:

class A {
}
A.prototype.map = {};
A.prototype.foo = 'bar';

const new a1 = new A();
const new a2 = new A();

a1.map['foo'] = 'baz';
a1.foo = 'baz';

console.log(a2.map['foo']); // logs 'baz'
console.log(a2.foo); // logs 'bar'
Fruneau commented 7 years ago

Well, the proposal add no expression-level syntaxes, it only deals with field declaration, and only for field declarations that contain a default value. In that case the behavior is opt-in, meaning it is only confusing for those who chose that behavior.

About the alignment with ECMAScript, the change is additive to the very last draft of ECMAScript, so it does not break valid or future ECMAScript code.

BTW, TypeScript allows "safely" patching the prototype by hand, safely in the sense that the prototype is properly typed. So I'm not proposing to create a new programming pattern that was previously forbidden, just to add a simpler syntax to do it.

class A {
    foo: number;
}

A.prototype.foo = "toto"; // Compilation Error because string is not assignable to number
A.prototype.foo = 12; // OK

So my point is nothing more than having a shorthand to do things that are quite common patterns (adding stuff to prototype is not something I would consider esoteric in the JS world). I would not be the only one to need it. See #1250.

kitsonk commented 7 years ago

Which was closed by design. So this is effectively a duplicate.

Fruneau commented 7 years ago

1250 is a support request "I wan't to do it, but it does not work as expected, is there a way to do it". Here I just propose a small feature addition to the language to solve that issue.

I understand you consider the proposal worthless. OK, got it. If you consider it should be closed as a duplicate, or because this is not an acceptable feature for the language, just do it.

mhegazy commented 7 years ago

There is a discussion about this going on in TC39, see https://github.com/erights/Orthogonal-Classes. I would recommend contributing to this discussion instead. For TypeScript we would not want to add such a concept unless we know the committee intends to adopt it to avoid conflicts with future versions of ECMAScript. If the orthogonal classes proposal, or a future modified version of it, end up approved by the committee, TypeScript will add support to it.

Fruneau commented 7 years ago

Thx for the pointer.

mhegazy commented 7 years ago

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.