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

Throw if an initializer would be decorated #188

Merged

Conversation

nicolo-ribaudo
Copy link
Member

Ref: #177 (comment)

This PR makes it an error to have code like this:

class A {
  @decorator
  @decoratorWhichReplacesTheMethodWithAnInitializer
  foo() {}
}

so that decorators don't have to handle initializers as input (since they can't be created from syntax)

@ljharb
Copy link
Member

ljharb commented Dec 16, 2018

I’m a bit confused; don’t decorators already have to be able to handle an initializer as input, since you can decorate a public field?

@nicolo-ribaudo
Copy link
Member Author

Since we will change the name of initializers, we shouldn't associate them with fields.
In #177 (comment) Daniel proposed (and noone disagreed) that we should throw if a finisher is passed to a subsequent decorator. For the same reason, we should throw for initializers.

@ljharb
Copy link
Member

ljharb commented Dec 16, 2018

ahh, gotcha - so you're saying when a decorator would otherwise receive as input a "no-op" descriptor (an initializer/effect/etc), it throws. That makes sense :-) the name "initializer" confused me :-D

Copy link
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.

We can change the name of initializer in a follow-on patch.

Thanks for this! Not sure why I thought it would be more complicated.

@littledan littledan merged commit 53d648d into tc39:master Dec 16, 2018
@nicolo-ribaudo nicolo-ribaudo deleted the disallow-decorated-initializer branch December 16, 2018 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants