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

Fix super method call in private instance method calling overridden method #9801

Merged
merged 3 commits into from Mar 31, 2019

Conversation

@MattiasBuelens
Copy link
Contributor

commented Mar 31, 2019

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

Previously, we transpiled the following code:

class Sub extends Base {
  #privateMethod() {
    return super.superMethod();
  }
}

to:

var _privateMethod2 = function _privateMethod2() {
  return babelHelpers.get(babelHelpers.getPrototypeOf(this), "superMethod", this).call(this);
};

This is incorrect: the lookup happens on the wrong prototype. The prototype chain of this looks something like:

this ---> Sub.prototype ---> Base.prototype ---> Object.prototype ---> null

getPrototypeOf(this) === Sub.prototype, so we look up Sub.prototype.superMethod. This may be overridden by Sub, so it's incorrect for the super call.

With this PR, the transpiled code becomes:

var _privateMethod2 = function _privateMethod2() {
  return babelHelpers.get(babelHelpers.getPrototypeOf(Sub.prototype), "superMethod", this).call(this);
};

getPrototypeOf(Sub.prototype) === Base.prototype, so we look up Base.prototype.superMethod. This is the expected method for the super call.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Mar 31, 2019

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

1 similar comment
@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Mar 31, 2019

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

@nicolo-ribaudo nicolo-ribaudo added this to In progress in Class features via automation Mar 31, 2019
@jridgewell jridgewell merged commit 3c11a4a into babel:master Mar 31, 2019
5 checks passed
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.29% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Class features automation moved this from In progress to Done Mar 31, 2019
@MattiasBuelens MattiasBuelens deleted the MattiasBuelens:fix-9788 branch Apr 1, 2019
@MattiasBuelens

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

You might want to remove the "needs triage" label on #9788, seeing that it's fixed now. 😉

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