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

Jump to bottom

Don't duplicate the base class when using constantSuper #13303

Merged
merged 1 commit into from May 14, 2021
Merged

Don't duplicate the base class when using constantSuper #13303

merged 1 commit into from May 14, 2021

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented May 12, 2021 •
edited by gitpod-io bot

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

@nicolo-ribaudo nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Class Fields area: assumptions labels May 12, 2021
nicolo-ribaudo
nicolo-ribaudo commented May 12, 2021
View reviewed changes
...oposal-class-properties/test/fixtures/assumption-constantSuper/complex-super-class/output.js
Comment on lines +1 to +5
var _B;

class A extends (_B = class B {}) {}

babelHelpers.defineProperty(A, "x", _B.x);
Copy link
Member Author

Choose a reason for hiding this comment

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

The output was

class A extends class B {} {}

babelHelpers.defineProperty(A, "x", (class B {}).x);

which is wrong if the extends clause has side effects or if it has identity.

@babel-bot
Copy link
Collaborator

babel-bot commented May 12, 2021 •
edited

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 12, 2021 •
edited

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f52b06b:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

JLHwung
JLHwung approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

For reference, Babel works as expected when the assumption constantSuper is not enabled (default).

@nicolo-ribaudo nicolo-ribaudo merged commit 989aac2 into babel:main May 14, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the fix-constantSuper branch May 14, 2021 22:02
This was referenced May 26, 2021
Closed
Closed
Closed
@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 Aug 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@existentialism existentialism existentialism approved these changes

@JLHwung JLHwung JLHwung approved these changes

Assignees
No one assigned
Labels
area: assumptions 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: Class Fields
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nicolo-ribaudo @babel-bot @existentialism @JLHwung