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: hoist variable declaration within do block #13122

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
@@ -0,0 +1,10 @@
expect(
do {
var bar = "foo";
if (!bar) throw new Error(
"unreachable"
)
bar;
}
).toBe("foo");
expect(bar).toBe("foo");
@@ -0,0 +1,7 @@
var x = do {
var bar = "foo";
if (!bar) throw new Error(
"unreachable"
)
bar;
};
@@ -0,0 +1,7 @@
var bar;

var x = function () {
bar = "foo";
if (!bar) throw new Error("unreachable");
return bar;
}();
@@ -0,0 +1,7 @@
expect(
do {
var bar = "foo";
bar;
}
).toBe("foo");
expect(bar).toBe("foo");
@@ -0,0 +1,4 @@
var x = do {
var bar = "foo";
bar;
};
@@ -0,0 +1,2 @@
var bar;
var x = (bar = "foo", bar);

This file was deleted.

@@ -0,0 +1,8 @@
expect(
() => do {
() => {
var bar = "foo";
};
bar;
}
).toThrow(ReferenceError);
1 change: 1 addition & 0 deletions packages/babel-traverse/package.json
Expand Up @@ -19,6 +19,7 @@
"@babel/code-frame": "workspace:^7.12.13",
"@babel/generator": "workspace:^7.14.2",
"@babel/helper-function-name": "workspace:^7.14.2",
"@babel/helper-hoist-variables": "workspace:^7.13.0",
"@babel/helper-split-export-declaration": "workspace:^7.12.13",
"@babel/parser": "workspace:^7.14.2",
"@babel/types": "workspace:^7.14.2",
Expand Down
53 changes: 17 additions & 36 deletions packages/babel-traverse/src/path/replacement.ts
Expand Up @@ -6,35 +6,7 @@ import NodePath from "./index";
import { path as pathCache } from "../cache";
import { parse } from "@babel/parser";
import * as t from "@babel/types";

const hoistVariablesVisitor = {
Function(path) {
path.skip();
},

VariableDeclaration(path) {
if (path.node.kind !== "var") return;

const bindings = path.getBindingIdentifiers();
for (const key of Object.keys(bindings)) {
path.scope.push({ id: bindings[key] });
}

const exprs = [];

for (const declar of path.node.declarations as Array<any>) {
if (declar.init) {
exprs.push(
t.expressionStatement(
t.assignmentExpression("=", declar.id, declar.init),
),
);
}
}

path.replaceWithMultiple(exprs);
},
};
import hoistVariables from "@babel/helper-hoist-variables";

/**
* Replace a node with an array of multiple. This method performs the following steps:
Expand Down Expand Up @@ -238,7 +210,16 @@ export function replaceExpressionWithStatements(
t.CallExpression & { callee: t.ArrowFunctionExpression }
>;

this.traverse(hoistVariablesVisitor);
// hoist variable declaration in do block
// `(do { var x = 1; x;})` -> `var x; (() => { x = 1; return x; })()`
const callee = (this as ThisType).get("callee");
hoistVariables(
callee.get("body"),
(id: t.Identifier) => {
this.scope.push({ id });
},
"var",
);

// add implicit returns to all ending expression statements
const completionRecords: Array<NodePath> = (this as ThisType)
Expand All @@ -252,7 +233,6 @@ export function replaceExpressionWithStatements(
let uid = loop.getData("expressionReplacementReturnUid");

if (!uid) {
const callee = (this as ThisType).get("callee");
uid = callee.scope.generateDeclaredUidIdentifier("ret");
callee
.get("body")
Expand All @@ -272,10 +252,11 @@ export function replaceExpressionWithStatements(
}
}

const callee = this.get("callee") as NodePath<t.FunctionExpression>;

// This is an IIFE, so we don't need to worry about the noNewArrows assumption
callee.arrowFunctionToExpression();
// Fixme: we can not `assert this is NodePath<t.FunctionExpression>` in `arrowFunctionToExpression`
// because it is not a class method known at compile time.
const newCallee = (callee as unknown) as NodePath<t.FunctionExpression>;

// (() => await xxx)() -> await (async () => await xxx)();
const needToAwaitFunction =
Expand All @@ -293,18 +274,18 @@ export function replaceExpressionWithStatements(
t.FUNCTION_TYPES,
);
if (needToAwaitFunction) {
callee.set("async", true);
newCallee.set("async", true);
// yield* will await the generator return result
if (!needToYieldFunction) {
this.replaceWith(t.awaitExpression((this as ThisType).node));
}
}
if (needToYieldFunction) {
callee.set("generator", true);
newCallee.set("generator", true);
this.replaceWith(t.yieldExpression((this as ThisType).node, true));
}

return callee.get("body.body");
return newCallee.get("body.body");
}

export function replaceInline(this: NodePath, nodes: t.Node | Array<t.Node>) {
Expand Down
2 changes: 2 additions & 0 deletions yarn.lock
Expand Up @@ -643,6 +643,7 @@ __metadata:
resolution: "@babel/helper-module-transforms@condition:BABEL_8_BREAKING?:workspace:^7.14.2#5131c1"
dependencies:
"@babel/helper-module-transforms-BABEL_8_BREAKING-false": "npm:@babel/helper-module-transforms@workspace:^7.14.2"
checksum: 99b7a76ae03cc33a6d7a2e970aa72831e096f15bc2c58be10fb297e3fa411d9ba344cd0f93590c1b802a1b4450b7ea29db9aeeb7aeb7b2f9241e751c20a5079c
languageName: node
linkType: hard

Expand Down Expand Up @@ -3544,6 +3545,7 @@ __metadata:
"@babel/code-frame": "workspace:^7.12.13"
"@babel/generator": "workspace:^7.14.2"
"@babel/helper-function-name": "workspace:^7.14.2"
"@babel/helper-hoist-variables": "workspace:^7.13.0"
"@babel/helper-plugin-test-runner": "workspace:*"
"@babel/helper-split-export-declaration": "workspace:^7.12.13"
"@babel/parser": "workspace:^7.14.2"
Expand Down