Skip to content

Disallow duplicated decorated methods#86

Merged
littledan merged 1 commit intotc39:masterfrom
nicolo-ribaudo:duplicated-decorated-methods
Apr 17, 2018
Merged

Disallow duplicated decorated methods#86
littledan merged 1 commit intotc39:masterfrom
nicolo-ribaudo:duplicated-decorated-methods

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

#84

This proposed patch disallows code where the decorator isn't applied to the function it's written before of, but to the last method function, which can be very confusing:

class A {
  @decorate
  method() {
    return 1;
  }

  method() {
    return 2;
  }
}

I didn't change the behavior of get/set pairs since this should still be possible:

class A {
  @decorate
  get method() {
    return 1;
  }

  set method(x) {

  }
}

Copy link
Copy Markdown
Member

@littledan littledan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this very clean specification of the semantics we discussed. Personally, this seems like a good approach to me, but I'd like to have feedback from other decorators co-champions first.

@littledan
Copy link
Copy Markdown
Member

We discussed this change with other decorator co-champions and decided it made sense. We might come back and weaken this, but honestly, any code which hits this runtime error is probably already buggy.

@littledan
Copy link
Copy Markdown
Member

littledan commented Apr 14, 2018

Nicolo, could you rebase this patch and resolve conflicts?

@nicolo-ribaudo nicolo-ribaudo force-pushed the duplicated-decorated-methods branch from a504e2f to ddd3920 Compare April 17, 2018 21:39
@nicolo-ribaudo
Copy link
Copy Markdown
Member Author

Rebased

@littledan littledan merged commit 444d0b3 into tc39:master Apr 17, 2018
@nicolo-ribaudo nicolo-ribaudo deleted the duplicated-decorated-methods branch April 18, 2018 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants