Skip to content

Commit

Permalink
Add workaround for Safari for loop lexical scope bug
Browse files Browse the repository at this point in the history
Safari raises a syntax error for a `let` or `const` declaration in a
`for` loop initalization that shadows a parent function's parameter:

```js
function a(b) {
  let (b;;);
}
```

This is valid code, but Safari throws a syntax error. The bug has been
[reported](https://bugs.webkit.org/show_bug.cgi?id=171041) and since
[fixed](https://trac.webkit.org/changeset/217200/webkit/trunk/Source) in
WebKit, so future versions of Safari will not have this problem.

This modifies the scope tracker's `canUseInReferencedScopes` method to
detect cases where a rename would trigger this bug and use a different
name.

Fixes babel#559
  • Loading branch information
btmills committed Jun 6, 2017
1 parent be3f3a7 commit 83de7cb
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 0 deletions.
Expand Up @@ -1281,4 +1281,67 @@ describe("mangle-names", () => {
`);
expect(transform(source, { topLevel: true }, "module")).toBe(expected);
});

// Safari raises a syntax error for a `let` or `const` declaration in a `for`
// loop initialization that shadows a parent function's parameter.
// https://github.com/babel/babili/issues/559
// https://bugs.webkit.org/show_bug.cgi?id=171041
// https://trac.webkit.org/changeset/217200/webkit/trunk/Source
describe("Safari for loop lexical scope workaround", () => {
it("should not shadow params in ForStatement.init", () => {
const source = unpad(`
function a(b) {
for (let c;;);
}
`);
const expected = unpad(`
function a(a) {
for (let b;;);
}
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in ForInStatement.left", () => {
const source = unpad(`
function a(b) {
for (let c in d);
}
`);
const expected = unpad(`
function a(a) {
for (let b in d);
}
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in ForOfStatement.left", () => {
const source = unpad(`
function a(b) {
for (let c of d);
}
`);
const expected = unpad(`
function a(a) {
for (let b of d);
}
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in complex ForStatement.init", () => {
const source = unpad(`
function a(b) {
for (let [c, d] of e);
}
`);
const expected = unpad(`
function a(a) {
for (let [b, c] of e);
}
`);
expect(transform(source)).toBe(expected);
});
});
});
31 changes: 31 additions & 0 deletions packages/babel-plugin-minify-mangle-names/src/scope-tracker.js
Expand Up @@ -121,6 +121,37 @@ module.exports = class ScopeTracker {
return false;
}

// Safari raises a syntax error for a `let` or `const` declaration in a
// `for` loop initialization that shadows a parent function's parameter.
// https://github.com/babel/babili/issues/559
// https://bugs.webkit.org/show_bug.cgi?id=171041
// https://trac.webkit.org/changeset/217200/webkit/trunk/Source
const isBlockScoped = binding.kind === "let" || binding.kind === "const";
const isForLoopDeclaration =
(binding.path.parentPath.parent.type === "ForStatement" &&
binding.path.parentPath.key === "init") ||
(binding.path.parentPath.parent.type === "ForInStatement" &&
binding.path.parentPath.key === "left") ||
(binding.path.parentPath.parent.type === "ForOfStatement" &&
binding.path.parentPath.key === "left");
if (isBlockScoped && isForLoopDeclaration) {
const functionParentScope = binding.scope.getFunctionParent();
const isIncorrectlyTopLevelInSafari =
binding.scope.parent === functionParentScope;
if (isIncorrectlyTopLevelInSafari) {
const parentFunctionBinding = this.bindings
.get(functionParentScope)
.get(next);
if (parentFunctionBinding) {
const parentFunctionHasParamBinding =
parentFunctionBinding.kind === "param";
if (parentFunctionHasParamBinding) {
return false;
}
}
}
}

for (let i = 0; i < binding.constantViolations.length; i++) {
const violation = binding.constantViolations[i];
if (tracker.hasBindingOrReference(violation.scope, binding, next)) {
Expand Down

0 comments on commit 83de7cb

Please sign in to comment.