Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
This change assumes loop blocks have scope (they do in ES6), which means
it "fixes" (i.e. changes) the pre-ES6 behaviour of `const` declarations
within loops. Whilst this is "correct", it differs from the previous
behaviour and so early JS engines that previously returned "incorrect"
results will now return "correct" results: specifically `const` in a
loop is local and can be deferenced in a callback with it's loop value
rather than it's terminal value.

This change affects NodeJS versions before 6.x, Chrome until approx v48,
Safari 9 and possibly 10 (I've not verified which browser versions are
actually affected). These engine will run generate "correct" ES6 results
for loops like:

```
async function nop(x) { return x }
var resolve,p = new Promise(function(r){resolve = r}) ;
var i = 0, x = 0, s = 1 ;
while (i<5) {
	const j = i+1 ;
	await nop() ;
	setTimeout(function(){
		x += 1 ;
		s *= j ; // REFERS to "j"
		if (x===5) {
			resolve(s) ;
		}
	}, 0);
	i++ ;
}

return await p ;
```
  • Loading branch information
matAtWork committed May 2, 2018
1 parent 3ee467c commit 5fa3d01
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 24 deletions.
43 changes: 20 additions & 23 deletions arboriculture.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ var examinations = {
case 'ClassExpression':
return true;
case 'VariableDeclaration':
return this.node.kind && this.node.kind !== 'var';
return this.node.kind && this.node.kind === 'let';
case 'FunctionDeclaration':
case 'FunctionExpression':
return !(!this.node.generator);
Expand Down Expand Up @@ -183,6 +183,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
var continuations = {};
var generatedSymbol = 1;
var genIdent = {};

Object.keys(opts).filter(function (k) {
return k[0] === '$';
}).forEach(function (k) {
Expand Down Expand Up @@ -429,7 +430,12 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
}
return null;
}



var looksLikeES6 = opts.es6target || pr.ast.type === 'Program' && pr.ast.sourceType === 'module' || contains(pr.ast, function (n) {
return examine(n).isES6;
}, true);

// actually do the transforms
if (opts.engine) {
// Only Transform extensions:
Expand All @@ -454,18 +460,8 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
pr.ast = fixSuperReferences(pr.ast);
asyncTransforms(pr.ast);
}
/*
if (opts.babelTree) {
treeWalk(pr.ast, function (node, descend, path) {
if (node.type === 'Literal') {
coerce(node, literal(node.value));
} else {
descend();
}
});
}
*/
return pr;

function asyncTransforms(ast, awaitFlag) {
var useLazyLoops = !(opts.promises || opts.generators || opts.engine) && opts.lazyThenables;
// Because we create functions (and scopes), we need all declarations before use
Expand Down Expand Up @@ -1956,6 +1952,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {

/* Find all nodes within this scope matching the specified function */
function scopedNodes(ast, matching, flat) {
if (!flat) flat = ['isScope','isLoop'] ;
var matches = [];
treeWalk(ast, function (node, descend, path) {
if (node === ast)
Expand All @@ -1964,9 +1961,11 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
matches.push([].concat(path));
return;
}
if (flat || examine(node).isScope) {
return;
}
if (flat === true) return ;
var x = examine(node) ;
for (var i=0; i<flat.length; i++)
if (x[flat[i]])
return ;
descend();
});
return matches;
Expand Down Expand Up @@ -2249,7 +2248,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
if (examine(node).isBlockStatement) {
if (containsAwait(node)) {
// For this scope/block, find all the hoistable functions, vars and directives
var isScope = !path[0].parent || examine(path[0].parent).isScope;
var isScope = !path[0].parent || examine(path[0].parent).isScope || examine(path[0].parent).isLoop ;
/* 'const' is highly problematic. In early version of Chrome (Node 0.10, 4.x, 5.x) non-standard behaviour:
*
* Node scope multiple-decls
Expand All @@ -2275,11 +2274,12 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
// Work out which const identifiers are duplicates
// Ones that are only declared once can simply be treated as vars, whereas
// identifiers that are declared multiple times have block-scope and are
// only valid in node 6 or in strict mode on node 4/5
// only valid in node 6 or in strict mode on node 4/5.
// In strict mode or ES6-a-like targets always assume the correct ES6 semantics (block scope)
consts.forEach(function (d) {
d[0].self.declarations.forEach(function (e) {
getDeclNames(e.id).forEach(function (n) {
if (names[n] || duplicates[n]) {
if (inStrictBody || looksLikeES6 || names[n] || duplicates[n]) {
delete names[n];
duplicates[n] = e;
} else {
Expand Down Expand Up @@ -2650,9 +2650,6 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
}
// Hoist generated FunctionDeclarations within ES5 Strict functions (actually put them at the
// end of the scope-block, don't hoist them, it's just an expensive operation)
var looksLikeES6 = ast.type === 'Program' && ast.sourceType === 'module' || contains(ast, function (n) {
return examine(n).isES6;
}, true);
if (!looksLikeES6) {
var useStrict = isStrict(ast);
(function (ast) {
Expand All @@ -2667,7 +2664,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
if (n.type === 'FunctionDeclaration') {
return path[0].parent !== functionScope;
}
});
},['isScope']);
functions = functions.map(function (path) {
return path[0].remove();
});
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "nodent-transform",
"version": "3.2.4",
"version": "3.2.5",
"description": "AST transform and basic tools for nodent and nodent-compiler",
"main": "arboriculture.js",
"scripts": {
Expand Down

0 comments on commit 5fa3d01

Please sign in to comment.