Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ES6 Computed Properties #1082

Closed
JsonFreeman opened this issue Nov 6, 2014 · 19 comments
Closed

ES6 Computed Properties #1082

JsonFreeman opened this issue Nov 6, 2014 · 19 comments
Assignees
Labels
Spec Issues related to the TypeScript language specification

Comments

@JsonFreeman
Copy link
Contributor

ES6 allows an arbitrary expression as the name of a property. The syntax is like this:

var x = {
   ["some" + "arbitrary" + "name"]: 0
}

Here is my current proposal for supporting this feature:

Computed expressions would be allowed in the following places if target is ES6:

  • Object literal properties
  • Class methods and accessors (both static and instance), but not property declarations.
  • 'this' references are not allowed in computed properties.
  • No parameter properties
  • Do not allow in interfaces
  • No enum members

Below ES6:

  • Computed properties are not allowed

Note that emit for all of the above is straightforward.

Type check:

  • All computed names must be of type string, number, or any (or eventually symbol)
  • No property will be added to the containing type based on a computed name.
  • Computed names that are not well known do not add a property to the type, but they will be included in the union type for the indexer, if the containing type has an indexer.

Questions:

  • Should an unknown name cause the surrounding type to have an indexer, if it wouldn't have acquired one otherwise? Our decision here is 'no'. Here is an argument for and against:

Argument for - we would want this to work:

var n = "some " + "unknown " + "name";
var obj = {
    [n]: 0;
};
var num = obj[n]; // This should be number

Argument against - Computed properties would be allowed in class properties, so someone might have a class hierarchy like this:

var n = "some " + "unknown " + "name";
class C {
    [n]: number;
}
class D extends C {
   foo: string; // Error because C was given an implicit indexer that D inherited, but foo doesn't conform to the indexer
}
@JsonFreeman JsonFreeman added the Spec Issues related to the TypeScript language specification label Nov 6, 2014
@yuit
Copy link
Contributor

yuit commented Nov 7, 2014

I think the down-level should be apply or not apply to both classes and object literals. However, down-level one but not the other seems confusing and asymmetric to me.

Some question

  • For the enum member, is there a reason why we won't support the feature or is it just not part of ES6 spec?
  • for below ES6, allowing in an interface will only be a well known name as well ? In the case, how we gonna emit them?

@JsonFreeman
Copy link
Contributor Author

@yuit Your point about the symmetry of down leveling is well taken.

To answer your questions:

  • Firstly, enums are not in ES6, so this decision is owned completely by us. Enum members with constant values are inlined though, and our inlining requires us to know exactly which member we are referencing. Computed names would produce an enum member that cannot be inlined. Plus, it is really not useful to have a computed name in an enum.
  • We never emit interfaces, so we would never emit the computed names of interfaces. Therefore, it is pointless to have a computed name in an interface, unless it has a well known value and can contribute to the type of the interface. This is independent of which EcmaScript version we are targeting.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 7, 2014

Here is a proposal for how to emit them:

  • interfaces, we only have .d.ts to worry about, and for these, you emit them as is, but ensure that all identifiers in the expression are exported as well.
  • Classes
declare var staticProperty: string;
declare var instanceProperty: string;
declare var staticMethod: string;
declare var instanceMethod: string;
declare var instanceAccessor: string;
declare var staticAccessor: string;

class C {
    static [staticProperty]: string = "a";
    private [instanceProperty] : string = "i";
    static [staticMethod]() { }
    [instanceMethod]() { }
    get [instanceAccessor] () { return 0 }
    get [staticAccessor] () { return 0 }
}

results in:

var C = (function () {
    var _instanceProperty;
    function C() {
        this[_instanceProperty] = "i";
    }
    C[staticProperty] = "a";
    _instanceProperty = instanceProperty;    // capture the value of the expression
    C[staticMethod] = function () {  };
    C.prototype[instanceMethod] = function () { };
    Object.defineProperty(C.prototype, instanceAccessor, {
        get: function () {
            return 0;
        },
        enumerable: true,
        configurable: true
    });
    Object.defineProperty(C.prototype, staticAccessor, {
        get: function () {
            return 0;
        },
        enumerable: true,
        configurable: true
    });

    return C;
})();

Notes:

  • A computed property has to be initialized, a non initialized computed property is considered an error
  • 'this' is not allowed in computed properties expression
  • Emitting properties wither static or instance needs to happen in the order of declaration, to allow for using a previous member of the static side of the class as a key
  • Object literals
declare function func(a: any): void;
declare var computed: string;

// Object literal
func({
    before: 0,
    [computed]: 1, 
    after : 2
});
var _$1 = {
    before: 0
}, _$1[computed] = 1 ,
   _$1.after = 2;
func(_$1);

@mhegazy
Copy link
Contributor

mhegazy commented Nov 7, 2014

A few more notes:

Well known computed names (like string literals, number literals, and eventually symbols) will cause a property to be added to the type that contains it. Example:

What is the value of enabling this?

// This looks silly
var x = { ["name"] : 0 } 

-Do not allow in interfaces, unless it is a well known name (#980)

This should also include object type literals

Computed names that are not well known do not add a property to the type, but they will be included in the union type for the indexer, if the containing type has an indexer.

I am not sure i understand that, do you mean "checked against"?

@JsonFreeman
Copy link
Contributor Author

  • Why add a property for well known names: This is more of a precursor to the symbol work. The idea is that if we have full symbol support, then when the user references a well known symbol, we want to add a property. I am just generalizing that to apply to any well known name, symbol or not. However, I am open to the idea that this generalization might not be useful, and we can decide to skip it. Because without symbols, it is pretty pointless. I will add that if we do not add this generalization, then we should not support computed names at all in interfaces / object types (even well known ones).
  • Good point about disallowing 'this' in computed properties in classes. Unless we decide that the instance side computed properties can be evaluated in the constructor, but that is not the current design.
  • I do not think we have to forbid uninitialized computed properties in a class. We can still emit the expression, and assign it to a local var in the class closure. The reason this is useful is that when we have a well known name, we would still want to add the property to the type even if the user is not ready to initialize the property yet.
  • If an indexer type is computed as the union of properties, we include the property type in the union. If the indexer already exists, we have to check the computed properties against the indexer type.

@JsonFreeman
Copy link
Contributor Author

Also, I don't think the following emit will work:

var _$1 = {
    before: 0
}, _$1[computed] = 1 ,
   _$1.after = 2;
func(_$1);

Recall that a comma in a variable statement actually separates variable declarations from each other, so you need to use semicolons instead:

var _$1 = {
    before: 0
};
_$1[computed] = 1;
_$1.after = 2;
func(_$1);

@JsonFreeman
Copy link
Contributor Author

Some updates:

  • We may want to only support adding a property for well known names for symbols and constant enum members. The string and number literal case is truly useless.
  • We will allow only well known names in uninitialized class properties (both instance and static). We will basically allow the same thing as we allow in an interface.

@JsonFreeman
Copy link
Contributor Author

I'm updating this with the following changes:

  • Computed properties not allowed in interfaces, but eventually well known symbols will be allowed.
  • Ditto for uninitialized properties in a class.
  • Will downlevel for object literals and classes.
  • 'this' is not allowed in class property names (neither static nor instance)

@JsonFreeman
Copy link
Contributor Author

Updating a few things:

  • No downlevel for computed properties (for now at least, but we may add it later)
  • Computed property declarations are not allowed in classes
  • Computed properties do not add an indexer to a type

I will update the proposal above to reflect these.

@JsonFreeman
Copy link
Contributor Author

Here is an interesting question. One thing that seems apparent is that we should check that the computed properties in a class conform to the indexer declared by that class. It seems equally natural to do this if the indexer is inherited. The question is, what about inherited computed properties? These properties never actually appear on the type, so one can argue that they do not get inherited. On the other hand, we know they are there!

class Base {
    ["name"]() { }
}
class Derived extends Base {
    [s: string]: number; // Error?
}

@JsonFreeman
Copy link
Contributor Author

Assigning to @ahejlsberg for spec integration.

@danquirk
Copy link
Member

Could we just close this and make a new issue for spec work if this one was just to track the dev work?

@JsonFreeman
Copy link
Contributor Author

Opened a separate issue to track the spec work.

@ahejlsberg
Copy link
Member

@JsonFreeman @mhegazy Regarding downlevel emit for computed properties, for the example

func({ before: 0, [computed]: 1, after : 2 });

we could use temporary variables and the comma operator to emit

func((_a = { before: 0 }, _a[computed] = 1, _a.after = 2, _a));
var _a;

This would work in any expression context and we already have the infrastructure in place to track and emit the temporary variable declaration (from my work on downlevel destructuring).

@JsonFreeman
Copy link
Contributor Author

Yes, that is a good idea. I will open two issues, one for downlevelling computed properties, and one for allowing them in class property declarations.

@JsonFreeman
Copy link
Contributor Author

@ahejlsberg I opened #1840 and #1841

@noggin182
Copy link

Can we have an explanation as to why we disallow 'this' ? I believe the following code is valid ES6 and it worked in my limited testing (only tested chrome using the console).

toObject() {
    let o = {
        [this.idField]: 12,
        name: "dummy"
    };
    return o;
}

I can understand this may not be possible in some contexts such as member initialisers or interfaces, however why is such usage blocked when composing objects within functions?

@saschanaz
Copy link
Contributor

saschanaz commented Aug 9, 2016

@noggin182

The following code is valid TypeScript. Maybe it was fixed after the original proposal.

class Foo {
    idField: number;

    toObject() {
        let o = {
            [this.idField]: 12,
            name: "dummy"
        };
        return o;
    }
}

PS: It seems the this reference restriction is only for classes.

@noggin182
Copy link

noggin182 commented Aug 9, 2016

Thanks @saschanaz. After some investigation it seems that this error is coming from ReShaper. I initially ruled our ReSharper as there was no options to ignore the error or configure the inspection severity. I'll open a new ticket with them.

Update: Reported here: https://youtrack.jetbrains.com/issue/RSRP-460192

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Spec Issues related to the TypeScript language specification
Projects
None yet
Development

No branches or pull requests

7 participants