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

Ignore abstract methods when decorating class #11345

Merged

Conversation

oliverdunk
Copy link
Contributor

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

Description

Implementations of abstract methods were not callable on classes, when the abstract class being implemented contained a decorated field.

The bug

When compiling a decorated class, we replace the classes definition with a call to a helper. This helper takes an array of objects, each representing something it needs to define in the class it builds.

To generate one of these objects, we take the node, and pass it through extractElementDescriptor. The object this function generates includes a type key, which describes which type of definition the helper needs to add. This is set using isMethod ? node.kind : "field".

We were including abstract methods in the set of nodes that we were generating objects from. Since isMethod was false for these, we were telling the decorate helper to produce a field. This resulted in the abstract class defining a field, whose name overlapped with the method being implemented. As a result, the method could not be accessed.

My fix

Abstract methods are filtered out, so the decorate helper no longer tries to define them.

@nicolo-ribaudo nicolo-ribaudo added area: typescript PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators labels Mar 28, 2020
@nicolo-ribaudo
Copy link
Member

@oliverdunk Could you try rebasing and re-generating output.js? The tests are failing because we recently (7.9.0) modified the _iterableToArray, causing a difference in the output for this test.

@oliverdunk oliverdunk force-pushed the 10514-abstract-methods-decorators-bug branch from 738b71d to 0f6c195 Compare March 28, 2020 22:09
@oliverdunk
Copy link
Contributor Author

@oliverdunk Could you try rebasing and re-generating output.js? The tests are failing because we recently (7.9.0) modified the _iterableToArray, causing a difference in the output for this test.

No problem, done.

@nicolo-ribaudo nicolo-ribaudo merged commit 71e7a7b into babel:master Mar 28, 2020
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

preset-typescript + decorator on abstract base class overwrites members with undefined
3 participants