From d4eeb54dae518c60c90b07524e685d130e85ba2a Mon Sep 17 00:00:00 2001 From: Brian Donovan Date: Mon, 13 Jul 2015 13:50:37 -0700 Subject: [PATCH 1/5] Add a failing test that illustrates a problem with RSVP. --- .../top-level-side-effects-are-preserved/_config.js | 3 +++ test/function/top-level-side-effects-are-preserved/asap.js | 1 + .../top-level-side-effects-are-preserved/config.js | 1 + .../function/top-level-side-effects-are-preserved/defer.js | 5 +++++ test/function/top-level-side-effects-are-preserved/main.js | 3 +++ test/function/top-level-side-effects-are-preserved/rsvp.js | 7 +++++++ 6 files changed, 20 insertions(+) create mode 100644 test/function/top-level-side-effects-are-preserved/_config.js create mode 100644 test/function/top-level-side-effects-are-preserved/asap.js create mode 100644 test/function/top-level-side-effects-are-preserved/config.js create mode 100644 test/function/top-level-side-effects-are-preserved/defer.js create mode 100644 test/function/top-level-side-effects-are-preserved/main.js create mode 100644 test/function/top-level-side-effects-are-preserved/rsvp.js diff --git a/test/function/top-level-side-effects-are-preserved/_config.js b/test/function/top-level-side-effects-are-preserved/_config.js new file mode 100644 index 0000000..b121b33 --- /dev/null +++ b/test/function/top-level-side-effects-are-preserved/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'top level side effects are preserved' +}; diff --git a/test/function/top-level-side-effects-are-preserved/asap.js b/test/function/top-level-side-effects-are-preserved/asap.js new file mode 100644 index 0000000..6b43f3a --- /dev/null +++ b/test/function/top-level-side-effects-are-preserved/asap.js @@ -0,0 +1 @@ +export default function asap() {} diff --git a/test/function/top-level-side-effects-are-preserved/config.js b/test/function/top-level-side-effects-are-preserved/config.js new file mode 100644 index 0000000..c4470d2 --- /dev/null +++ b/test/function/top-level-side-effects-are-preserved/config.js @@ -0,0 +1 @@ +export const config = {}; diff --git a/test/function/top-level-side-effects-are-preserved/defer.js b/test/function/top-level-side-effects-are-preserved/defer.js new file mode 100644 index 0000000..2ab1d6d --- /dev/null +++ b/test/function/top-level-side-effects-are-preserved/defer.js @@ -0,0 +1,5 @@ +import { config } from './config'; + +export default function defer() { + config.async(); +} diff --git a/test/function/top-level-side-effects-are-preserved/main.js b/test/function/top-level-side-effects-are-preserved/main.js new file mode 100644 index 0000000..d844cc0 --- /dev/null +++ b/test/function/top-level-side-effects-are-preserved/main.js @@ -0,0 +1,3 @@ +import { defer } from './rsvp'; + +defer(); diff --git a/test/function/top-level-side-effects-are-preserved/rsvp.js b/test/function/top-level-side-effects-are-preserved/rsvp.js new file mode 100644 index 0000000..d3bcb42 --- /dev/null +++ b/test/function/top-level-side-effects-are-preserved/rsvp.js @@ -0,0 +1,7 @@ +import { config } from './config'; +import asap from './asap'; +import defer from './defer'; + +config.async = asap; + +export { defer }; From 33effb4b3ba1967fedeb869fe6bc15fde5812cd5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 14 Jul 2015 11:04:59 -0400 Subject: [PATCH 2/5] order methods alphabetically. easier to find what youre looking for --- src/Bundle.js | 230 +++++++++++++++++++++--------------------- src/ExternalModule.js | 8 +- src/Module.js | 138 ++++++++++++------------- src/ast/Scope.js | 10 +- 4 files changed, 193 insertions(+), 193 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index c5f66f2..d822852 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -51,39 +51,6 @@ export default class Bundle { this.assumedGlobals = blank(); } - fetchModule ( importee, importer ) { - return Promise.resolve( this.resolveId( importee, importer, this.resolveOptions ) ) - .then( id => { - if ( !id ) { - // external module - if ( !this.modulePromises[ importee ] ) { - const module = new ExternalModule( importee ); - this.externalModules.push( module ); - this.modulePromises[ importee ] = Promise.resolve( module ); - } - - return this.modulePromises[ importee ]; - } - - if ( !this.modulePromises[ id ] ) { - this.modulePromises[ id ] = Promise.resolve( this.load( id, this.loadOptions ) ) - .then( source => { - const module = new Module({ - id, - source, - bundle: this - }); - - this.modules.push( module ); - - return module; - }); - } - - return this.modulePromises[ id ]; - }); - } - build () { // bring in top-level AST nodes from the entry module return this.fetchModule( this.entry, undefined ) @@ -213,96 +180,37 @@ export default class Bundle { } } - sort () { - let seen = {}; - let ordered = []; - let hasCycles; - - let strongDeps = {}; - let stronglyDependsOn = {}; - - function visit ( module ) { - seen[ module.id ] = true; - - const { strongDependencies, weakDependencies } = module.consolidateDependencies(); - - strongDeps[ module.id ] = []; - stronglyDependsOn[ module.id ] = {}; - - keys( strongDependencies ).forEach( id => { - const imported = strongDependencies[ id ]; - - strongDeps[ module.id ].push( imported ); - - if ( seen[ id ] ) { - // we need to prevent an infinite loop, and note that - // we need to check for strong/weak dependency relationships - hasCycles = true; - return; - } - - visit( imported ); - }); - - keys( weakDependencies ).forEach( id => { - const imported = weakDependencies[ id ]; + fetchModule ( importee, importer ) { + return Promise.resolve( this.resolveId( importee, importer, this.resolveOptions ) ) + .then( id => { + if ( !id ) { + // external module + if ( !this.modulePromises[ importee ] ) { + const module = new ExternalModule( importee ); + this.externalModules.push( module ); + this.modulePromises[ importee ] = Promise.resolve( module ); + } - if ( seen[ id ] ) { - // we need to prevent an infinite loop, and note that - // we need to check for strong/weak dependency relationships - hasCycles = true; - return; + return this.modulePromises[ importee ]; } - 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 ); - - ordered.push( module ); - } - - 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 ); + if ( !this.modulePromises[ id ] ) { + this.modulePromises[ id ] = Promise.resolve( this.load( id, this.loadOptions ) ) + .then( source => { + const module = new Module({ + id, + source, + bundle: this + }); - function place ( dep ) { - if ( !stronglyDependsOn[ dep.id ][ module.id ] && !~ordered.indexOf( dep ) ) { - strongDeps[ dep.id ].forEach( place ); - ordered.push( dep ); - } - } + this.modules.push( module ); - if ( !~ordered.indexOf( module ) ) { - ordered.push( module ); + return module; + }); } - }); - } - let statements = []; - - ordered.forEach( module => { - module.statements.forEach( statement => { - if ( statement.isIncluded ) statements.push( statement ); + return this.modulePromises[ id ]; }); - }); - - return statements; } generate ( options = {} ) { @@ -498,4 +406,96 @@ export default class Bundle { return { code, map }; } + + sort () { + let seen = {}; + let ordered = []; + let hasCycles; + + let strongDeps = {}; + let stronglyDependsOn = {}; + + function visit ( module ) { + seen[ module.id ] = true; + + const { strongDependencies, weakDependencies } = module.consolidateDependencies(); + + strongDeps[ module.id ] = []; + stronglyDependsOn[ module.id ] = {}; + + keys( strongDependencies ).forEach( id => { + const imported = strongDependencies[ id ]; + + strongDeps[ module.id ].push( imported ); + + if ( seen[ id ] ) { + // we need to prevent an infinite loop, and note that + // we need to check for strong/weak dependency relationships + hasCycles = true; + return; + } + + visit( imported ); + }); + + keys( weakDependencies ).forEach( id => { + const imported = weakDependencies[ id ]; + + if ( seen[ id ] ) { + // we need to prevent an infinite loop, and note that + // we need to check for strong/weak dependency relationships + 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 ); + + ordered.push( module ); + } + + 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 ); + } + } + + if ( !~ordered.indexOf( module ) ) { + ordered.push( module ); + } + }); + } + + let statements = []; + + ordered.forEach( module => { + module.statements.forEach( statement => { + if ( statement.isIncluded ) statements.push( statement ); + }); + }); + + return statements; + } } diff --git a/src/ExternalModule.js b/src/ExternalModule.js index 29273a6..d8c944d 100644 --- a/src/ExternalModule.js +++ b/src/ExternalModule.js @@ -15,6 +15,10 @@ export default class ExternalModule { this.needsNamed = false; } + findDefiningStatement () { + return null; + } + getCanonicalName ( name ) { if ( name === 'default' ) { return this.needsNamed ? `${this.name}__default` : this.name; @@ -37,8 +41,4 @@ export default class ExternalModule { this.suggestedNames[ exportName ] = suggestion; } } - - findDefiningStatement () { - return null; - } } diff --git a/src/Module.js b/src/Module.js index 907d357..2f6aff6 100644 --- a/src/Module.js +++ b/src/Module.js @@ -271,75 +271,6 @@ export default class Module { return { strongDependencies, weakDependencies }; } - findDeclaration ( localName ) { - const importDeclaration = this.imports[ localName ]; - - // name was defined by another module - if ( importDeclaration ) { - const module = importDeclaration.module; - - if ( module.isExternal ) return null; - - const exportDeclaration = module.exports[ importDeclaration.name ]; - return module.findDeclaration( exportDeclaration.localName ); - } - - // name was defined by this module, if any - let i = this.statements.length; - while ( i-- ) { - const declaration = this.statements[i].scope.declarations[ localName ]; - if ( declaration ) { - return declaration; - } - } - - return null; - } - - getCanonicalName ( localName ) { - // Special case - if ( localName === 'default' && ( this.exports.default.isModified || !this.suggestedNames.default ) ) { - let canonicalName = makeLegalIdentifier( this.id.replace( dirname( this.bundle.entryModule.id ) + '/', '' ).replace( /\.js$/, '' ) ); - return deconflict( canonicalName, this.definitions ); - } - - if ( this.suggestedNames[ localName ] ) { - localName = this.suggestedNames[ localName ]; - } - - if ( !this.canonicalNames[ localName ] ) { - let canonicalName; - - if ( this.imports[ localName ] ) { - const importDeclaration = this.imports[ localName ]; - const module = importDeclaration.module; - - if ( importDeclaration.name === '*' ) { - canonicalName = module.suggestedNames[ '*' ]; - } else { - let exporterLocalName; - - if ( module.isExternal ) { - exporterLocalName = importDeclaration.name; - } else { - const exportDeclaration = module.exports[ importDeclaration.name ]; - exporterLocalName = exportDeclaration.localName; - } - - canonicalName = module.getCanonicalName( exporterLocalName ); - } - } - - else { - canonicalName = localName; - } - - this.canonicalNames[ localName ] = canonicalName; - } - - return this.canonicalNames[ localName ]; - } - define ( name ) { // shortcut cycles. TODO this won't work everywhere... if ( this.definitionPromises[ name ] ) { @@ -507,6 +438,75 @@ export default class Module { }); } + findDeclaration ( localName ) { + const importDeclaration = this.imports[ localName ]; + + // name was defined by another module + if ( importDeclaration ) { + const module = importDeclaration.module; + + if ( module.isExternal ) return null; + + const exportDeclaration = module.exports[ importDeclaration.name ]; + return module.findDeclaration( exportDeclaration.localName ); + } + + // name was defined by this module, if any + let i = this.statements.length; + while ( i-- ) { + const declaration = this.statements[i].scope.declarations[ localName ]; + if ( declaration ) { + return declaration; + } + } + + return null; + } + + getCanonicalName ( localName ) { + // Special case + if ( localName === 'default' && ( this.exports.default.isModified || !this.suggestedNames.default ) ) { + let canonicalName = makeLegalIdentifier( this.id.replace( dirname( this.bundle.entryModule.id ) + '/', '' ).replace( /\.js$/, '' ) ); + return deconflict( canonicalName, this.definitions ); + } + + if ( this.suggestedNames[ localName ] ) { + localName = this.suggestedNames[ localName ]; + } + + if ( !this.canonicalNames[ localName ] ) { + let canonicalName; + + if ( this.imports[ localName ] ) { + const importDeclaration = this.imports[ localName ]; + const module = importDeclaration.module; + + if ( importDeclaration.name === '*' ) { + canonicalName = module.suggestedNames[ '*' ]; + } else { + let exporterLocalName; + + if ( module.isExternal ) { + exporterLocalName = importDeclaration.name; + } else { + const exportDeclaration = module.exports[ importDeclaration.name ]; + exporterLocalName = exportDeclaration.localName; + } + + canonicalName = module.getCanonicalName( exporterLocalName ); + } + } + + else { + canonicalName = localName; + } + + this.canonicalNames[ localName ] = canonicalName; + } + + return this.canonicalNames[ localName ]; + } + rename ( name, replacement ) { this.canonicalNames[ name ] = replacement; } diff --git a/src/ast/Scope.js b/src/ast/Scope.js index 87bbf4d..8dd054a 100644 --- a/src/ast/Scope.js +++ b/src/ast/Scope.js @@ -43,11 +43,6 @@ export default class Scope { } } - getDeclaration ( name ) { - return this.declarations[ name ] || - this.parent && this.parent.getDeclaration( name ); - } - contains ( name ) { return !!this.getDeclaration( name ); } @@ -63,4 +58,9 @@ export default class Scope { return null; } + + getDeclaration ( name ) { + return this.declarations[ name ] || + this.parent && this.parent.getDeclaration( name ); + } } From eadbfcbdcd22fcf1ab789e46ba9821e2a5bf546c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 14 Jul 2015 13:16:17 -0400 Subject: [PATCH 3/5] refactoring --- src/Bundle.js | 11 +- src/Module.js | 311 ++++++++++++++++++++++++++------------------------ 2 files changed, 164 insertions(+), 158 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index d822852..88f8f37 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -45,14 +45,13 @@ export default class Bundle { this.modulePromises = blank(); this.modules = []; - this.statements = []; + this.statements = null; this.externalModules = []; this.internalNamespaceModules = []; this.assumedGlobals = blank(); } build () { - // bring in top-level AST nodes from the entry module return this.fetchModule( this.entry, undefined ) .then( entryModule => { const defaultExport = entryModule.exports.default; @@ -87,11 +86,9 @@ export default class Bundle { return entryModule.expandAllStatements( true ); }) - .then( statements => { - this.statements = statements; + .then( () => { + this.statements = this.sort(); this.deconflict(); - - this.orderedStatements = this.sort(); }); } @@ -260,7 +257,7 @@ export default class Bundle { let previousIndex = -1; let previousMargin = 0; - this.orderedStatements.forEach( statement => { + this.statements.forEach( statement => { // skip `export { foo, bar, baz }` if ( statement.node.type === 'ExportNamedDeclaration' ) { // skip `export { foo, bar, baz }` diff --git a/src/Module.js b/src/Module.js index 2f6aff6..efeebbe 100644 --- a/src/Module.js +++ b/src/Module.js @@ -37,194 +37,143 @@ export default class Module { this.suggestedNames = blank(); this.comments = []; - // Try to extract a list of top-level statements/declarations. If - // the parse fails, attach file info and abort - let ast; + this.statements = this._parse(); - try { - ast = parse( source, { - ecmaVersion: 6, - sourceType: 'module', - onComment: ( block, text, start, end ) => this.comments.push({ block, text, start, end }) - }); - } catch ( err ) { - err.code = 'PARSE_ERROR'; - err.file = id; // see above - not necessarily true, but true enough - throw err; - } + // imports and exports, indexed by ID + this.imports = blank(); + this.exports = blank(); - walk( ast, { - enter: node => { - this.magicString.addSourcemapLocation( node.start ); - this.magicString.addSourcemapLocation( node.end ); - } - }); + this.canonicalNames = blank(); - this.statements = []; + this.definitions = blank(); + this.definitionPromises = blank(); + this.modifications = blank(); - ast.body.map( node => { - // special case - top-level var declarations with multiple declarators - // should be split up. Otherwise, we may end up including code we - // don't need, just because an unwanted declarator is included - if ( node.type === 'VariableDeclaration' && node.declarations.length > 1 ) { - node.declarations.forEach( declarator => { - const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); - magicString.prepend( `${node.kind} ` ).append( ';' ); + this.analyse(); + } - const syntheticNode = { - type: 'VariableDeclaration', - kind: node.kind, - start: node.start, - end: node.end, - declarations: [ declarator ] + addExport ( statement ) { + const node = statement.node; + const source = node.source && node.source.value; + + // export default function foo () {} + // export default foo; + // export default 42; + if ( node.type === 'ExportDefaultDeclaration' ) { + const isDeclaration = /Declaration$/.test( node.declaration.type ); + const isAnonymous = /(?:Class|Function)Expression$/.test( node.declaration.type ); + + const declaredName = isDeclaration && node.declaration.id.name; + const identifier = node.declaration.type === 'Identifier' && node.declaration.name; + + this.exports.default = { + statement, + name: 'default', + localName: declaredName || 'default', + declaredName, + identifier, + isDeclaration, + isAnonymous, + isModified: false // in case of `export default foo; foo = somethingElse` + }; + } + + // export { foo, bar, baz } + // export var foo = 42; + // export function foo () {} + else if ( node.type === 'ExportNamedDeclaration' ) { + if ( node.specifiers.length ) { + // export { foo, bar, baz } + node.specifiers.forEach( specifier => { + const localName = specifier.local.name; + const exportedName = specifier.exported.name; + + this.exports[ exportedName ] = { + localName, + exportedName }; - const statement = new Statement( syntheticNode, magicString, this, this.statements.length ); - this.statements.push( statement ); + // export { foo } from './foo'; + if ( source ) { + this.imports[ localName ] = { + source, + localName, + name: localName + }; + } }); } else { - const magicString = this.magicString.snip( node.start, node.end ).trim(); - const statement = new Statement( node, magicString, this, this.statements.length ); - - this.statements.push( statement ); - } - }); - - this.importDeclarations = this.statements.filter( isImportDeclaration ); - this.exportDeclarations = this.statements.filter( isExportDeclaration ); - - this.analyse(); - } - - analyse () { - // imports and exports, indexed by ID - this.imports = blank(); - this.exports = blank(); - - this.importDeclarations.forEach( statement => { - const node = statement.node; - const source = node.source.value; + let declaration = node.declaration; - node.specifiers.forEach( specifier => { - const isDefault = specifier.type === 'ImportDefaultSpecifier'; - const isNamespace = specifier.type === 'ImportNamespaceSpecifier'; + let name; - const localName = specifier.local.name; - const name = isDefault ? 'default' : isNamespace ? '*' : specifier.imported.name; - - if ( this.imports[ localName ] ) { - const err = new Error( `Duplicated import '${localName}'` ); - err.file = this.id; - err.loc = getLocation( this.source, specifier.start ); - throw err; + if ( declaration.type === 'VariableDeclaration' ) { + // export var foo = 42 + name = declaration.declarations[0].id.name; + } else { + // export function foo () {} + name = declaration.id.name; } - this.imports[ localName ] = { - source, - name, - localName - }; - }); - }); - - this.exportDeclarations.forEach( statement => { - const node = statement.node; - const source = node.source && node.source.value; - - // export default function foo () {} - // export default foo; - // export default 42; - if ( node.type === 'ExportDefaultDeclaration' ) { - const isDeclaration = /Declaration$/.test( node.declaration.type ); - const isAnonymous = /(?:Class|Function)Expression$/.test( node.declaration.type ); - - const declaredName = isDeclaration && node.declaration.id.name; - const identifier = node.declaration.type === 'Identifier' && node.declaration.name; - - this.exports.default = { + this.exports[ name ] = { statement, - name: 'default', - localName: declaredName || 'default', - declaredName, - identifier, - isDeclaration, - isAnonymous, - isModified: false // in case of `export default foo; foo = somethingElse` + localName: name, + expression: declaration }; } + } + } - // export { foo, bar, baz } - // export var foo = 42; - // export function foo () {} - else if ( node.type === 'ExportNamedDeclaration' ) { - if ( node.specifiers.length ) { - // export { foo, bar, baz } - node.specifiers.forEach( specifier => { - const localName = specifier.local.name; - const exportedName = specifier.exported.name; - - this.exports[ exportedName ] = { - localName, - exportedName - }; + addImport ( statement ) { + const node = statement.node; + const source = node.source.value; - // export { foo } from './foo'; - if ( source ) { - this.imports[ localName ] = { - source, - localName, - name: localName - }; - } - }); - } + node.specifiers.forEach( specifier => { + const isDefault = specifier.type === 'ImportDefaultSpecifier'; + const isNamespace = specifier.type === 'ImportNamespaceSpecifier'; - else { - let declaration = node.declaration; + const localName = specifier.local.name; + const name = isDefault ? 'default' : isNamespace ? '*' : specifier.imported.name; - let name; + if ( this.imports[ localName ] ) { + const err = new Error( `Duplicated import '${localName}'` ); + err.file = this.id; + err.loc = getLocation( this.source, specifier.start ); + throw err; + } - if ( declaration.type === 'VariableDeclaration' ) { - // export var foo = 42 - name = declaration.declarations[0].id.name; - } else { - // export function foo () {} - name = declaration.id.name; - } + this.imports[ localName ] = { + source, + name, + localName + }; + }); + } - this.exports[ name ] = { - statement, - localName: name, - expression: declaration - }; - } - } + analyse () { + // discover this module's imports and exports + this.statements.forEach( statement => { + if ( isImportDeclaration( statement ) ) this.addImport( statement ); + else if ( isExportDeclaration( statement ) ) this.addExport( statement ); }); analyse( this.magicString, this ); - this.canonicalNames = blank(); - - this.definitions = blank(); - this.definitionPromises = blank(); - this.modifications = blank(); - + // consolidate names that are defined/modified in this module this.statements.forEach( statement => { keys( statement.defines ).forEach( name => { this.definitions[ name ] = statement; }); keys( statement.modifies ).forEach( name => { - if ( !this.modifications[ name ] ) { - this.modifications[ name ] = []; - } - - this.modifications[ name ].push( statement ); + ( this.modifications[ name ] || ( this.modifications[ name ] = [] ) ).push( statement ); }); }); + // if names are referenced that are neither defined nor imported + // in this module, we assume that they're globals this.statements.forEach( statement => { keys( statement.dependsOn ).forEach( name => { if ( !this.definitions[ name ] && !this.imports[ name ] ) { @@ -507,6 +456,66 @@ export default class Module { return this.canonicalNames[ localName ]; } + // TODO rename this to parse, once https://github.com/rollup/rollup/issues/42 is fixed + _parse () { + // Try to extract a list of top-level statements/declarations. If + // the parse fails, attach file info and abort + let ast; + + try { + ast = parse( this.source, { + ecmaVersion: 6, + sourceType: 'module', + onComment: ( block, text, start, end ) => this.comments.push({ block, text, start, end }) + }); + } catch ( err ) { + err.code = 'PARSE_ERROR'; + err.file = this.id; // see above - not necessarily true, but true enough + throw err; + } + + walk( ast, { + enter: node => { + this.magicString.addSourcemapLocation( node.start ); + this.magicString.addSourcemapLocation( node.end ); + } + }); + + let statements = []; + + ast.body.map( node => { + // special case - top-level var declarations with multiple declarators + // should be split up. Otherwise, we may end up including code we + // don't need, just because an unwanted declarator is included + if ( node.type === 'VariableDeclaration' && node.declarations.length > 1 ) { + node.declarations.forEach( declarator => { + const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); + magicString.prepend( `${node.kind} ` ).append( ';' ); + + const syntheticNode = { + type: 'VariableDeclaration', + kind: node.kind, + start: node.start, + end: node.end, + declarations: [ declarator ] + }; + + const statement = new Statement( syntheticNode, magicString, this, statements.length ); + statements.push( statement ); + }); + } + + else { + const magicString = this.magicString.snip( node.start, node.end ).trim(); + const statement = new Statement( node, magicString, this, statements.length ); + + statements.push( statement ); + } + }); + + return statements; + } + rename ( name, replacement ) { this.canonicalNames[ name ] = replacement; } From e17076250cbcc4c292e3638612ae5e6422aa74b6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 14 Jul 2015 17:37:10 -0400 Subject: [PATCH 4/5] mark statements rather than expanding them --- src/Bundle.js | 33 ++++++++++++++++++++++++++++++++- src/Module.js | 42 ++++++++++-------------------------------- src/Statement.js | 46 +++++----------------------------------------- 3 files changed, 47 insertions(+), 74 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 88f8f37..9732463 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -84,7 +84,10 @@ export default class Bundle { } } - return entryModule.expandAllStatements( true ); + return entryModule.markAllStatements( true ); + }) + .then( () => { + return this.markAllModifierStatements(); }) .then( () => { this.statements = this.sort(); @@ -404,6 +407,34 @@ export default class Bundle { return { code, map }; } + markAllModifierStatements () { + let settled = true; + let promises = []; + + this.modules.forEach( module => { + module.statements.forEach( statement => { + if ( statement.isIncluded ) return; + + keys( statement.modifies ).forEach( name => { + const definingStatement = module.definitions[ name ]; + const exportDeclaration = module.exports[ name ]; + + const shouldMark = ( definingStatement && definingStatement.isIncluded ) || + ( exportDeclaration && exportDeclaration.isUsed ); + + if ( shouldMark ) { + settled = false; + promises.push( statement.mark() ); + } + }); + }); + }); + + return Promise.all( promises ).then( () => { + if ( !settled ) return this.markAllModifierStatements(); + }); + } + sort () { let seen = {}; let ordered = []; diff --git a/src/Module.js b/src/Module.js index efeebbe..a99ce0e 100644 --- a/src/Module.js +++ b/src/Module.js @@ -220,7 +220,7 @@ export default class Module { return { strongDependencies, weakDependencies }; } - define ( name ) { + mark ( name ) { // shortcut cycles. TODO this won't work everywhere... if ( this.definitionPromises[ name ] ) { return emptyArrayPromise; @@ -272,7 +272,7 @@ export default class Module { this.bundle.internalNamespaceModules.push( module ); } - return module.expandAllStatements(); + return module.markAllStatements(); } const exportDeclaration = module.exports[ importDeclaration.name ]; @@ -281,7 +281,7 @@ export default class Module { throw new Error( `Module ${module.id} does not export ${importDeclaration.name} (imported by ${this.id})` ); } - return module.define( exportDeclaration.localName ); + return module.mark( exportDeclaration.localName ); }); } @@ -289,14 +289,14 @@ export default class Module { else if ( name === 'default' && this.exports.default.isDeclaration ) { // We have something like `export default foo` - so we just start again, // searching for `foo` instead of default - promise = this.define( this.exports.default.name ); + promise = this.mark( this.exports.default.name ); } else { let statement; statement = name === 'default' ? this.exports.default.statement : this.definitions[ name ]; - promise = statement && !statement.isIncluded ? statement.expand() : emptyArrayPromise; + promise = statement && !statement.isIncluded ? statement.mark() : emptyArrayPromise; // Special case - `export default foo; foo += 1` - need to be // vigilant about maintaining the correct order of the export @@ -331,22 +331,9 @@ export default class Module { return this.definitionPromises[ name ]; } - expandAllStatements ( isEntryModule ) { - let allStatements = []; - + markAllStatements ( isEntryModule ) { return sequence( this.statements, statement => { - // A statement may have already been included, in which case we need to - // curb rollup's enthusiasm and move it down here. It remains to be seen - // if this approach is bulletproof - if ( statement.isIncluded ) { - const index = allStatements.indexOf( statement ); - if ( ~index ) { - allStatements.splice( index, 1 ); - allStatements.push( statement ); - } - - return; - } + if ( statement.isIncluded ) return; // TODO can this happen? probably not... // skip import declarations... if ( statement.isImportDeclaration ) { @@ -356,10 +343,7 @@ export default class Module { return this.bundle.fetchModule( statement.node.source.value, this.id ) .then( module => { statement.module = module; - return module.expandAllStatements(); - }) - .then( statements => { - allStatements.push.apply( allStatements, statements ); + return module.markAllStatements(); }); } @@ -370,20 +354,14 @@ export default class Module { if ( statement.node.type === 'ExportNamedDeclaration' && statement.node.specifiers.length ) { // ...but ensure they are defined, if this is the entry module if ( isEntryModule ) { - return statement.expand().then( statements => { - allStatements.push.apply( allStatements, statements ); - }); + return statement.mark(); } return; } // include everything else - return statement.expand().then( statements => { - allStatements.push.apply( allStatements, statements ); - }); - }).then( () => { - return allStatements; + return statement.mark(); }); } diff --git a/src/Statement.js b/src/Statement.js index 3cfc4a9..96d47c8 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -234,52 +234,16 @@ export default class Statement { } } - expand () { - this.isIncluded = true; // prevent statement being included twice + mark () { + if ( this.included ) return; // prevent infinite loops + this.isIncluded = true; - let result = []; - - // We have a statement, and it hasn't been included yet. First, include - // the statements it depends on const dependencies = Object.keys( this.dependsOn ); return sequence( dependencies, name => { if ( this.defines[ name ] ) return; // TODO maybe exclude from `this.dependsOn` in the first place? - - return this.module.define( name ).then( definition => { - result.push.apply( result, definition ); - }); - }) - - // then include the statement itself - .then( () => { - result.push( this ); - }) - - // then include any statements that could modify the - // thing(s) this statement defines - .then( () => { - return sequence( keys( this.defines ), name => { - const modifications = this.module.modifications[ name ]; - - if ( modifications ) { - return sequence( modifications, statement => { - if ( !statement.isIncluded ) { - return statement.expand() - .then( statements => { - result.push.apply( result, statements ); - }); - } - }); - } - }); - }) - - // the `result` is an array of all statements that need - // to be included if this one is - .then( () => { - return result; - }); + return this.module.mark( name ); + }); } replaceIdentifiers ( names, bundleExports ) { From 412544268c613b18a6ca8f83322928f102774c48 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 14 Jul 2015 17:59:44 -0400 Subject: [PATCH 5/5] include side-effects (#40) --- src/Bundle.js | 21 +++ src/Module.js | 153 ++++++++++-------- .../_config.js | 2 +- 3 files changed, 106 insertions(+), 70 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 9732463..e461c27 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -425,7 +425,28 @@ export default class Bundle { if ( shouldMark ) { settled = false; promises.push( statement.mark() ); + return; } + + // special case - https://github.com/rollup/rollup/pull/40 + const importDeclaration = module.imports[ name ]; + if ( !importDeclaration ) return; + + const promise = Promise.resolve( importDeclaration.module || this.fetchModule( importDeclaration.source, module.id ) ) + .then( module => { + importDeclaration.module = module; + const exportDeclaration = module.exports[ importDeclaration.name ]; + // TODO things like `export default a + b` don't apply here... right? + return module.findDefiningStatement( exportDeclaration.localName ); + }) + .then( definingStatement => { + if ( !definingStatement ) return; + + settled = false; + return statement.mark(); + }); + + promises.push( promise ); }); }); }); diff --git a/src/Module.js b/src/Module.js index a99ce0e..0308278 100644 --- a/src/Module.js +++ b/src/Module.js @@ -220,6 +220,90 @@ export default class Module { return { strongDependencies, weakDependencies }; } + findDefiningStatement ( name ) { + if ( this.definitions[ name ] ) return this.definitions[ name ]; + + // TODO what about `default`/`*`? + + const importDeclaration = this.imports[ name ]; + if ( !importDeclaration ) return null; + + return Promise.resolve( importDeclaration.module || this.bundle.fetchModule( importDeclaration.source, this.id ) ) + .then( module => { + importDeclaration.module = module; + return module.findDefiningStatement( name ); + }); + } + + findDeclaration ( localName ) { + const importDeclaration = this.imports[ localName ]; + + // name was defined by another module + if ( importDeclaration ) { + const module = importDeclaration.module; + + if ( module.isExternal ) return null; + + const exportDeclaration = module.exports[ importDeclaration.name ]; + return module.findDeclaration( exportDeclaration.localName ); + } + + // name was defined by this module, if any + let i = this.statements.length; + while ( i-- ) { + const declaration = this.statements[i].scope.declarations[ localName ]; + if ( declaration ) { + return declaration; + } + } + + return null; + } + + getCanonicalName ( localName ) { + // Special case + if ( localName === 'default' && ( this.exports.default.isModified || !this.suggestedNames.default ) ) { + let canonicalName = makeLegalIdentifier( this.id.replace( dirname( this.bundle.entryModule.id ) + '/', '' ).replace( /\.js$/, '' ) ); + return deconflict( canonicalName, this.definitions ); + } + + if ( this.suggestedNames[ localName ] ) { + localName = this.suggestedNames[ localName ]; + } + + if ( !this.canonicalNames[ localName ] ) { + let canonicalName; + + if ( this.imports[ localName ] ) { + const importDeclaration = this.imports[ localName ]; + const module = importDeclaration.module; + + if ( importDeclaration.name === '*' ) { + canonicalName = module.suggestedNames[ '*' ]; + } else { + let exporterLocalName; + + if ( module.isExternal ) { + exporterLocalName = importDeclaration.name; + } else { + const exportDeclaration = module.exports[ importDeclaration.name ]; + exporterLocalName = exportDeclaration.localName; + } + + canonicalName = module.getCanonicalName( exporterLocalName ); + } + } + + else { + canonicalName = localName; + } + + this.canonicalNames[ localName ] = canonicalName; + } + + return this.canonicalNames[ localName ]; + } + mark ( name ) { // shortcut cycles. TODO this won't work everywhere... if ( this.definitionPromises[ name ] ) { @@ -365,75 +449,6 @@ export default class Module { }); } - findDeclaration ( localName ) { - const importDeclaration = this.imports[ localName ]; - - // name was defined by another module - if ( importDeclaration ) { - const module = importDeclaration.module; - - if ( module.isExternal ) return null; - - const exportDeclaration = module.exports[ importDeclaration.name ]; - return module.findDeclaration( exportDeclaration.localName ); - } - - // name was defined by this module, if any - let i = this.statements.length; - while ( i-- ) { - const declaration = this.statements[i].scope.declarations[ localName ]; - if ( declaration ) { - return declaration; - } - } - - return null; - } - - getCanonicalName ( localName ) { - // Special case - if ( localName === 'default' && ( this.exports.default.isModified || !this.suggestedNames.default ) ) { - let canonicalName = makeLegalIdentifier( this.id.replace( dirname( this.bundle.entryModule.id ) + '/', '' ).replace( /\.js$/, '' ) ); - return deconflict( canonicalName, this.definitions ); - } - - if ( this.suggestedNames[ localName ] ) { - localName = this.suggestedNames[ localName ]; - } - - if ( !this.canonicalNames[ localName ] ) { - let canonicalName; - - if ( this.imports[ localName ] ) { - const importDeclaration = this.imports[ localName ]; - const module = importDeclaration.module; - - if ( importDeclaration.name === '*' ) { - canonicalName = module.suggestedNames[ '*' ]; - } else { - let exporterLocalName; - - if ( module.isExternal ) { - exporterLocalName = importDeclaration.name; - } else { - const exportDeclaration = module.exports[ importDeclaration.name ]; - exporterLocalName = exportDeclaration.localName; - } - - canonicalName = module.getCanonicalName( exporterLocalName ); - } - } - - else { - canonicalName = localName; - } - - this.canonicalNames[ localName ] = canonicalName; - } - - return this.canonicalNames[ localName ]; - } - // TODO rename this to parse, once https://github.com/rollup/rollup/issues/42 is fixed _parse () { // Try to extract a list of top-level statements/declarations. If diff --git a/test/function/top-level-side-effects-are-preserved/_config.js b/test/function/top-level-side-effects-are-preserved/_config.js index b121b33..7201d85 100644 --- a/test/function/top-level-side-effects-are-preserved/_config.js +++ b/test/function/top-level-side-effects-are-preserved/_config.js @@ -1,3 +1,3 @@ module.exports = { - description: 'top level side effects are preserved' + description: 'top level side effects are preserved' };