Skip to content

Commit 5fa3d01

Browse files
committed
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 ; ```
1 parent 3ee467c commit 5fa3d01

File tree

2 files changed

+21
-24
lines changed

2 files changed

+21
-24
lines changed

arboriculture.js

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ var examinations = {
105105
case 'ClassExpression':
106106
return true;
107107
case 'VariableDeclaration':
108-
return this.node.kind && this.node.kind !== 'var';
108+
return this.node.kind && this.node.kind === 'let';
109109
case 'FunctionDeclaration':
110110
case 'FunctionExpression':
111111
return !(!this.node.generator);
@@ -183,6 +183,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
183183
var continuations = {};
184184
var generatedSymbol = 1;
185185
var genIdent = {};
186+
186187
Object.keys(opts).filter(function (k) {
187188
return k[0] === '$';
188189
}).forEach(function (k) {
@@ -429,7 +430,12 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
429430
}
430431
return null;
431432
}
432-
433+
434+
435+
var looksLikeES6 = opts.es6target || pr.ast.type === 'Program' && pr.ast.sourceType === 'module' || contains(pr.ast, function (n) {
436+
return examine(n).isES6;
437+
}, true);
438+
433439
// actually do the transforms
434440
if (opts.engine) {
435441
// Only Transform extensions:
@@ -454,18 +460,8 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
454460
pr.ast = fixSuperReferences(pr.ast);
455461
asyncTransforms(pr.ast);
456462
}
457-
/*
458-
if (opts.babelTree) {
459-
treeWalk(pr.ast, function (node, descend, path) {
460-
if (node.type === 'Literal') {
461-
coerce(node, literal(node.value));
462-
} else {
463-
descend();
464-
}
465-
});
466-
}
467-
*/
468463
return pr;
464+
469465
function asyncTransforms(ast, awaitFlag) {
470466
var useLazyLoops = !(opts.promises || opts.generators || opts.engine) && opts.lazyThenables;
471467
// Because we create functions (and scopes), we need all declarations before use
@@ -1956,6 +1952,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
19561952

19571953
/* Find all nodes within this scope matching the specified function */
19581954
function scopedNodes(ast, matching, flat) {
1955+
if (!flat) flat = ['isScope','isLoop'] ;
19591956
var matches = [];
19601957
treeWalk(ast, function (node, descend, path) {
19611958
if (node === ast)
@@ -1964,9 +1961,11 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
19641961
matches.push([].concat(path));
19651962
return;
19661963
}
1967-
if (flat || examine(node).isScope) {
1968-
return;
1969-
}
1964+
if (flat === true) return ;
1965+
var x = examine(node) ;
1966+
for (var i=0; i<flat.length; i++)
1967+
if (x[flat[i]])
1968+
return ;
19701969
descend();
19711970
});
19721971
return matches;
@@ -2249,7 +2248,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
22492248
if (examine(node).isBlockStatement) {
22502249
if (containsAwait(node)) {
22512250
// For this scope/block, find all the hoistable functions, vars and directives
2252-
var isScope = !path[0].parent || examine(path[0].parent).isScope;
2251+
var isScope = !path[0].parent || examine(path[0].parent).isScope || examine(path[0].parent).isLoop ;
22532252
/* 'const' is highly problematic. In early version of Chrome (Node 0.10, 4.x, 5.x) non-standard behaviour:
22542253
*
22552254
* Node scope multiple-decls
@@ -2275,11 +2274,12 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
22752274
// Work out which const identifiers are duplicates
22762275
// Ones that are only declared once can simply be treated as vars, whereas
22772276
// identifiers that are declared multiple times have block-scope and are
2278-
// only valid in node 6 or in strict mode on node 4/5
2277+
// only valid in node 6 or in strict mode on node 4/5.
2278+
// In strict mode or ES6-a-like targets always assume the correct ES6 semantics (block scope)
22792279
consts.forEach(function (d) {
22802280
d[0].self.declarations.forEach(function (e) {
22812281
getDeclNames(e.id).forEach(function (n) {
2282-
if (names[n] || duplicates[n]) {
2282+
if (inStrictBody || looksLikeES6 || names[n] || duplicates[n]) {
22832283
delete names[n];
22842284
duplicates[n] = e;
22852285
} else {
@@ -2650,9 +2650,6 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
26502650
}
26512651
// Hoist generated FunctionDeclarations within ES5 Strict functions (actually put them at the
26522652
// end of the scope-block, don't hoist them, it's just an expensive operation)
2653-
var looksLikeES6 = ast.type === 'Program' && ast.sourceType === 'module' || contains(ast, function (n) {
2654-
return examine(n).isES6;
2655-
}, true);
26562653
if (!looksLikeES6) {
26572654
var useStrict = isStrict(ast);
26582655
(function (ast) {
@@ -2667,7 +2664,7 @@ function asynchronize(pr, opts, logger, parsePart, printNode) {
26672664
if (n.type === 'FunctionDeclaration') {
26682665
return path[0].parent !== functionScope;
26692666
}
2670-
});
2667+
},['isScope']);
26712668
functions = functions.map(function (path) {
26722669
return path[0].remove();
26732670
});

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "nodent-transform",
3-
"version": "3.2.4",
3+
"version": "3.2.5",
44
"description": "AST transform and basic tools for nodent and nodent-compiler",
55
"main": "arboriculture.js",
66
"scripts": {

0 commit comments

Comments
 (0)