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

Implement @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining #13009

Merged
merged 7 commits into from Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 Gulpfile.mjs
Expand Up @@ -426,6 +426,7 @@ const libBundles = [
"packages/babel-plugin-proposal-optional-chaining",
"packages/babel-preset-typescript",
"packages/babel-helper-member-expression-to-functions",
"packages/babel-plugin-bugfix-v8-spread-parameters-in-optional-chaining",
].map(src => ({
src,
format: "cjs",
Expand Down
15 changes: 12 additions & 3 deletions lib/babel-packages.js.flow
Expand Up @@ -166,7 +166,9 @@ declare module "@babel/helper-function-name" {
}

declare module "@babel/helper-split-export-declaration" {
declare export default function splitExportDeclaration(exportDeclaration: any): any;
declare export default function splitExportDeclaration(
exportDeclaration: any
): any;
}

declare module "@babel/traverse" {
Expand Down Expand Up @@ -196,6 +198,13 @@ declare module "@babel/highlight" {
/**
* Highlight `code`.
*/
declare export default function highlight(code: string, options?: Options): string;
declare export { getChalk, shouldHighlight };
declare export default function highlight(
code: string,
options?: Options
): string;
declare export { getChalk, shouldHighlight };
}

declare module "@babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining" {
declare module.exports: any;
}
@@ -0,0 +1,3 @@
src
test
*.log
@@ -0,0 +1,19 @@
# @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining

> Transform optional chaining operators to workaround a [v8 bug](https://crbug.com/v8/11558).

See our website [@babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining](https://babeljs.io/docs/en/babel-plugin-bugfix-v8-spread-parameters-in-optional-chaining) for more information.

## Install

Using npm:

```sh
npm install --save-dev @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining
```

or using yarn:

```sh
yarn add @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining --dev
```
@@ -0,0 +1,38 @@
{
"name": "@babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining",
"version": "7.13.8",
"description": "Transform optional chaining operators to workaround https://crbug.com/v8/11558",
"repository": {
"type": "git",
"url": "https://github.com/babel/babel.git",
"directory": "packages/babel-plugin-bugfix-v8-spread-parameters-in-optional-chaining"
},
"homepage": "https://babel.dev/docs/en/next/babel-plugin-bugfix-v8-spread-parameters-in-optional-chaining",
"license": "MIT",
"publishConfig": {
"access": "public"
},
"main": "lib/index.js",
"exports": {
".": [
"./lib/index.js"
]
},
"keywords": [
"babel-plugin",
"bugfix"
],
"dependencies": {
"@babel/helper-plugin-utils": "workspace:^7.13.0",
"@babel/helper-skip-transparent-expression-wrappers": "workspace:^7.12.1",
"@babel/plugin-proposal-optional-chaining": "workspace:^7.13.8"
},
"peerDependencies": {
"@babel/core": "^7.13.0"
},
"devDependencies": {
"@babel/core": "workspace:*",
"@babel/helper-plugin-test-runner": "workspace:*",
"@babel/traverse": "workspace:*"
}
}
@@ -0,0 +1,22 @@
import { declare } from "@babel/helper-plugin-utils";
import { transform } from "@babel/plugin-proposal-optional-chaining";
import { shouldTransform } from "./util";

export default declare(api => {
api.assertVersion(7);

const noDocumentAll = api.assumption("noDocumentAll");
const pureGetters = api.assumption("pureGetters");

return {
name: "bugfix-v8-spread-parameters-in-optional-chaining",

visitor: {
"OptionalCallExpression|OptionalMemberExpression"(path) {
if (shouldTransform(path)) {
transform(path, { noDocumentAll, pureGetters });
}
},
},
};
});
@@ -0,0 +1,59 @@
import { skipTransparentExprWrappers } from "@babel/helper-skip-transparent-expression-wrappers";
import type { NodePath } from "@babel/traverse";
import { types as t } from "@babel/core";
// https://crbug.com/v8/11558

// check if there is a spread element followed by another argument.
// (...[], 0) or (...[], ...[])

function matchAffectedArguments(argumentNodes) {
const spreadIndex = argumentNodes.findIndex(node => t.isSpreadElement(node));
return spreadIndex >= 0 && spreadIndex !== argumentNodes.length - 1;
}

/**
* Check whether the optional chain is affected by https://crbug.com/v8/11558.
* This routine MUST not manipulate NodePath
*
* @export
* @param {(NodePath<t.OptionalMemberExpression | t.OptionalCallExpression>)} path
* @returns {boolean}
*/
export function shouldTransform(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This routine determine whether we should transpile the affected optional chain.

path: NodePath<t.OptionalMemberExpression | t.OptionalCallExpression>,
): boolean {
let optionalPath = path;
const chains = [];
while (
optionalPath.isOptionalMemberExpression() ||
optionalPath.isOptionalCallExpression()
) {
const { node } = optionalPath;
chains.push(node);

if (optionalPath.isOptionalMemberExpression()) {
optionalPath = skipTransparentExprWrappers(optionalPath.get("object"));
} else if (optionalPath.isOptionalCallExpression()) {
optionalPath = skipTransparentExprWrappers(optionalPath.get("callee"));
}
}
for (let i = 0; i < chains.length; i++) {
const node = chains[i];
if (
t.isOptionalCallExpression(node) &&
matchAffectedArguments(node.arguments)
) {
// f?.(...[], 0)
if (node.optional) {
return true;
}
// o?.m(...[], 0)
// when node.optional is false, chains[i + 1] is always well defined
const callee = chains[i + 1];
if (t.isOptionalMemberExpression(callee, { optional: true })) {
return true;
}
}
}
return false;
}
@@ -0,0 +1,7 @@
fn?.(...b, 1);

a?.b(...c, 1);

a?.b?.(...c, 1);
JLHwung marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

It would be enough to transform this to

(_a2$b = (_a2 = a)?.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);

instead of

(_a2 = a) === null || _a2 === void 0 ? void 0 : (_a2$b = _a2.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);

We don't need to transform inner optional checks, only the one that directly affects the call expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I mentioned that this PR is sub-optimal for nested chains. But because it's an edge case and we already have to maintain two optional chain transform, I prefer to leave it as-is.


a.b?.(...c, 1);
@@ -0,0 +1,6 @@
var _fn, _a, _a2, _a2$b, _a$b, _a3;

(_fn = fn) === null || _fn === void 0 ? void 0 : _fn(...b, 1);
(_a = a) === null || _a === void 0 ? void 0 : _a.b(...c, 1);
(_a2 = a) === null || _a2 === void 0 ? void 0 : (_a2$b = _a2.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);
(_a$b = (_a3 = a).b) === null || _a$b === void 0 ? void 0 : _a$b.call(_a3, ...c, 1);
@@ -0,0 +1,19 @@
foo?.(...[], 1);

foo?.bar(...[], 1)

foo.bar?.(foo.bar, ...[], 1)

foo?.bar?.(foo.bar, ...[], 1)

foo?.(...[], 1).bar

foo?.(...[], 1)?.bar

foo.bar?.(...[], 1).baz

foo.bar?.(...[], 1)?.baz

foo?.bar?.(...[], 1).baz

foo?.bar?.(...[], 1)?.baz

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

@@ -0,0 +1,5 @@
fn?.(...b, 1);

a?.b(...c, 1);

a?.b?.(...c, 1);
@@ -0,0 +1,5 @@
{
"plugins": [
"proposal-optional-chaining", "bugfix-v8-spread-parameters-in-optional-chaining"
]
}
@@ -0,0 +1,5 @@
var _fn, _a, _a2, _a2$b;

(_fn = fn) === null || _fn === void 0 ? void 0 : _fn(...b, 1);
(_a = a) === null || _a === void 0 ? void 0 : _a.b(...c, 1);
(_a2 = a) === null || _a2 === void 0 ? void 0 : (_a2$b = _a2.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);
@@ -0,0 +1,7 @@
fn?.(...b, 1);

a?.b(...c, 1);

a?.b?.(...c, 1);

a.b?.(...c, 1);
@@ -0,0 +1,6 @@
var _fn, _a, _a2, _a2$b, _a$b, _a3;

(_fn = fn) === null || _fn === void 0 ? void 0 : _fn(...b, 1);
(_a = a) === null || _a === void 0 ? void 0 : _a.b(...c, 1);
(_a2 = a) === null || _a2 === void 0 ? void 0 : (_a2$b = _a2.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);
(_a$b = (_a3 = a).b) === null || _a$b === void 0 ? void 0 : _a$b.call(_a3, ...c, 1);
@@ -0,0 +1,13 @@
class C {
#m;
constructor() {
const o = null;
const n = this;
const p = o?.#m(...c, 1);
const q = n?.#m?.(...c, 1);
expect(p).toBe(undefined);
expect(q).toBe(undefined);
}
}

new C();
@@ -0,0 +1,9 @@
class C {
#m;
constructor() {
const o = null;
const n = this;
const p = o?.#m(...c, 1);
const q = n?.#m?.(...c, 1);
}
}
@@ -0,0 +1,5 @@
{
"plugins": [
"proposal-class-properties", "proposal-private-methods", "proposal-optional-chaining", "bugfix-v8-spread-parameters-in-optional-chaining"
]
}
@@ -0,0 +1,18 @@
var _m = new WeakMap();

class C {
constructor() {
var _babelHelpers$classPr;

_m.set(this, {
writable: true,
value: void 0
});

const o = null;
const n = this;
const p = o === null || o === void 0 ? void 0 : babelHelpers.classPrivateFieldGet(o, _m).call(o, ...c, 1);
const q = n === null || n === void 0 ? void 0 : (_babelHelpers$classPr = babelHelpers.classPrivateFieldGet(n, _m)) === null || _babelHelpers$classPr === void 0 ? void 0 : _babelHelpers$classPr.call(n, ...c, 1);
}

}
@@ -0,0 +1,13 @@
class C {
#m;
constructor() {
const o = null;
const n = this;
const p = o?.#m(...c, 1);
const q = n?.#m?.(...c, 1);
expect(p).toBe(undefined);
expect(q).toBe(undefined);
}
}

new C();
@@ -0,0 +1,9 @@
class C {
#m;
constructor() {
const o = null;
const n = this;
const p = o?.#m(...c, 1);
const q = n?.#m?.(...c, 1);
}
}
@@ -0,0 +1,5 @@
{
"plugins": [
"proposal-class-properties", "proposal-private-methods", "bugfix-v8-spread-parameters-in-optional-chaining"
]
}
@@ -0,0 +1,18 @@
var _m = new WeakMap();

class C {
constructor() {
var _babelHelpers$classPr;

_m.set(this, {
writable: true,
value: void 0
});

const o = null;
const n = this;
const p = o === null || o === void 0 ? void 0 : babelHelpers.classPrivateFieldGet(o, _m).call(o, ...c, 1);
const q = n === null || n === void 0 ? void 0 : (_babelHelpers$classPr = babelHelpers.classPrivateFieldGet(n, _m)) === null || _babelHelpers$classPr === void 0 ? void 0 : _babelHelpers$classPr.call(n, ...c, 1);
}

}
@@ -0,0 +1,13 @@
class C {
#m;
constructor() {
const o = null;
const n = this;
const p = o?.#m(...c, 1);
const q = n?.#m?.(...c, 1);
expect(p).toBe(undefined);
expect(q).toBe(undefined);
}
}

new C;
@@ -0,0 +1,9 @@
class C {
#m;
constructor() {
const o = null;
const n = this;
const p = o?.#m(...c, 1);
const q = n?.#m?.(...c, 1);
}
}
@@ -0,0 +1,10 @@
{
"minNodeVersion": "14.0.0",
"parserOpts": {
"plugins": [
"classPrivateMethods",
"classPrivateProperties",
"classProperties"
]
}
}