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.

Sign up for GitHub

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

Not depending on return value of super(). Fixes #9020. #9060

Merged
merged 4 commits into from Dec 4, 2018
Merged

Not depending on return value of super(). Fixes #9020. #9060

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3a3925
Not depending on return value of super(). Fixes #9020.
joeldenning Nov 22, 2018
f3ecd00
Merge branch 'master' of github.com:babel/babel into issue-9020
joeldenning Dec 3, 2018
7d646b0
Feedback from nicolo-ribaudo
joeldenning Dec 3, 2018
5a223ac
Feedback -- fixing bad call to replaceWithMultiple
joeldenning Dec 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/babel-plugin-transform-arrow-functions/test/fixtures/arrow-functions/this/output.js
View file
Open in desktop
Expand Up @@ -15,9 +15,9 @@ class Foo extends function () {} {
};

if (true) {
console.log(_this2 = super(), foo());
console.log(super(), foo());
} else {
_this2 = super();
super();
console.log(foo());
}
}
Expand Down
9 changes: 7 additions & 2 deletions packages/babel-traverse/src/path/conversion.js
View file
Open in desktop
Expand Up @@ -461,8 +461,13 @@ function getThisBinding(thisEnvFn, inConstructor) {
if (supers.has(child.node)) return;
supers.add(child.node);

child.replaceWith(
t.assignmentExpression("=", t.identifier(thisBinding), child.node),
child.replaceWithMultiple(
Copy link
Member

Choose a reason for hiding this comment

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

It takes an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicolo-ribaudo oops! I just fixed that. I don't fully understand the expected output in some of the tests, so I didn't catch that the second argument was being ignored.

child.node,
t.assignmentExpression(
"=",
t.identifier(thisBinding),
t.identifier("this"),
),
);
},
});
Expand Down
12 changes: 6 additions & 6 deletions packages/babel-traverse/test/arrow-transform.js
View file
Open in desktop
Expand Up @@ -82,8 +82,8 @@ describe("arrow function conversion", () => {
() => this;
`,
`
var _supercall = (..._args) => _this = super(..._args),
_this;
var _supercall = (..._args) => super(..._args),
_this;

(function () {
_supercall(345);
Expand Down Expand Up @@ -115,9 +115,9 @@ describe("arrow function conversion", () => {
(function () {
_this;
});
_this = super();
super();
this;
() => _this = super();
() => super();
() => this;
`,
{ methodName: "constructor", extend: true },
Expand All @@ -144,9 +144,9 @@ describe("arrow function conversion", () => {

_this;
}).bind(_arrowCheckId);
_this = super();
super();
this;
() => _this = super();
() => super();
() => this;
`,
{
Expand Down