Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Clarification on duplicate getter/setter definitions #14

Closed
jkrems opened this issue Jul 29, 2016 · 8 comments
Closed

Clarification on duplicate getter/setter definitions #14

jkrems opened this issue Jul 29, 2016 · 8 comments

Comments

@jkrems
Copy link

jkrems commented Jul 29, 2016

Let coalesced be the new List created by combining getters and setters with the same [[Key]]. If both a getter and setter with the same [[Key]] have a non-empty [[Decorators]], throw a new Error.

Is that paragraph meant to include cases where both a getter and a setter have a decorator but the coalesced record would only include one of them, e.g.:

class X {
  @f get a() {}
  get a() {}
  @f set a() {}
}

I'm not 100% sure what the expected coalesced list would be in this case, e.g. if f would be called once or twice and with what arguments.

@jkrems jkrems changed the title Clarification on duplicate property definitions Clarification on duplicate getter/setter definitions Jul 29, 2016
@jkrems
Copy link
Author

jkrems commented Jul 31, 2016

After looking into how V8 treats conflicting property declarations, I'd assume the above is not an error and causes f to be called exactly once with the second getter and the setter in the property descriptor. I also wrote up a more complex test case including a mix of get/set/method.

P.S.: Calling decorators of elements that would normally be shadowed by later declarations becomes relevant when a decorator changes the key.

@silkentrance
Copy link

silkentrance commented Aug 12, 2016

What about the parser/compiler throwing an error in case of duplicate method/getter/setter declaration in strict mode, which class declarations always are?

@jkrems
Copy link
Author

jkrems commented Aug 12, 2016

At least in V8 5.0 objects in strict mode can contain duplicate method definitions:

> node -v && node -p "(function() { 'use strict'; return { x() {}, x() {} }; })()"
v6.3.0
{ x: [Function: x] }

Also, since the first decorator can change they key, it's not even a given that there is a conflict. This can only be determined at runtime. I don't think it's possible to make this a compile time error.

@jkrems
Copy link
Author

jkrems commented Aug 12, 2016

Same for classes with duplicate methods:

> node -v && node -p "(function() { 'use strict'; return class X { x() {} x() {} }; })()"
v6.3.0
[Function: X]

@littledan
Copy link
Member

You can have duplicate methods with the later overwriting the earlier, but it was supposed to be an error to decorate getters and setters separtely (in ClassDefinitionEvaluation). I thought i wrote spec text for it, but I don't see it uploaded; I'll try to get something for this within a couple weeks.

@jkrems
Copy link
Author

jkrems commented Nov 28, 2017

Just to clarify: I think the same general problem would apply if multiple get of the same property are decorated. See: https://github.com/jkrems/babel-plugin-transform-decorators-stage-2-initial/blob/468cc8d21426568bbc75f0131c93ef57a73d66f6/test/language/decorators/coalesce#L21

It boils down to the fact that the spec wasn't specific about the exact algorithm to coalesce the getter/setter pair(s) before passing them into the decorator.

edited to make that last paragraph a little easier to parse

@littledan
Copy link
Member

Agreed. I thought I wrote out spec text clarifying coalescing but now I can't find it. Maybe I left it on my other computer. I will check after I return from traveling for TC39

@littledan
Copy link
Member

Continue tracking in tc39/proposal-decorators#52 .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants