Skip to content

Commit

Permalink
Merge pull request ampproject#1 from developit/transform-safari-for-s…
Browse files Browse the repository at this point in the history
…hadowing

Add transform to fix Safari for shadowing bug
  • Loading branch information
developit authored Aug 9, 2019
2 parents 3bbd020 + 6c3cfb3 commit 0f94cfa
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
32 changes: 32 additions & 0 deletions src/plugins/transform-safari-for-shadowing/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Safari ~11 (unsure of version) had a bug where variable declarations in a For statement would throw if they shadowed parameter names.
* This is fixed simply by renaming any declarations in the left/init part of a For* statement so they don't shadow parameters.
* @see https://bugs.webkit.org/show_bug.cgi?id=171041
* @example
* e => { for (let e of []) e } // throws
* e => { for (let _e of []) _e } // works
*/

export default ({ types: t }) => ({
name: 'transform-safari-for-shadowing',
visitor: {
VariableDeclarator(path) {
if (!path.parentPath) return;
const name = path.node.id.name;
const parent = path.parentPath.parentPath;
const key = path.parentPath.parentKey;
if (
// for (let e of ..) & for (let e in ..)
(t.isForXStatement(parent) && key==='left') ||
// for (let e=..; ;)
(t.isForStatement(parent) && key==='init')
) {
// check if there is a shadowed binding coming from a parameter
const fn = path.getFunctionParent();
if (fn && fn.scope.hasOwnBinding(name) && fn.scope.getOwnBinding(name).kind === 'param') {
path.scope.rename(name);
}
}
}
}
});
26 changes: 26 additions & 0 deletions src/plugins/transform-safari-for-shadowing/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { babel } from '../../../test/_util';
import plugin from '.';

const CONFIG = {
plugins: [
plugin
]
};

describe('transform-safari-for-shadowing', () => {
it('should ignore unshadowed for declarations', () => {
expect(babel(`()=>{for(let x of{})x;};`, CONFIG)).toEqual(`()=>{for(let x of{})x;};`);
expect(babel(`()=>{for(const x in[])x;};`, CONFIG)).toEqual(`()=>{for(const x in[])x;};`);
expect(babel(`()=>{for(let x=0;;)x;};`, CONFIG)).toEqual(`()=>{for(let x=0;;)x;};`);
expect(babel(`x=>{for(let y of{})y;};`, CONFIG)).toEqual(`x=>{for(let y of{})y;};`);
expect(babel(`x=>{for(let y of{}){let x;x;}};`, CONFIG)).toEqual(`x=>{for(let y of{}){let x;x;}};`);
});

it('should rename parameter-shadowed declarations within for blocks', () => {
expect(babel(`x=>{for(let x of{})x;};`, CONFIG)).toEqual(`x=>{for(let _x of{})_x;};`);
expect(babel(`x=>{for(const x in[])x;};`, CONFIG)).toEqual(`x=>{for(const _x in[])_x;};`);
expect(babel(`x=>{for(let x=0;;)x;};`, CONFIG)).toEqual(`x=>{for(let _x=0;;)_x;};`);
expect(babel(`()=>{let x;for(let x of{})x;};`, CONFIG)).toEqual(`()=>{let x;for(let x of{})x;};`);
expect(babel(`x=>{for(let x of{}){let x;x;}};`, CONFIG)).toEqual(`x=>{for(let _x of{}){let x;x;}};`);
});
});

0 comments on commit 0f94cfa

Please sign in to comment.