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

fix: Throw error when compiling super() in arrow functions with default / rest parameters #15163

Merged
merged 4 commits into from Nov 22, 2022
Merged

fix: Throw error when compiling super() in arrow functions with default / rest parameters #15163

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
073b3b0
Fix: Throw when super in arrow with default / rest
SuperSodaSea Nov 8, 2022
c66111b
Add allowInsertArrowWithRest and improve throw
SuperSodaSea Nov 21, 2022
db066ab
Merge branch 'main' into fix/15148
SuperSodaSea Nov 21, 2022
fd549b3
Update packages/babel-traverse/src/path/conversion.ts
SuperSodaSea Nov 22, 2022
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
1 change: 1 addition & 0 deletions packages/babel-plugin-transform-parameters/package.json
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
},
"main": "./lib/index.js",
"dependencies": {
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-plugin-utils": "workspace:^"
},
"keywords": [
Expand Down
19 changes: 19 additions & 0 deletions packages/babel-plugin-transform-parameters/src/index.ts
View file
Open in desktop
@@ -1,8 +1,25 @@
import { traverse } from "@babel/core";
import environmentVisitor from "@babel/helper-environment-visitor";
import { declare } from "@babel/helper-plugin-utils";
import convertFunctionParams from "./params";
import convertFunctionRest from "./rest";
export { convertFunctionParams };

const checkSuperCallsVisitor = traverse.visitors.merge<{}>([
{
CallExpression(child) {
if (child.get("callee").isSuper()) {
// NOTE: this may happen in `@babel/preset-env` with target `since 2017`, or in the default setting of `create-react-app`.
throw child.buildCodeFrameError(
"It's not possible to compile `super()` in an arrow function with default or rest parameters without compiling classes.\n" +
"Please add '@babel/plugin-transform-classes' to your Babel configuration.",
);
}
},
},
environmentVisitor,
]);

export interface Options {
loose?: boolean;
}
Expand All @@ -26,6 +43,8 @@ export default declare((api, options: Options) => {
.get("params")
.some(param => param.isRestElement() || param.isAssignmentPattern())
) {
path.traverse(checkSuperCallsVisitor);

// default/rest visitors require access to `arguments`, so it cannot be an arrow
path.arrowFunctionToExpression({ noNewArrows });

Expand Down
6 changes: 6 additions & 0 deletions ...nsform-parameters/test/fixtures/parameters/arrow-default-super-no-classes-plugin/input.js
View file
Open in desktop
@@ -0,0 +1,6 @@
class Bar extends Foo {
constructor() {
let f = (x = super()) => x;
f();
}
}
4 changes: 4 additions & 0 deletions ...rm-parameters/test/fixtures/parameters/arrow-default-super-no-classes-plugin/options.json
View file
Open in desktop
@@ -0,0 +1,4 @@
{
"plugins": ["transform-parameters"],
"throws": "It's not possible to compile `super()` in an arrow function with default or rest parameters without compiling classes.\nPlease add '@babel/plugin-transform-classes' to your Babel configuration."
}
6 changes: 6 additions & 0 deletions ...transform-parameters/test/fixtures/parameters/arrow-rest-super-no-classes-plugin/input.js
View file
Open in desktop
@@ -0,0 +1,6 @@
class Bar extends Foo {
constructor() {
let f = (...args) => super(...args);
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the input here is very similar to how we would transform super():

babel/packages/babel-traverse/test/arrow-transform.js

Lines 53 to 71 in 9862d42

assertConversion(
`
() => {
super(345);
};
super();
() => super();
`,
`
var _supercall = (..._args) => super(..._args);
(function () {
_supercall(345);
});
_supercall();
() => _supercall();
`,
{ methodName: "constructor", extend: true },
);

My gut is that it should have been fixed in how we generate the super call:

babel/packages/babel-traverse/src/path/conversion.ts

Line 652 in 9862d42

function getSuperBinding(thisEnvFn: NodePath<t.Function>) {

We already have an arrow function check when constructing supercall,

babel/packages/babel-traverse/src/path/conversion.ts

Lines 287 to 291 in 9862d42

if (!allowInsertArrow) {
throw superCalls[0].buildCodeFrameError(
"Unable to handle nested super() usage in arrow",
);
}

somehow it does not throw, probably because allowInsertArrow is not properly passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we can add allowInsertArrow: false here?

babel/packages/babel-plugin-transform-parameters/src/index.ts

Lines 48 to 49 in 073b3b0

// default/rest visitors require access to `arguments`, so it cannot be an arrow
path.arrowFunctionToExpression({ noNewArrows });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to add allowInsertArrow: false and it fails on #13939 related tests...:thinking:
Maybe we need something like allowInsertArrowWithRest?

    × 13939
    × 13939 complex
    × 13939 private
    × 13939 private complex

SyntaxError: babel/packages/babel-plugin-transform-parameters/test/fixtures/regression/13939/input.js: Unable to transform arrow inside class property
      1 | class A {
    > 2 |   handle = (x = 0) => {
        |            ^
      3 |     console.log(x);
      4 |   };
      5 | }

...

f();
}
}
4 changes: 4 additions & 0 deletions ...sform-parameters/test/fixtures/parameters/arrow-rest-super-no-classes-plugin/options.json
View file
Open in desktop
@@ -0,0 +1,4 @@
{
"plugins": ["transform-parameters"],
"throws": "It's not possible to compile `super()` in an arrow function with default or rest parameters without compiling classes.\nPlease add '@babel/plugin-transform-classes' to your Babel configuration."
Copy link
Contributor

Choose a reason for hiding this comment

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

This error message is way better than Unable to handle nested super() usage in arrow, we can improve that message if we figure out how to make that work.

}
1 change: 1 addition & 0 deletions yarn.lock
View file
Open in desktop
Expand Up @@ -2862,6 +2862,7 @@ __metadata:
resolution: "@babel/plugin-transform-parameters@workspace:packages/babel-plugin-transform-parameters"
dependencies:
"@babel/core": "workspace:^"
"@babel/helper-environment-visitor": "workspace:^"
"@babel/helper-plugin-test-runner": "workspace:^"
"@babel/helper-plugin-utils": "workspace:^"
peerDependencies:
Expand Down