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

[legacy decorators] Allow decorating generator methods #9912

Conversation

@nicolo-ribaudo
Copy link
Member

commented Apr 26, 2019

The old proposal used LeftHandSideExpression (instead of
AssignmentExpression) to satisfy this usecase:
wycats/javascript-decorators@e240cbc

Q                       A
Fixed Issues? Fixes #9852
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR will break the code of people doing things like

class A {
  @foo && bar
  method() {}
}

I don't think that it is a problem because:

  1. I have never seen that pattern anywhere
  2. Prettier inserts parentheses, which still work (@(foo && bar))
  3. The "legacy" proposal disallows that code.
[legacy decorators] Allow decorating generator methods
The old proposal used LeftHandSideExpression (instead of
AssignmentExpression) to satisfy this usecase:
wycats/javascript-decorators@e240cbc
@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Apr 26, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/10739/

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Apr 26, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/10738/

@@ -0,0 +1,4 @@
class Foo {
@deco
*generatorMethod() {}

This comment has been minimized.

Copy link
@NullVoxPopuli

NullVoxPopuli Apr 27, 2019

YAAAAAS. This'll make ember-concurrency beautiful!

@Alonski
Alonski approved these changes May 3, 2019
@danez

This comment has been minimized.

Copy link
Member

commented May 3, 2019

This PR will break the code of people doing things like

class A {
  @foo && bar
  method() {}
}

Just to understand this correctly: it wasn't allowed in legacy decorators, but it was in the old "new" implementation, but now in the "newest" it is disallowed?

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

It was allowed in a very early version of legacy decorators. It is disallowed in the "last" version of legacy decorators, in the "old new (a few months ago)" implementation and in the new not-yet-implemented version. Other than that, it is also disallowed by Flow and TypeScript.

@robclancy

This comment has been minimized.

Copy link

commented May 13, 2019

Is this going to be in the next release? It's a pretty big issue requiring ugly workarounds if needing decorators for generators. A lot of people are waiting on this.

@nicolo-ribaudo nicolo-ribaudo added this to the v7.5.0 milestone May 14, 2019

buschtoens added a commit to machty/ember-concurrency-decorators that referenced this pull request Jun 4, 2019
buschtoens added a commit to machty/ember-concurrency-decorators that referenced this pull request Jun 4, 2019
buschtoens added a commit to machty/ember-concurrency-decorators that referenced this pull request Jun 4, 2019
@hzoo
hzoo approved these changes Jun 12, 2019
Copy link
Member

left a comment

nice fix!

maybe also document this on the docs page?

@nicolo-ribaudo nicolo-ribaudo merged commit 8bf9714 into babel:master Jun 30, 2019

5 checks passed

babel/repl REPL preview is available
Details
buildsize No significant change
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 87.37% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nicolo-ribaudo nicolo-ribaudo deleted the nicolo-ribaudo:legacy-decorators/01-generator-method branch Jun 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.