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 feature: computed property name #1037

Closed
ikarienator opened this Issue Feb 14, 2015 · 11 comments

Comments

Projects
None yet
3 participants
@ikarienator
Contributor

ikarienator commented Feb 14, 2015

Syntax:

ComputedPropertyName[Yield] :
    [ AssignmentExpression[In, ?Yield] ]

Referred to by:

PropertyName[Yield,GeneratorParameter] :
    LiteralPropertyName
    [+GeneratorParameter] ComputedPropertyName
    [~GeneratorParameter] ComputedPropertyName[?Yield]

Early errors:

None.

Spec:

12.2.5 Object Initializer

See also

Esprima issue on Google Code
V8 support for computed property names
Error messages in V8

@ikarienator

This comment has been minimized.

Show comment
Hide comment
@ikarienator

ikarienator Feb 14, 2015

Contributor

Continuing on the discussion, some issues:

  1. the value is alway computed. Therefore to me a computed flag does not feel like its referring to the value.
  2. type Literal | Identifier | Expression will be simply Expression.

My suggestion will be:

interface ObjectExpression <: Expression {
    type: "ObjectExpression";
    properties: [ Property ];
}

interface Property {
    kind: "init" | "get" | "set";
    name: Expression;
    computed: boolean;
    value: Expression;
}
Contributor

ikarienator commented Feb 14, 2015

Continuing on the discussion, some issues:

  1. the value is alway computed. Therefore to me a computed flag does not feel like its referring to the value.
  2. type Literal | Identifier | Expression will be simply Expression.

My suggestion will be:

interface ObjectExpression <: Expression {
    type: "ObjectExpression";
    properties: [ Property ];
}

interface Property {
    kind: "init" | "get" | "set";
    name: Expression;
    computed: boolean;
    value: Expression;
}
@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Feb 14, 2015

Member

@ikarienator is the proposal here diff from SpiderMonkey? If so, file an issue at estree too please?

Member

mikesherov commented Feb 14, 2015

@ikarienator is the proposal here diff from SpiderMonkey? If so, file an issue at estree too please?

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Feb 14, 2015

Contributor

@ikarienator I agree with @mikesherov. Since computed flag is already becoming almost the de-facto standard, we should stick with this for now and brainstorm the further enhancement at estree.

Contributor

ariya commented Feb 14, 2015

@ikarienator I agree with @mikesherov. Since computed flag is already becoming almost the de-facto standard, we should stick with this for now and brainstorm the further enhancement at estree.

@ariya ariya added this to the 2.1 milestone Feb 14, 2015

@ikarienator

This comment has been minimized.

Show comment
Hide comment
@ikarienator

ikarienator Feb 14, 2015

Contributor

This is only migrating the google code issue to here. I haven't checked estree yet.

Contributor

ikarienator commented Feb 14, 2015

This is only migrating the google code issue to here. I haven't checked estree yet.

@ikarienator

This comment has been minimized.

Show comment
Hide comment
@ikarienator

ikarienator Feb 14, 2015

Contributor

I checked estree and it does not include much es6 contents. It does not include computed property name among other features. Why do we want to keep sync with it? https://github.com/shapesecurity/shift-spec/tree/es6 has a very complete and up to date definition of ES6 AST and very few issues if any. It is described in a compilable WebIDL. We can probably be more compliant to that than estree.

Are we trying to make the ES6 AST additive to the ES5 one? I'm not sure it's possible.

Contributor

ikarienator commented Feb 14, 2015

I checked estree and it does not include much es6 contents. It does not include computed property name among other features. Why do we want to keep sync with it? https://github.com/shapesecurity/shift-spec/tree/es6 has a very complete and up to date definition of ES6 AST and very few issues if any. It is described in a compilable WebIDL. We can probably be more compliant to that than estree.

Are we trying to make the ES6 AST additive to the ES5 one? I'm not sure it's possible.

@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Feb 14, 2015

Member

Let's discuss at the meeting on Wednesday then. The idea is compat with acorn and spidermonkey, several of which have living breathing es6 implementations in current versions.

Member

mikesherov commented Feb 14, 2015

Let's discuss at the meeting on Wednesday then. The idea is compat with acorn and spidermonkey, several of which have living breathing es6 implementations in current versions.

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Feb 14, 2015

Contributor

@ikarienator Unfortunately that how's been implemented in the harmony branch so far. It seems to be less troublesome to support incremental addition, even it's not perfect, in the current situation.

Contributor

ariya commented Feb 14, 2015

@ikarienator Unfortunately that how's been implemented in the harmony branch so far. It seems to be less troublesome to support incremental addition, even it's not perfect, in the current situation.

@ikarienator

This comment has been minimized.

Show comment
Hide comment
@ikarienator

ikarienator Feb 14, 2015

Contributor

Make sense. I will create issues in estree to address missing features that
has been implemented in harmony and master. Michael, what is the problem
preventing us from having a maximally additive ES6 AST in shift spec?
On Sat, Feb 14, 2015 at 13:28 Ariya Hidayat notifications@github.com
wrote:

@ikarienator https://github.com/ikarienator Unfortunately that how's
been implemented in the harmony branch so far. It seems to be less
troublesome to support incremental addition, even it's not perfect, in the
current situation.


Reply to this email directly or view it on GitHub
#1037 (comment).

Contributor

ikarienator commented Feb 14, 2015

Make sense. I will create issues in estree to address missing features that
has been implemented in harmony and master. Michael, what is the problem
preventing us from having a maximally additive ES6 AST in shift spec?
On Sat, Feb 14, 2015 at 13:28 Ariya Hidayat notifications@github.com
wrote:

@ikarienator https://github.com/ikarienator Unfortunately that how's
been implemented in the harmony branch so far. It seems to be less
troublesome to support incremental addition, even it's not perfect, in the
current situation.


Reply to this email directly or view it on GitHub
#1037 (comment).

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Feb 15, 2015

Contributor

BTW, the more proper old bug to be referred is this one: https://code.google.com/p/esprima/issues/detail?id=480. The corresponding harmony branch implementation is in commit 2bb17ef.

Contributor

ariya commented Feb 15, 2015

BTW, the more proper old bug to be referred is this one: https://code.google.com/p/esprima/issues/detail?id=480. The corresponding harmony branch implementation is in commit 2bb17ef.

@ikarienator

This comment has been minimized.

Show comment
Hide comment
@ikarienator

ikarienator Feb 15, 2015

Contributor

Thanks! Interesting... I may want move computed to the end of the finishProperty method.

Contributor

ikarienator commented Feb 15, 2015

Thanks! Interesting... I may want move computed to the end of the finishProperty method.

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Feb 15, 2015

Contributor

There is also commit 54f49ada87, make sure we include the test case so that it is also covered.

Contributor

ariya commented Feb 15, 2015

There is also commit 54f49ada87, make sure we include the test case so that it is also covered.

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 15, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 17, 2015

ikarienator added a commit to ikarienator/esprima that referenced this issue Feb 18, 2015

@ariya ariya closed this in 772716a Feb 18, 2015

@ariya ariya added es6 and removed es6migration labels Feb 18, 2015

@mikesherov mikesherov referenced this issue Mar 7, 2015

Closed

ES6 Main Tracking Issue #1099

25 of 25 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment