Browse Source

Merge pull request #443 from rollup/gh-435

Fix module ordering
gh-438-b
Rich Harris 9 years ago
parent
commit
a015c19867
  1. 101
      src/Bundle.js
  2. 36
      src/Module.js
  3. 4
      src/Statement.js
  4. 42
      src/utils/pureFunctions.js
  5. 100
      src/utils/run.js
  6. 3
      test/form/side-effect-n/_config.js
  7. 13
      test/form/side-effect-n/_expected/amd.js
  8. 11
      test/form/side-effect-n/_expected/cjs.js
  9. 9
      test/form/side-effect-n/_expected/es6.js
  10. 14
      test/form/side-effect-n/_expected/iife.js
  11. 17
      test/form/side-effect-n/_expected/umd.js
  12. 9
      test/form/side-effect-n/main.js
  13. 3
      test/form/side-effect-o/_config.js
  14. 17
      test/form/side-effect-o/_expected/amd.js
  15. 15
      test/form/side-effect-o/_expected/cjs.js
  16. 13
      test/form/side-effect-o/_expected/es6.js
  17. 18
      test/form/side-effect-o/_expected/iife.js
  18. 21
      test/form/side-effect-o/_expected/umd.js
  19. 13
      test/form/side-effect-o/main.js
  20. 21
      test/function/cycles-pathological/_config.js
  21. 2
      test/function/iife-comments/_config.js
  22. 19
      test/function/iife-strong-dependencies/_config.js

101
src/Bundle.js

@ -169,7 +169,7 @@ export default class Bundle {
}
fetchAllDependencies ( module ) {
const promises = module.dependencies.map( source => {
const promises = module.sources.map( source => {
return this.resolveId( source, module.id )
.then( resolvedId => {
// If the `resolvedId` is supposed to be external, make it so.
@ -277,82 +277,83 @@ export default class Bundle {
sort () {
let seen = {};
let ordered = [];
let hasCycles;
let ordered = [];
let strongDeps = {};
let stronglyDependsOn = {};
let stronglyDependsOn = blank();
let dependsOn = blank();
function visit ( module ) {
if ( seen[ module.id ] ) return;
seen[ module.id ] = true;
this.modules.forEach( module => {
stronglyDependsOn[ module.id ] = blank();
dependsOn[ module.id ] = blank();
});
const { strongDependencies, weakDependencies } = module.consolidateDependencies();
this.modules.forEach( module => {
function processStrongDependency ( dependency ) {
if ( dependency === module || stronglyDependsOn[ module.id ][ dependency.id ] ) return;
strongDeps[ module.id ] = [];
stronglyDependsOn[ module.id ] = {};
stronglyDependsOn[ module.id ][ dependency.id ] = true;
dependency.strongDependencies.forEach( processStrongDependency );
}
strongDependencies.forEach( imported => {
strongDeps[ module.id ].push( imported );
function processDependency ( dependency ) {
if ( dependency === module || dependsOn[ module.id ][ dependency.id ] ) return;
if ( seen[ imported.id ] ) {
// we need to prevent an infinite loop, and note that
// we need to check for strong/weak dependency relationships
hasCycles = true;
return;
dependsOn[ module.id ][ dependency.id ] = true;
dependency.dependencies.forEach( processDependency );
}
visit( imported );
module.strongDependencies.forEach( processStrongDependency );
module.dependencies.forEach( processDependency );
});
weakDependencies.forEach( imported => {
if ( seen[ imported.id ] ) {
// we need to prevent an infinite loop, and note that
// we need to check for strong/weak dependency relationships
const visit = module => {
if ( seen[ module.id ] ) {
hasCycles = true;
return;
}
visit( imported );
});
// add second (and third...) order dependencies
function addStrongDependencies ( dependency ) {
if ( stronglyDependsOn[ module.id ][ dependency.id ] ) return;
stronglyDependsOn[ module.id ][ dependency.id ] = true;
strongDeps[ dependency.id ].forEach( addStrongDependencies );
}
strongDeps[ module.id ].forEach( addStrongDependencies );
seen[ module.id ] = true;
module.dependencies.forEach( visit );
ordered.push( module );
}
};
this.modules.forEach( visit );
visit( this.entryModule );
if ( hasCycles ) {
let unordered = ordered;
ordered = [];
// unordered is actually semi-ordered, as [ fewer dependencies ... more dependencies ]
unordered.forEach( module => {
// ensure strong dependencies of `module` that don't strongly depend on `module` go first
strongDeps[ module.id ].forEach( place );
function place ( dep ) {
if ( !stronglyDependsOn[ dep.id ][ module.id ] && !~ordered.indexOf( dep ) ) {
strongDeps[ dep.id ].forEach( place );
ordered.push( dep );
ordered.forEach( ( a, i ) => {
for ( i += 1; i < ordered.length; i += 1 ) {
const b = ordered[i];
if ( stronglyDependsOn[ a.id ][ b.id ] ) {
// somewhere, there is a module that imports b before a. Because
// b imports a, a is placed before b. We need to find the module
// in question, so we can provide a useful error message
let parent = '[[unknown]]';
const findParent = module => {
if ( dependsOn[ module.id ][ a.id ] && dependsOn[ module.id ][ b.id ] ) {
parent = module.id;
} else {
for ( let i = 0; i < module.dependencies.length; i += 1 ) {
const dependency = module.dependencies[i];
if ( findParent( dependency ) ) return;
}
}
};
if ( !~ordered.indexOf( module ) ) {
ordered.push( module );
findParent( this.entryModule );
this.onwarn(
`Module ${a.id} may be unable to evaluate without ${b.id}, but is included first due to a cyclical dependency. Consider swapping the import statements in ${parent} to ensure correct ordering`
);
}
}
});
}
return ordered;
}
}

36
src/Module.js

@ -22,6 +22,7 @@ export default class Module {
this.id = id;
// all dependencies
this.sources = [];
this.dependencies = [];
this.resolvedIds = blank();
@ -62,7 +63,7 @@ export default class Module {
// export { name } from './other.js'
if ( source ) {
if ( !~this.dependencies.indexOf( source ) ) this.dependencies.push( source );
if ( !~this.sources.indexOf( source ) ) this.sources.push( source );
if ( node.type === 'ExportAllDeclaration' ) {
// Store `export * from '...'` statements in an array of delegates.
@ -136,7 +137,7 @@ export default class Module {
const node = statement.node;
const source = node.source.value;
if ( !~this.dependencies.indexOf( source ) ) this.dependencies.push( source );
if ( !~this.sources.indexOf( source ) ) this.sources.push( source );
node.specifiers.forEach( specifier => {
const localName = specifier.local.name;
@ -212,6 +213,13 @@ export default class Module {
const id = this.resolvedIds[ source ];
return this.bundle.moduleById[ id ];
});
this.sources.forEach( source => {
const id = this.resolvedIds[ source ];
const module = this.bundle.moduleById[ id ];
if ( !module.isExternal ) this.dependencies.push( module );
});
}
bindReferences () {
@ -243,26 +251,6 @@ export default class Module {
});
}
consolidateDependencies () {
let strongDependencies = [];
let weakDependencies = [];
// treat all imports as weak dependencies
this.dependencies.forEach( source => {
const id = this.resolvedIds[ source ];
const dependency = this.bundle.moduleById[ id ];
if ( !dependency.isExternal && !~weakDependencies.indexOf( dependency ) ) {
weakDependencies.push( dependency );
}
});
strongDependencies = this.strongDependencies
.filter( module => module !== this );
return { strongDependencies, weakDependencies };
}
getExports () {
let exports = blank();
@ -566,11 +554,11 @@ export default class Module {
return magicString.trim();
}
run ( safe ) {
run () {
let marked = false;
this.statements.forEach( statement => {
marked = statement.run( this.strongDependencies, safe ) || marked;
marked = statement.run( this.strongDependencies ) || marked;
});
return marked;

4
src/Statement.js

@ -130,11 +130,11 @@ export default class Statement {
});
}
run ( strongDependencies, safe ) {
run ( strongDependencies ) {
if ( ( this.ran && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return;
this.ran = true;
if ( run( this.node, this.scope, this, strongDependencies, false, safe ) ) {
if ( run( this.node, this.scope, this, strongDependencies, false ) ) {
this.mark();
return true;
}

42
src/utils/pureFunctions.js

@ -0,0 +1,42 @@
let pureFunctions = {};
const arrayTypes = 'Array Int8Array Uint8Array Uint8ClampedArray Int16Array Uint16Array Int32Array Uint32Array Float32Array Float64Array'.split( ' ' );
const simdTypes = 'Int8x16 Int16x8 Int32x4 Float32x4 Float64x2'.split( ' ' );
const simdMethods = 'abs add and bool check div equal extractLane fromFloat32x4 fromFloat32x4Bits fromFloat64x2 fromFloat64x2Bits fromInt16x8Bits fromInt32x4 fromInt32x4Bits fromInt8x16Bits greaterThan greaterThanOrEqual lessThan lessThanOrEqual load max maxNum min minNum mul neg not notEqual or reciprocalApproximation reciprocalSqrtApproximation replaceLane select selectBits shiftLeftByScalar shiftRightArithmeticByScalar shiftRightLogicalByScalar shuffle splat sqrt store sub swizzle xor'.split( ' ' );
let allSimdMethods = [];
simdTypes.forEach( t => {
simdMethods.forEach( m => {
allSimdMethods.push( `SIMD.${t}.${m}` );
});
});
[
'Array.isArray',
'Error', 'EvalError', 'InternalError', 'RangeError', 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError',
'isFinite', 'isNaN', 'parseFloat', 'parseInt', 'decodeURI', 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', 'escape', 'unescape',
'Object', 'Object.create', 'Object.getNotifier', 'Object.getOwn', 'Object.getOwnPropertyDescriptor', 'Object.getOwnPropertyNames', 'Object.getOwnPropertySymbols', 'Object.getPrototypeOf', 'Object.is', 'Object.isExtensible', 'Object.isFrozen', 'Object.isSealed', 'Object.keys',
'Function', 'Boolean',
'Number', 'Number.isFinite', 'Number.isInteger', 'Number.isNaN', 'Number.isSafeInteger', 'Number.parseFloat', 'Number.parseInt',
'Symbol', 'Symbol.for', 'Symbol.keyFor',
'Math.abs', 'Math.acos', 'Math.acosh', 'Math.asin', 'Math.asinh', 'Math.atan', 'Math.atan2', 'Math.atanh', 'Math.cbrt', 'Math.ceil', 'Math.clz32', 'Math.cos', 'Math.cosh', 'Math.exp', 'Math.expm1', 'Math.floor', 'Math.fround', 'Math.hypot', 'Math.imul', 'Math.log', 'Math.log10', 'Math.log1p', 'Math.log2', 'Math.max', 'Math.min', 'Math.pow', 'Math.random', 'Math.round', 'Math.sign', 'Math.sin', 'Math.sinh', 'Math.sqrt', 'Math.tan', 'Math.tanh', 'Math.trunc',
'Date', 'Date.UTC', 'Date.now', 'Date.parse',
'String', 'String.fromCharCode', 'String.fromCodePoint', 'String.raw',
'RegExp',
'Map', 'Set', 'WeakMap', 'WeakSet',
'ArrayBuffer', 'ArrayBuffer.isView',
'DataView',
'JSON.parse', 'JSON.stringify',
'Promise', 'Promise.all', 'Promise.race', 'Promise.reject', 'Promise.resolve',
'Intl.Collator', 'Intl.Collator.supportedLocalesOf', 'Intl.DateTimeFormat', 'Intl.DateTimeFormat.supportedLocalesOf', 'Intl.NumberFormat', 'Intl.NumberFormat.supportedLocalesOf'
// TODO properties of e.g. window...
].concat(
arrayTypes,
arrayTypes.map( t => `${t}.from` ),
arrayTypes.map( t => `${t}.of` ),
simdTypes.map( t => `SIMD.${t}` ),
allSimdMethods
).forEach( name => pureFunctions[ name ] = true );
// TODO add others to this list from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects
export default pureFunctions;

100
src/utils/run.js

@ -2,49 +2,39 @@ import { walk } from 'estree-walker';
import modifierNodes, { isModifierNode } from '../ast/modifierNodes.js';
import isReference from '../ast/isReference.js';
import flatten from '../ast/flatten';
import pureFunctions from './pureFunctions.js';
let pureFunctions = {};
function call ( callee, scope, statement, strongDependencies ) {
while ( callee.type === 'ParenthesizedExpression' ) callee = callee.expression;
const arrayTypes = 'Array Int8Array Uint8Array Uint8ClampedArray Int16Array Uint16Array Int32Array Uint32Array Float32Array Float64Array'.split( ' ' );
const simdTypes = 'Int8x16 Int16x8 Int32x4 Float32x4 Float64x2'.split( ' ' );
const simdMethods = 'abs add and bool check div equal extractLane fromFloat32x4 fromFloat32x4Bits fromFloat64x2 fromFloat64x2Bits fromInt16x8Bits fromInt32x4 fromInt32x4Bits fromInt8x16Bits greaterThan greaterThanOrEqual lessThan lessThanOrEqual load max maxNum min minNum mul neg not notEqual or reciprocalApproximation reciprocalSqrtApproximation replaceLane select selectBits shiftLeftByScalar shiftRightArithmeticByScalar shiftRightLogicalByScalar shuffle splat sqrt store sub swizzle xor'.split( ' ' );
let allSimdMethods = [];
simdTypes.forEach( t => {
simdMethods.forEach( m => {
allSimdMethods.push( `SIMD.${t}.${m}` );
});
});
if ( callee.type === 'Identifier' ) {
const declaration = scope.findDeclaration( callee.name ) ||
statement.module.trace( callee.name );
[
'Array.isArray',
'Error', 'EvalError', 'InternalError', 'RangeError', 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError',
'isFinite', 'isNaN', 'parseFloat', 'parseInt', 'decodeURI', 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', 'escape', 'unescape',
'Object', 'Object.create', 'Object.getNotifier', 'Object.getOwn', 'Object.getOwnPropertyDescriptor', 'Object.getOwnPropertyNames', 'Object.getOwnPropertySymbols', 'Object.getPrototypeOf', 'Object.is', 'Object.isExtensible', 'Object.isFrozen', 'Object.isSealed', 'Object.keys',
'Function', 'Boolean',
'Number', 'Number.isFinite', 'Number.isInteger', 'Number.isNaN', 'Number.isSafeInteger', 'Number.parseFloat', 'Number.parseInt',
'Symbol', 'Symbol.for', 'Symbol.keyFor',
'Math.abs', 'Math.acos', 'Math.acosh', 'Math.asin', 'Math.asinh', 'Math.atan', 'Math.atan2', 'Math.atanh', 'Math.cbrt', 'Math.ceil', 'Math.clz32', 'Math.cos', 'Math.cosh', 'Math.exp', 'Math.expm1', 'Math.floor', 'Math.fround', 'Math.hypot', 'Math.imul', 'Math.log', 'Math.log10', 'Math.log1p', 'Math.log2', 'Math.max', 'Math.min', 'Math.pow', 'Math.random', 'Math.round', 'Math.sign', 'Math.sin', 'Math.sinh', 'Math.sqrt', 'Math.tan', 'Math.tanh', 'Math.trunc',
'Date', 'Date.UTC', 'Date.now', 'Date.parse',
'String', 'String.fromCharCode', 'String.fromCodePoint', 'String.raw',
'RegExp',
'Map', 'Set', 'WeakMap', 'WeakSet',
'ArrayBuffer', 'ArrayBuffer.isView',
'DataView',
'JSON.parse', 'JSON.stringify',
'Promise', 'Promise.all', 'Promise.race', 'Promise.reject', 'Promise.resolve',
'Intl.Collator', 'Intl.Collator.supportedLocalesOf', 'Intl.DateTimeFormat', 'Intl.DateTimeFormat.supportedLocalesOf', 'Intl.NumberFormat', 'Intl.NumberFormat.supportedLocalesOf'
// TODO properties of e.g. window...
].concat(
arrayTypes,
arrayTypes.map( t => `${t}.from` ),
arrayTypes.map( t => `${t}.of` ),
simdTypes.map( t => `SIMD.${t}` ),
allSimdMethods
).forEach( name => pureFunctions[ name ] = true );
// TODO add others to this list from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects
if ( declaration ) return declaration.run( strongDependencies );
return !pureFunctions[ callee.name ];
}
if ( /FunctionExpression/.test( callee.type ) ) {
return run( callee.body, scope, statement, strongDependencies );
}
if ( callee.type === 'MemberExpression' ) {
const flattened = flatten( callee );
if ( flattened ) {
// if we're calling e.g. Object.keys(thing), there are no side-effects
// TODO make pureFunctions configurable
const declaration = scope.findDeclaration( flattened.name ) || statement.module.trace( flattened.name );
return ( !!declaration || !pureFunctions[ flattened.keypath ] );
}
}
// complex case like `( a ? b : c )()` or foo[bar].baz()`
// – err on the side of caution
return true;
}
export default function run ( node, scope, statement, strongDependencies, force ) {
let hasSideEffect = false;
@ -78,39 +68,7 @@ export default function run ( node, scope, statement, strongDependencies, force
}
else if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) {
if ( node.callee.type === 'Identifier' ) {
const declaration = scope.findDeclaration( node.callee.name ) ||
statement.module.trace( node.callee.name );
if ( declaration ) {
if ( declaration.run( strongDependencies ) ) {
hasSideEffect = true;
}
} else if ( !pureFunctions[ node.callee.name ] ) {
hasSideEffect = true;
}
}
else if ( node.callee.type === 'MemberExpression' ) {
const flattened = flatten( node.callee );
if ( flattened ) {
// if we're calling e.g. Object.keys(thing), there are no side-effects
// TODO make pureFunctions configurable
const declaration = scope.findDeclaration( flattened.name ) || statement.module.trace( flattened.name );
if ( !!declaration || !pureFunctions[ flattened.keypath ] ) {
hasSideEffect = true;
}
} else {
// is not a keypath like `foo.bar.baz` – could be e.g.
// `foo[bar].baz()`. Err on the side of caution
hasSideEffect = true;
}
}
// otherwise we're probably dealing with a function expression
else if ( run( node.callee, scope, statement, strongDependencies, true ) ) {
if ( call( node.callee, scope, statement, strongDependencies ) ) {
hasSideEffect = true;
}
}

3
test/form/side-effect-n/_config.js

@ -0,0 +1,3 @@
module.exports = {
description: 'detects side-effects in complex call expressions'
};

13
test/form/side-effect-n/_expected/amd.js

@ -0,0 +1,13 @@
define(function () { 'use strict';
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
( Math.random() < 0.5 ? foo : bar )();
});

11
test/form/side-effect-n/_expected/cjs.js

@ -0,0 +1,11 @@
'use strict';
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
( Math.random() < 0.5 ? foo : bar )();

9
test/form/side-effect-n/_expected/es6.js

@ -0,0 +1,9 @@
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
( Math.random() < 0.5 ? foo : bar )();

14
test/form/side-effect-n/_expected/iife.js

@ -0,0 +1,14 @@
(function () {
'use strict';
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
( Math.random() < 0.5 ? foo : bar )();
}());

17
test/form/side-effect-n/_expected/umd.js

@ -0,0 +1,17 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
typeof define === 'function' && define.amd ? define(factory) :
(factory());
}(this, function () { 'use strict';
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
( Math.random() < 0.5 ? foo : bar )();
}));

9
test/form/side-effect-n/main.js

@ -0,0 +1,9 @@
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
( Math.random() < 0.5 ? foo : bar )();

3
test/form/side-effect-o/_config.js

@ -0,0 +1,3 @@
module.exports = {
description: 'detects side-effects in complex call expressions'
};

17
test/form/side-effect-o/_expected/amd.js

@ -0,0 +1,17 @@
define(function () { 'use strict';
function fn () {
return Math.random() < 0.5 ? foo : bar;
}
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
fn()();
});

15
test/form/side-effect-o/_expected/cjs.js

@ -0,0 +1,15 @@
'use strict';
function fn () {
return Math.random() < 0.5 ? foo : bar;
}
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
fn()();

13
test/form/side-effect-o/_expected/es6.js

@ -0,0 +1,13 @@
function fn () {
return Math.random() < 0.5 ? foo : bar;
}
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
fn()();

18
test/form/side-effect-o/_expected/iife.js

@ -0,0 +1,18 @@
(function () {
'use strict';
function fn () {
return Math.random() < 0.5 ? foo : bar;
}
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
fn()();
}());

21
test/form/side-effect-o/_expected/umd.js

@ -0,0 +1,21 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
typeof define === 'function' && define.amd ? define(factory) :
(factory());
}(this, function () { 'use strict';
function fn () {
return Math.random() < 0.5 ? foo : bar;
}
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
fn()();
}));

13
test/form/side-effect-o/main.js

@ -0,0 +1,13 @@
function fn () {
return Math.random() < 0.5 ? foo : bar;
}
function foo () {
console.log( 'foo' );
}
function bar () {
console.log( 'bar' );
}
fn()();

21
test/function/cycles-pathological/_config.js

@ -1,18 +1,17 @@
var assert = require( 'assert' );
var warned;
module.exports = {
description: 'resolves pathological cyclical dependencies gracefully',
babel: true,
exports: function ( exports ) {
assert.ok( exports.a.isA );
assert.ok( exports.b1.isA );
assert.ok( exports.b1.isB );
assert.ok( exports.b2.isA );
assert.ok( exports.b2.isB );
assert.ok( exports.c1.isC );
assert.ok( exports.c1.isD );
assert.ok( exports.c2.isC );
assert.ok( exports.c2.isD );
assert.ok( exports.d.isD );
options: {
onwarn: function ( message ) {
assert.ok( /Module .+B\.js may be unable to evaluate without .+A\.js, but is included first due to a cyclical dependency. Consider swapping the import statements in .+main\.js to ensure correct ordering/.test( message ) );
warned = true;
}
},
runtimeError: function () {
assert.ok( warned );
}
};

2
test/function/iife-comments/_config.js

@ -5,4 +5,4 @@ module.exports = {
exports: function ( exports ) {
assert.equal( exports, 42 );
}
}
};

19
test/function/iife-strong-dependencies/_config.js

@ -1,15 +1,16 @@
var assert = require( 'assert' );
var warned;
module.exports = {
description: 'does not treat references inside IIFEs as weak dependencies', // edge case encountered in THREE.js codebase
exports: function ( exports ) {
assert.ok( exports.a1.isA );
assert.ok( exports.b1.isB );
assert.ok( exports.c1.isC );
assert.ok( exports.d1.isD );
assert.ok( exports.a2.isA );
assert.ok( exports.b2.isB );
assert.ok( exports.c2.isC );
assert.ok( exports.d2.isD );
options: {
onwarn: function ( message ) {
assert.ok( /Module .+D\.js may be unable to evaluate without .+C\.js, but is included first due to a cyclical dependency. Consider swapping the import statements in .+main\.js to ensure correct ordering/.test( message ) );
warned = true;
}
},
runtimeError: function () {
assert.ok( warned );
}
};

Loading…
Cancel
Save