Skip to content

Commit

Permalink
Fix hoisting of "var" variables on wrapped modules
Browse files Browse the repository at this point in the history
  • Loading branch information
fathyb committed Jul 23, 2018
1 parent c0a84ae commit 3df4bb5
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 0 deletions.
30 changes: 30 additions & 0 deletions src/scope-hoisting/hoist.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,36 @@ module.exports = {
) {
scope.push({id: exportsIdentifier, init: t.objectExpression([])});
}

// Move all "var" variables to the top-level to prevent out of order definitions when wrapped.
for (let name in scope.bindings) {
let binding = scope.getBinding(name);

if (binding.path.scope !== scope && binding.kind === 'var') {
let {parentPath} = binding.path;

if (!parentPath.removed) {
if (parentPath.node.declarations.length) {
binding.path.getStatementParent().insertBefore(
parentPath.node.declarations
.map(decl => {
binding.scope.removeBinding(decl.id.name);
scope.push({id: decl.id});

return decl.init
? t.assignmentExpression('=', decl.id, decl.init)
: null;
})
.filter(decl => decl !== null)
);
}

parentPath.remove();
}

binding.path.remove();
}
}
}

path.stop();
Expand Down
3 changes: 3 additions & 0 deletions test/integration/scope-hoisting/commonjs/hoist-vars/a.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const foo = require('./b')

output = foo()
13 changes: 13 additions & 0 deletions test/integration/scope-hoisting/commonjs/hoist-vars/b.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
if(process.env.NODE_ENV === 'test') {
var c = require('./c')()

for(var i = 0, {length} = c, out = ''; i < length; i++) {
out += c[i]
}

module.exports = function() {
if(c != out) throw new Error()

return out
}
}
12 changes: 12 additions & 0 deletions test/integration/scope-hoisting/commonjs/hoist-vars/c.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module.exports = doFoo

function doFoo() {
return foo
}

if(process.env.NODE_ENV === 'test') {
var foo = 'bar'
}
else {
foo = 'foo'
}
9 changes: 9 additions & 0 deletions test/scope-hoisting.js
Original file line number Diff line number Diff line change
Expand Up @@ -842,5 +842,14 @@ describe('scope hoisting', function() {
let output = await run(b);
assert.deepEqual(output, 9);
});

it('should correctly hoist "var" on wrapped modules', async function() {
let b = await bundle(
__dirname + '/integration/scope-hoisting/commonjs/hoist-vars/a.js'
);

let output = await run(b);
assert.deepEqual(output, 'bar');
});
});
});

0 comments on commit 3df4bb5

Please sign in to comment.