From fc2b77315887a4a0dee42076cc26ca29bac42014 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 13 Jul 2017 08:16:56 -0400 Subject: [PATCH 01/15] move build/watch out into separate modules --- bin/src/index.js | 4 +- bin/src/run/build.js | 49 +++++++++++++ bin/src/{runRollup.js => run/index.js} | 98 ++------------------------ bin/src/run/watch.js | 49 +++++++++++++ 4 files changed, 105 insertions(+), 95 deletions(-) create mode 100644 bin/src/run/build.js rename bin/src/{runRollup.js => run/index.js} (65%) create mode 100644 bin/src/run/watch.js diff --git a/bin/src/index.js b/bin/src/index.js index cbc81238cfd..db432c28ce8 100644 --- a/bin/src/index.js +++ b/bin/src/index.js @@ -1,7 +1,7 @@ import minimist from 'minimist'; import help from './help.md'; import { version } from '../../package.json'; -import runRollup from './runRollup'; +import run from './run/index.js'; const command = minimist( process.argv.slice( 2 ), { alias: { @@ -35,5 +35,5 @@ else if ( command.version ) { } else { - runRollup( command ); + run( command ); } diff --git a/bin/src/run/build.js b/bin/src/run/build.js new file mode 100644 index 00000000000..704711c14db --- /dev/null +++ b/bin/src/run/build.js @@ -0,0 +1,49 @@ +import * as rollup from 'rollup'; +import { handleError } from '../logging.js'; +import SOURCEMAPPING_URL from '../sourceMappingUrl.js'; + +export default function build ( options ) { + return rollup.rollup( options ) + .then( bundle => { + if ( options.dest ) { + return bundle.write( options ); + } + + if ( options.targets ) { + let result = null; + + options.targets.forEach( target => { + result = bundle.write( assign( clone( options ), target ) ); + }); + + return result; + } + + if ( options.sourceMap && options.sourceMap !== 'inline' ) { + handleError({ + code: 'MISSING_OUTPUT_OPTION', + message: 'You must specify an --output (-o) option when creating a file with a sourcemap' + }); + } + + return bundle.generate(options).then( ({ code, map }) => { + if ( options.sourceMap === 'inline' ) { + code += `\n//# ${SOURCEMAPPING_URL}=${map.toUrl()}\n`; + } + + process.stdout.write( code ); + }); + }) + .catch( handleError ); +} + +function clone ( object ) { + return assign( {}, object ); +} + +function assign ( target, source ) { + Object.keys( source ).forEach( key => { + target[ key ] = source[ key ]; + }); + return target; +} \ No newline at end of file diff --git a/bin/src/runRollup.js b/bin/src/run/index.js similarity index 65% rename from bin/src/runRollup.js rename to bin/src/run/index.js index 4b6f6a4c134..288316da238 100644 --- a/bin/src/runRollup.js +++ b/bin/src/run/index.js @@ -2,9 +2,9 @@ import path from 'path'; import { realpathSync } from 'fs'; import * as rollup from 'rollup'; import relative from 'require-relative'; -import chalk from 'chalk'; -import { handleWarning, handleError, stderr } from './logging.js'; -import SOURCEMAPPING_URL from './sourceMappingUrl.js'; +import { handleWarning, handleError } from '../logging.js'; +import build from './build.js'; +import watch from './watch.js'; import { install as installSourcemapSupport } from 'source-map-support'; installSourcemapSupport(); @@ -192,96 +192,8 @@ function execute ( options, command ) { }); if ( command.watch ) { - if ( !options.entry || ( !options.dest && !options.targets ) ) { - handleError({ - code: 'WATCHER_MISSING_INPUT_OR_OUTPUT', - message: 'must specify --input and --output when using rollup --watch' - }); - } - - try { - const watch = relative( 'rollup-watch', process.cwd() ); - const watcher = watch( rollup, options ); - - watcher.on( 'event', event => { - switch ( event.code ) { - case 'STARTING': // TODO this isn't emitted by newer versions of rollup-watch - stderr( 'checking rollup-watch version...' ); - break; - - case 'BUILD_START': - stderr( 'bundling...' ); - break; - - case 'BUILD_END': - stderr( 'bundled in ' + event.duration + 'ms. Watching for changes...' ); - break; - - case 'ERROR': - handleError( event.error, true ); - break; - - default: - stderr( 'unknown event', event ); - } - }); - } catch ( err ) { - if ( err.code === 'MODULE_NOT_FOUND' ) { - handleError({ - code: 'ROLLUP_WATCH_NOT_INSTALLED', - message: 'rollup --watch depends on the rollup-watch package, which could not be found. Install it with npm install -D rollup-watch' - }); - } - - handleError( err ); - } + watch( options ); } else { - bundle( options ).catch( handleError ); + build( options ).catch( handleError ); } } - -function clone ( object ) { - return assign( {}, object ); -} - -function assign ( target, source ) { - Object.keys( source ).forEach( key => { - target[ key ] = source[ key ]; - }); - return target; -} - -function bundle ( options ) { - return rollup.rollup( options ) - .then( bundle => { - if ( options.dest ) { - return bundle.write( options ); - } - - if ( options.targets ) { - let result = null; - - options.targets.forEach( target => { - result = bundle.write( assign( clone( options ), target ) ); - }); - - return result; - } - - if ( options.sourceMap && options.sourceMap !== 'inline' ) { - handleError({ - code: 'MISSING_OUTPUT_OPTION', - message: 'You must specify an --output (-o) option when creating a file with a sourcemap' - }); - } - - return bundle.generate(options).then( ({ code, map }) => { - if ( options.sourceMap === 'inline' ) { - code += `\n//# ${SOURCEMAPPING_URL}=${map.toUrl()}\n`; - } - - process.stdout.write( code ); - }); - }) - .catch( handleError ); -} diff --git a/bin/src/run/watch.js b/bin/src/run/watch.js new file mode 100644 index 00000000000..783c8aba168 --- /dev/null +++ b/bin/src/run/watch.js @@ -0,0 +1,49 @@ +import * as rollup from 'rollup'; +import relative from 'require-relative'; +import { handleError, stderr } from '../logging.js'; + +export default function watch ( options ) { + if ( !options.entry || ( !options.dest && !options.targets ) ) { + handleError({ + code: 'WATCHER_MISSING_INPUT_OR_OUTPUT', + message: 'must specify --input and --output when using rollup --watch' + }); + } + + try { + const watch = relative( 'rollup-watch', process.cwd() ); + const watcher = watch( rollup, options ); + + watcher.on( 'event', event => { + switch ( event.code ) { + case 'STARTING': // TODO this isn't emitted by newer versions of rollup-watch + stderr( 'checking rollup-watch version...' ); + break; + + case 'BUILD_START': + stderr( 'bundling...' ); + break; + + case 'BUILD_END': + stderr( 'bundled in ' + event.duration + 'ms. Watching for changes...' ); + break; + + case 'ERROR': + handleError( event.error, true ); + break; + + default: + stderr( 'unknown event', event ); + } + }); + } catch ( err ) { + if ( err.code === 'MODULE_NOT_FOUND' ) { + handleError({ + code: 'ROLLUP_WATCH_NOT_INSTALLED', + message: 'rollup --watch depends on the rollup-watch package, which could not be found. Install it with npm install -D rollup-watch' + }); + } + + handleError( err ); + } +} \ No newline at end of file From e7dbc60119361eb61b44337d9ab06da32fb4f1b9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 13 Jul 2017 11:53:23 -0400 Subject: [PATCH 02/15] coalesce warnings in CLI --- bin/src/run/batchWarnings.js | 210 ++++++++++++++++++++++++++++++ bin/src/run/build.js | 9 ++ bin/src/run/index.js | 20 +-- src/Bundle.js | 2 + src/Module.js | 37 ++---- src/ast/nodes/MemberExpression.js | 3 + src/rollup.js | 2 +- 7 files changed, 245 insertions(+), 38 deletions(-) create mode 100644 bin/src/run/batchWarnings.js diff --git a/bin/src/run/batchWarnings.js b/bin/src/run/batchWarnings.js new file mode 100644 index 00000000000..86092cdab24 --- /dev/null +++ b/bin/src/run/batchWarnings.js @@ -0,0 +1,210 @@ +import chalk from 'chalk'; +import { handleWarning } from '../logging.js'; +import relativeId from '../../../src/utils/relativeId.js'; + +export default function batchWarnings () { + const allWarnings = new Map(); + let count = 0; + + return { + add: warning => { + if ( typeof warning === 'string' ) { + warning = { code: 'UNKNOWN', message: warning }; + } + + if ( !allWarnings.has( warning.code ) ) allWarnings.set( warning.code, [] ); + allWarnings.get( warning.code ).push( warning ); + + count += 1; + }, + + flush: () => { + if ( count === 0 ) return; + + const codes = Array.from( allWarnings.keys() ) + .sort( ( a, b ) => { + if ( handlers[a] && handlers[b] ) { + return handlers[a].priority - handlers[b].priority; + } + + if ( handlers[a] ) return -1; + if ( handlers[b] ) return 1; + return allWarnings.get( b ).length - allWarnings.get( a ).length; + }); + + codes.forEach( code => { + const handler = handlers[ code ]; + const warnings = allWarnings.get( code ); + + if ( handler ) { + handler.fn( warnings ); + } else { + warnings.forEach( warning => { + handleWarning( warning ); + }); + } + }); + } + }; +} + +// TODO select sensible priorities +const handlers = { + UNUSED_EXTERNAL_IMPORT: { + priority: 1, + fn: warnings => { + group( 'Unused external imports' ); + warnings.forEach( warning => { + log( `${warning.message}` ); + }); + } + }, + + UNRESOLVED_IMPORT: { + priority: 1, + fn: warnings => { + group( 'Unresolved dependencies' ); + info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency' ); + + const dependencies = new Map(); + warnings.forEach( warning => { + if ( !dependencies.has( warning.source ) ) dependencies.set( warning.source, [] ); + dependencies.get( warning.source ).push( warning.importer ); + }); + + Array.from( dependencies.keys() ).forEach( dependency => { + const importers = dependencies.get( dependency ); + log( `${chalk.bold( dependency )} (imported by ${importers.join( ', ' )})` ); + }); + } + }, + + MISSING_EXPORT: { + priority: 1, + fn: warnings => { + group( 'Missing exports' ); + info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module' ); + + warnings.forEach( warning => { + log( chalk.bold( warning.importer ) ); + log( `${warning.missing} is not exported by ${warning.exporter}` ); + log( chalk.grey( warning.frame ) ); + }); + } + }, + + THIS_IS_UNDEFINED: { + priority: 1, + fn: warnings => { + group( '`this` has been rewritten to `undefined`' ); + info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#this-is-undefined' ); + + const modules = new Map(); + warnings.forEach( warning => { + if ( !modules.has( warning.loc.file ) ) modules.set( warning.loc.file, [] ); + modules.get( warning.loc.file ).push( warning ); + }); + + const allIds = Array.from( modules.keys() ); + const ids = allIds.length > 5 ? allIds.slice( 0, 3 ) : allIds; + + ids.forEach( id => { + const allOccurrences = modules.get( id ); + const occurrences = allOccurrences.length > 5 ? allOccurrences.slice( 0, 3 ) : allOccurrences; + + log( chalk.bold( relativeId( id ) ) ); + log( chalk.grey( occurrences.map( warning => warning.frame ).join( '\n\n' ) ) ); + + if ( allOccurrences.length > occurrences.length ) { + log( `\n...and ${allOccurrences.length - occurrences.length} occurrences` ); + } + }); + + if ( allIds.length > ids.length ) { + log( `\n...and ${allIds.length - ids.length} files` ); + } + } + }, + + EVAL: { + priority: 1, + fn: warnings => { + + } + }, + + NON_EXISTENT_EXPORT: { + priority: 1, + fn: warnings => { + + } + }, + + NAMESPACE_CONFLICT: { + priority: 1, + fn: warnings => { + + } + }, + + DEPRECATED_ES6: { + priority: 1, + fn: warnings => { + + } + }, + + EMPTY_BUNDLE: { + priority: 1, + fn: warnings => { + + } + }, + + MISSING_GLOBAL_NAME: { + priority: 1, + fn: warnings => { + + } + }, + + MISSING_NODE_BUILTINS: { + priority: 1, + fn: warnings => { + + } + }, + + MISSING_FORMAT: { + priority: 1, + fn: warnings => { + + } + }, // TODO make this an error + + SOURCEMAP_BROKEN: { + priority: 1, + fn: warnings => { + + } + }, + + MIXED_EXPORTS: { + priority: 1, + fn: warnings => { + + } + } +}; + +function group ( title ) { + log( `\n${chalk.bold.yellow('(!)')} ${chalk.bold.yellow( title )}` ); +} + +function info ( url ) { + log( chalk.grey( url ) ); +} + +function log ( message ) { + console.warn( message ); // eslint-disable-line no-console +} \ No newline at end of file diff --git a/bin/src/run/build.js b/bin/src/run/build.js index 704711c14db..20ec941718d 100644 --- a/bin/src/run/build.js +++ b/bin/src/run/build.js @@ -1,8 +1,16 @@ import * as rollup from 'rollup'; import { handleError } from '../logging.js'; import SOURCEMAPPING_URL from '../sourceMappingUrl.js'; +import batchWarnings from './batchWarnings.js'; export default function build ( options ) { + let batch; + + if ( !options.onwarn ) { + batch = batchWarnings(); + options.onwarn = batch.add; + } + return rollup.rollup( options ) .then( bundle => { if ( options.dest ) { @@ -34,6 +42,7 @@ export default function build ( options ) { process.stdout.write( code ); }); }) + .then( batch.flush ) .catch( handleError ); } diff --git a/bin/src/run/index.js b/bin/src/run/index.js index 288316da238..54854fca739 100644 --- a/bin/src/run/index.js +++ b/bin/src/run/index.js @@ -3,6 +3,7 @@ import { realpathSync } from 'fs'; import * as rollup from 'rollup'; import relative from 'require-relative'; import { handleWarning, handleError } from '../logging.js'; +import batchWarnings from './batchWarnings.js'; import build from './build.js'; import watch from './watch.js'; @@ -65,14 +66,18 @@ export default function runRollup ( command ) { config = realpathSync( config ); } + const batch = batchWarnings(); + rollup.rollup({ entry: config, external: id => { return (id[0] !== '.' && !path.isAbsolute(id)) || id.slice(-5,id.length) === '.json'; }, - onwarn: handleWarning + onwarn: batch.add }) .then( bundle => { + batch.flush(); + return bundle.generate({ format: 'cjs' }); @@ -169,19 +174,6 @@ function execute ( options, command ) { options.onwarn = () => {}; } - if ( !options.onwarn ) { - const seen = new Set(); - - options.onwarn = warning => { - const str = warning.toString(); - - if ( seen.has( str ) ) return; - seen.add( str ); - - handleWarning( warning ); - }; - } - options.external = external; // Use any options passed through the CLI as overrides. diff --git a/src/Bundle.js b/src/Bundle.js index 884eae76eac..d6e9ace3b04 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -389,6 +389,8 @@ export default class Bundle { this.warn({ code: 'UNRESOLVED_IMPORT', + source, + importer: relativeId( module.id ), message: `'${source}' is imported by ${relativeId( module.id )}, but could not be resolved – treating it as an external dependency`, url: 'https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency' }); diff --git a/src/Module.js b/src/Module.js index bb868548d59..9ba3f205dc3 100644 --- a/src/Module.js +++ b/src/Module.js @@ -181,29 +181,20 @@ export default class Module { } // export { foo, bar, baz } - else { - if ( node.specifiers.length ) { - node.specifiers.forEach( specifier => { - const localName = specifier.local.name; - const exportedName = specifier.exported.name; - - if ( this.exports[ exportedName ] || this.reexports[ exportedName ] ) { - this.error({ - code: 'DUPLICATE_EXPORT', - message: `A module cannot have multiple exports with the same name ('${exportedName}')` - }, specifier.start ); - } - - this.exports[ exportedName ] = { localName }; - }); - } else { - // TODO is this really necessary? `export {}` is valid JS, and - // might be used as a hint that this is indeed a module - this.warn({ - code: 'EMPTY_EXPORT', - message: `Empty export declaration` - }, node.start ); - } + else if ( node.specifiers.length ) { + node.specifiers.forEach( specifier => { + const localName = specifier.local.name; + const exportedName = specifier.exported.name; + + if ( this.exports[ exportedName ] || this.reexports[ exportedName ] ) { + this.error({ + code: 'DUPLICATE_EXPORT', + message: `A module cannot have multiple exports with the same name ('${exportedName}')` + }, specifier.start ); + } + + this.exports[ exportedName ] = { localName }; + }); } } diff --git a/src/ast/nodes/MemberExpression.js b/src/ast/nodes/MemberExpression.js index 5ab2954444d..23a007e986a 100644 --- a/src/ast/nodes/MemberExpression.js +++ b/src/ast/nodes/MemberExpression.js @@ -45,6 +45,9 @@ export default class MemberExpression extends Node { if ( !declaration ) { this.module.warn({ code: 'MISSING_EXPORT', + missing: part.name || part.value, + importer: relativeId( this.module.id ), + exporter: relativeId( exporterId ), message: `'${part.name || part.value}' is not exported by '${relativeId( exporterId )}'`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` }, part.start ); diff --git a/src/rollup.js b/src/rollup.js index 732cb5eab52..12eb5d44d6c 100644 --- a/src/rollup.js +++ b/src/rollup.js @@ -94,7 +94,7 @@ export function rollup ( options ) { function generate ( options = {} ) { if ( !options.format ) { - bundle.warn({ + bundle.warn({ // TODO make this an error code: 'MISSING_FORMAT', message: `No format option was supplied – defaulting to 'es'`, url: `https://github.com/rollup/rollup/wiki/JavaScript-API#format` From c378f20e08aaf87deda0506c1ac97826465b1266 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 13 Jul 2017 14:52:38 -0400 Subject: [PATCH 03/15] various! --- bin/src/run/batchWarnings.js | 37 +++--- bin/src/run/build.js | 20 ++-- bin/src/run/createWatcher.js | 217 +++++++++++++++++++++++++++++++++++ bin/src/run/index.js | 111 ++++-------------- bin/src/run/mergeOptions.js | 84 ++++++++++++++ bin/src/run/watch.js | 41 +++---- bin/src/utils/sequence.js | 14 +++ package-lock.json | 100 +++++++++++++--- package.json | 6 +- rollup.config.cli.js | 3 +- 10 files changed, 473 insertions(+), 160 deletions(-) create mode 100644 bin/src/run/createWatcher.js create mode 100644 bin/src/run/mergeOptions.js create mode 100644 bin/src/utils/sequence.js diff --git a/bin/src/run/batchWarnings.js b/bin/src/run/batchWarnings.js index 86092cdab24..8ed81e5dada 100644 --- a/bin/src/run/batchWarnings.js +++ b/bin/src/run/batchWarnings.js @@ -1,9 +1,9 @@ import chalk from 'chalk'; -import { handleWarning } from '../logging.js'; +import { handleWarning, stderr } from '../logging.js'; import relativeId from '../../../src/utils/relativeId.js'; export default function batchWarnings () { - const allWarnings = new Map(); + let allWarnings = new Map(); let count = 0; return { @@ -44,6 +44,8 @@ export default function batchWarnings () { }); } }); + + allWarnings = new Map(); } }; } @@ -55,7 +57,7 @@ const handlers = { fn: warnings => { group( 'Unused external imports' ); warnings.forEach( warning => { - log( `${warning.message}` ); + stderr( `${warning.message}` ); }); } }, @@ -74,7 +76,7 @@ const handlers = { Array.from( dependencies.keys() ).forEach( dependency => { const importers = dependencies.get( dependency ); - log( `${chalk.bold( dependency )} (imported by ${importers.join( ', ' )})` ); + stderr( `${chalk.bold( dependency )} (imported by ${importers.join( ', ' )})` ); }); } }, @@ -86,9 +88,9 @@ const handlers = { info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module' ); warnings.forEach( warning => { - log( chalk.bold( warning.importer ) ); - log( `${warning.missing} is not exported by ${warning.exporter}` ); - log( chalk.grey( warning.frame ) ); + stderr( chalk.bold( warning.importer ) ); + stderr( `${warning.missing} is not exported by ${warning.exporter}` ); + stderr( chalk.grey( warning.frame ) ); }); } }, @@ -109,19 +111,18 @@ const handlers = { const ids = allIds.length > 5 ? allIds.slice( 0, 3 ) : allIds; ids.forEach( id => { - const allOccurrences = modules.get( id ); - const occurrences = allOccurrences.length > 5 ? allOccurrences.slice( 0, 3 ) : allOccurrences; + const occurrences = modules.get( id ); - log( chalk.bold( relativeId( id ) ) ); - log( chalk.grey( occurrences.map( warning => warning.frame ).join( '\n\n' ) ) ); + stderr( chalk.bold( relativeId( id ) ) ); + stderr( chalk.grey( occurrences[0].frame ) ); - if ( allOccurrences.length > occurrences.length ) { - log( `\n...and ${allOccurrences.length - occurrences.length} occurrences` ); + if ( occurrences.length > 1 ) { + stderr( `...and ${occurrences.length - 1} other ${occurrences.length > 2 ? 'occurrences' : 'occurrence'}` ); } }); if ( allIds.length > ids.length ) { - log( `\n...and ${allIds.length - ids.length} files` ); + stderr( `\n...and ${allIds.length - ids.length} other files` ); } } }, @@ -198,13 +199,9 @@ const handlers = { }; function group ( title ) { - log( `\n${chalk.bold.yellow('(!)')} ${chalk.bold.yellow( title )}` ); + stderr( `${chalk.bold.yellow('(!)')} ${chalk.bold.yellow( title )}` ); } function info ( url ) { - log( chalk.grey( url ) ); -} - -function log ( message ) { - console.warn( message ); // eslint-disable-line no-console + stderr( chalk.grey( url ) ); } \ No newline at end of file diff --git a/bin/src/run/build.js b/bin/src/run/build.js index 20ec941718d..055bdae386b 100644 --- a/bin/src/run/build.js +++ b/bin/src/run/build.js @@ -1,15 +1,11 @@ import * as rollup from 'rollup'; -import { handleError } from '../logging.js'; +import chalk from 'chalk'; +import { handleError, stderr } from '../logging.js'; import SOURCEMAPPING_URL from '../sourceMappingUrl.js'; -import batchWarnings from './batchWarnings.js'; -export default function build ( options ) { - let batch; - - if ( !options.onwarn ) { - batch = batchWarnings(); - options.onwarn = batch.add; - } +export default function build ( options, warnings ) { + const start = Date.now(); + stderr( chalk.green( `\n${chalk.bold( options.entry )} → ${chalk.bold( options.dest )}...` ) ); return rollup.rollup( options ) .then( bundle => { @@ -42,7 +38,11 @@ export default function build ( options ) { process.stdout.write( code ); }); }) - .then( batch.flush ) + .then( () => { + warnings.flush(); + stderr( chalk.green( `${chalk.bold( options.dest )} created in ${Date.now() - start}ms\n` ) ); + // stderr( `${chalk.blue( '----------' )}\n` ); + }) .catch( handleError ); } diff --git a/bin/src/run/createWatcher.js b/bin/src/run/createWatcher.js new file mode 100644 index 00000000000..4ec02fa5ff0 --- /dev/null +++ b/bin/src/run/createWatcher.js @@ -0,0 +1,217 @@ +import EventEmitter from 'events'; +import relative from 'require-relative'; +import path from 'path'; +import * as fs from 'fs'; +import createFilter from 'rollup-pluginutils/src/createFilter.js'; +import sequence from '../utils/sequence.js'; + +const opts = { encoding: 'utf-8', persistent: true }; + +let chokidar; + +try { + chokidar = relative( 'chokidar', process.cwd() ); +} catch (err) { + chokidar = null; +} + +class FileWatcher { + constructor ( file, data, callback, chokidarOptions, dispose ) { + const handleWatchEvent = (event) => { + if ( event === 'rename' || event === 'unlink' ) { + this.fsWatcher.close(); + dispose(); + callback(); + } else { + // this is necessary because we get duplicate events... + const contents = fs.readFileSync( file, 'utf-8' ); + if ( contents !== data ) { + data = contents; + callback(); + } + } + }; + + try { + if (chokidarOptions) { + this.fsWatcher = chokidar.watch(file, chokidarOptions).on('all', handleWatchEvent); + } else { + this.fsWatcher = fs.watch( file, opts, handleWatchEvent); + } + + this.fileExists = true; + } catch ( err ) { + if ( err.code === 'ENOENT' ) { + // can't watch files that don't exist (e.g. injected + // by plugins somehow) + this.fileExists = false; + } else { + throw err; + } + } + } + + close () { + this.fsWatcher.close(); + } +} + +export default function watch ( rollup, options ) { + const watchOptions = options.watch || {}; + + if ( 'useChokidar' in watchOptions ) watchOptions.chokidar = watchOptions.useChokidar; + let chokidarOptions = 'chokidar' in watchOptions ? watchOptions.chokidar : !!chokidar; + if ( chokidarOptions ) { + chokidarOptions = Object.assign( chokidarOptions === true ? {} : chokidarOptions, { + ignoreInitial: true + }); + } + + if ( chokidarOptions && !chokidar ) { + throw new Error( `options.watch.chokidar was provided, but chokidar could not be found. Have you installed it?` ); + } + + const watcher = new EventEmitter(); + + const filter = createFilter( watchOptions.include, watchOptions.exclude ); + const dests = options.dest ? [ path.resolve( options.dest ) ] : options.targets.map( target => path.resolve( target.dest ) ); + let filewatchers = new Map(); + + let rebuildScheduled = false; + let building = false; + let watching = false; + let closed = false; + + let timeout; + let cache; + + function triggerRebuild () { + clearTimeout( timeout ); + rebuildScheduled = true; + + timeout = setTimeout( () => { + if ( !building ) build(); + }, 50 ); + } + + function addFileWatchersForModules ( modules ) { + modules.forEach( module => { + let id = module.id; + + // skip plugin helper modules and unwatched files + if ( /\0/.test( id ) ) return; + if ( !filter( id ) ) return; + + try { + id = fs.realpathSync( id ); + } catch ( err ) { + return; + } + + if ( ~dests.indexOf( id ) ) { + throw new Error( 'Cannot import the generated bundle' ); + } + + if ( !filewatchers.has( id ) ) { + const watcher = new FileWatcher( id, module.originalCode, triggerRebuild, chokidarOptions, () => { + filewatchers.delete( id ); + }); + + if ( watcher.fileExists ) filewatchers.set( id, watcher ); + } + }); + } + + function build () { + if ( building || closed ) return; + + rebuildScheduled = false; + + let start = Date.now(); + let initial = !watching; + if ( cache ) options.cache = cache; + + watcher.emit( 'event', { code: 'BUILD_START' }); + + building = true; + + return rollup.rollup( options ) + .then( bundle => { + // Save off bundle for re-use later + cache = bundle; + + if ( !closed ) { + addFileWatchersForModules(bundle.modules); + } + + // Now we're watching + watching = true; + + if ( options.targets ) { + return sequence( options.targets, target => { + const mergedOptions = Object.assign( {}, options, target ); + return bundle.write( mergedOptions ); + }); + } + + return bundle.write( options ); + }) + .then( () => { + watcher.emit( 'event', { + code: 'BUILD_END', + duration: Date.now() - start, + initial + }); + }, error => { + try { + //If build failed, make sure we are still watching those files from the most recent successful build. + addFileWatchersForModules( cache.modules ); + } + catch (e) { + //Ignore if they tried to import the output. We are already inside of a catch (probably caused by that). + } + watcher.emit( 'event', { + code: 'ERROR', + error + }); + }) + .then( () => { + building = false; + if ( rebuildScheduled && !closed ) build(); + }); + } + + // build on next tick, so consumers can listen for BUILD_START + process.nextTick( build ); + + function close () { + if ( closed ) return; + for ( const fw of filewatchers.values() ) { + fw.close(); + } + + process.removeListener('SIGINT', close); + process.removeListener('SIGTERM', close); + process.removeListener('uncaughtException', close); + process.stdin.removeListener('end', close); + + watcher.removeAllListeners(); + closed = true; + } + + watcher.close = close; + + // ctrl-c + process.on('SIGINT', close); + + // killall node + process.on('SIGTERM', close); + + // on error + process.on('uncaughtException', close); + + // in case we ever support stdin! + process.stdin.on('end', close); + + return watcher; +} diff --git a/bin/src/run/index.js b/bin/src/run/index.js index 54854fca739..06ac892c806 100644 --- a/bin/src/run/index.js +++ b/bin/src/run/index.js @@ -3,7 +3,9 @@ import { realpathSync } from 'fs'; import * as rollup from 'rollup'; import relative from 'require-relative'; import { handleWarning, handleError } from '../logging.js'; +import mergeOptions from './mergeOptions.js'; import batchWarnings from './batchWarnings.js'; +import sequence from '../utils/sequence.js'; import build from './build.js'; import watch from './watch.js'; @@ -66,25 +68,23 @@ export default function runRollup ( command ) { config = realpathSync( config ); } - const batch = batchWarnings(); + const warnings = batchWarnings(); rollup.rollup({ entry: config, external: id => { return (id[0] !== '.' && !path.isAbsolute(id)) || id.slice(-5,id.length) === '.json'; }, - onwarn: batch.add + onwarn: warnings.add }) .then( bundle => { - batch.flush(); + warnings.flush(); return bundle.generate({ format: 'cjs' }); }) .then( ({ code }) => { - if ( command.watch ) process.env.ROLLUP_WATCH = 'true'; - // temporarily override require const defaultLoader = require.extensions[ '.js' ]; require.extensions[ '.js' ] = ( m, filename ) => { @@ -96,96 +96,33 @@ export default function runRollup ( command ) { }; const configs = require( config ); - const normalized = Array.isArray( configs ) ? configs : [configs]; - - normalized.forEach(options => { - if ( Object.keys( options ).length === 0 ) { - handleError({ - code: 'MISSING_CONFIG', - message: 'Config file must export an options object', - url: 'https://github.com/rollup/rollup/wiki/Command-Line-Interface#using-a-config-file' - }); - } - - execute( options, command ); - }); + if ( Object.keys( configs ).length === 0 ) { + handleError({ + code: 'MISSING_CONFIG', + message: 'Config file must export an options object, or an array of options objects', + url: 'https://github.com/rollup/rollup/wiki/Command-Line-Interface#using-a-config-file' + }); + } require.extensions[ '.js' ] = defaultLoader; + + const normalized = Array.isArray( configs ) ? configs : [configs]; + return execute( normalized, command ); }) .catch( handleError ); } else { - execute( {}, command ); + return execute( [{}], command ); } } -const equivalents = { - useStrict: 'useStrict', - banner: 'banner', - footer: 'footer', - format: 'format', - globals: 'globals', - id: 'moduleId', - indent: 'indent', - input: 'entry', - intro: 'intro', - legacy: 'legacy', - name: 'moduleName', - output: 'dest', - outro: 'outro', - sourcemap: 'sourceMap', - treeshake: 'treeshake' -}; - -function execute ( options, command ) { - let external; - - const commandExternal = ( command.external || '' ).split( ',' ); - const optionsExternal = options.external; - - if ( command.globals ) { - const globals = Object.create( null ); - - command.globals.split( ',' ).forEach( str => { - const names = str.split( ':' ); - globals[ names[0] ] = names[1]; - - // Add missing Module IDs to external. - if ( commandExternal.indexOf( names[0] ) === -1 ) { - commandExternal.push( names[0] ); - } - }); - - command.globals = globals; - } - - if ( typeof optionsExternal === 'function' ) { - external = id => { - return optionsExternal( id ) || ~commandExternal.indexOf( id ); - }; - } else { - external = ( optionsExternal || [] ).concat( commandExternal ); - } - - if (typeof command.extend !== 'undefined') { - options.extend = command.extend; - } - - if ( command.silent ) { - options.onwarn = () => {}; - } - - options.external = external; - - // Use any options passed through the CLI as overrides. - Object.keys( equivalents ).forEach( cliOption => { - if ( command.hasOwnProperty( cliOption ) ) { - options[ equivalents[ cliOption ] ] = command[ cliOption ]; - } - }); - +function execute ( configs, command ) { if ( command.watch ) { - watch( options ); + process.env.ROLLUP_WATCH = 'true'; + watch( configs, command ); } else { - build( options ).catch( handleError ); + return sequence( config => { + const { options, warnings } = mergeOptions( config, command ); + return build( options, warnings ); + }); } -} +} \ No newline at end of file diff --git a/bin/src/run/mergeOptions.js b/bin/src/run/mergeOptions.js new file mode 100644 index 00000000000..c56e08e98f8 --- /dev/null +++ b/bin/src/run/mergeOptions.js @@ -0,0 +1,84 @@ +import batchWarnings from './batchWarnings.js'; + +const equivalents = { + useStrict: 'useStrict', + banner: 'banner', + footer: 'footer', + format: 'format', + globals: 'globals', + id: 'moduleId', + indent: 'indent', + input: 'entry', + intro: 'intro', + legacy: 'legacy', + name: 'moduleName', + output: 'dest', + outro: 'outro', + sourcemap: 'sourceMap', + treeshake: 'treeshake' +}; + +export default function mergeOptions ( config, command ) { + const options = Object.assign( {}, config ); + + let external; + + const commandExternal = ( command.external || '' ).split( ',' ); + const optionsExternal = options.external; + + if ( command.globals ) { + const globals = Object.create( null ); + + command.globals.split( ',' ).forEach( str => { + const names = str.split( ':' ); + globals[ names[0] ] = names[1]; + + // Add missing Module IDs to external. + if ( commandExternal.indexOf( names[0] ) === -1 ) { + commandExternal.push( names[0] ); + } + }); + + command.globals = globals; + } + + if ( typeof optionsExternal === 'function' ) { + external = id => { + return optionsExternal( id ) || ~commandExternal.indexOf( id ); + }; + } else { + external = ( optionsExternal || [] ).concat( commandExternal ); + } + + if (typeof command.extend !== 'undefined') { + options.extend = command.extend; + } + + const warnings = batchWarnings(); + + if ( command.silent ) { + options.onwarn = () => {}; + } else { + options.onwarn = warnings.add; + + const onwarn = options.onwarn; + if ( onwarn ) { + options.onwarn = warning => { + onwarn( warning, warnings.add ); + }; + } else { + options.onwarn = warnings.add; + } + } + + options.external = external; + + // Use any options passed through the CLI as overrides. + Object.keys( equivalents ).forEach( cliOption => { + if ( command.hasOwnProperty( cliOption ) ) { + options[ equivalents[ cliOption ] ] = command[ cliOption ]; + } + }); + + return { options, warnings }; +} \ No newline at end of file diff --git a/bin/src/run/watch.js b/bin/src/run/watch.js index 783c8aba168..9bd5ff90f33 100644 --- a/bin/src/run/watch.js +++ b/bin/src/run/watch.js @@ -1,18 +1,21 @@ import * as rollup from 'rollup'; -import relative from 'require-relative'; +import chalk from 'chalk'; +import createWatcher from './createWatcher.js'; +import mergeOptions from './mergeOptions.js'; import { handleError, stderr } from '../logging.js'; -export default function watch ( options ) { - if ( !options.entry || ( !options.dest && !options.targets ) ) { - handleError({ - code: 'WATCHER_MISSING_INPUT_OR_OUTPUT', - message: 'must specify --input and --output when using rollup --watch' - }); - } +export default function watch ( configs, command ) { + configs.forEach( config => { + const { options, warnings } = mergeOptions( config, command ); + + if ( !options.entry || ( !options.dest && !options.targets ) ) { + handleError({ + code: 'WATCHER_MISSING_INPUT_OR_OUTPUT', + message: 'must specify --input and --output when using rollup --watch' + }); + } - try { - const watch = relative( 'rollup-watch', process.cwd() ); - const watcher = watch( rollup, options ); + const watcher = createWatcher( rollup, options ); watcher.on( 'event', event => { switch ( event.code ) { @@ -21,11 +24,12 @@ export default function watch ( options ) { break; case 'BUILD_START': - stderr( 'bundling...' ); + stderr( `${chalk.blue.bold( options.entry )} -> ${chalk.blue.bold( options.dest )}...` ); break; case 'BUILD_END': - stderr( 'bundled in ' + event.duration + 'ms. Watching for changes...' ); + warnings.flush(); + stderr( `created ${chalk.blue.bold( options.dest )} in ${event.duration}ms. Watching for changes...` ); break; case 'ERROR': @@ -36,14 +40,5 @@ export default function watch ( options ) { stderr( 'unknown event', event ); } }); - } catch ( err ) { - if ( err.code === 'MODULE_NOT_FOUND' ) { - handleError({ - code: 'ROLLUP_WATCH_NOT_INSTALLED', - message: 'rollup --watch depends on the rollup-watch package, which could not be found. Install it with npm install -D rollup-watch' - }); - } - - handleError( err ); - } + }); } \ No newline at end of file diff --git a/bin/src/utils/sequence.js b/bin/src/utils/sequence.js new file mode 100644 index 00000000000..6f0db0c3ed3 --- /dev/null +++ b/bin/src/utils/sequence.js @@ -0,0 +1,14 @@ +export default function sequence ( array, fn ) { + const results = []; + let promise = Promise.resolve(); + + function next ( member, i ) { + return fn( member ).then( value => results[i] = value ); + } + + for ( let i = 0; i < array.length; i += 1 ) { + promise = promise.then( () => next( array[i], i ) ); + } + + return promise.then( () => results ); +} diff --git a/package-lock.json b/package-lock.json index 089b8bde5ef..500e3f2c595 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "rollup", - "version": "0.45.1", + "version": "0.45.2", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -405,6 +405,17 @@ "has-ansi": "2.0.0", "strip-ansi": "3.0.1", "supports-color": "2.0.0" + }, + "dependencies": { + "strip-ansi": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", + "dev": true, + "requires": { + "ansi-regex": "2.1.1" + } + } } }, "chokidar": { @@ -2409,6 +2420,17 @@ "string-width": "1.0.2", "strip-ansi": "3.0.1", "through": "2.3.8" + }, + "dependencies": { + "strip-ansi": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", + "dev": true, + "requires": { + "ansi-regex": "2.1.1" + } + } } }, "interpret": { @@ -3811,6 +3833,18 @@ "requires": { "buble": "0.15.2", "rollup-pluginutils": "1.5.2" + }, + "dependencies": { + "rollup-pluginutils": { + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-1.5.2.tgz", + "integrity": "sha1-HhVud4+UtyVb+hs9AXi+j1xVJAg=", + "dev": true, + "requires": { + "estree-walker": "0.2.1", + "minimatch": "3.0.4" + } + } } }, "rollup-plugin-commonjs": { @@ -3917,6 +3951,16 @@ "requires": { "vlq": "0.2.2" } + }, + "rollup-pluginutils": { + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-1.5.2.tgz", + "integrity": "sha1-HhVud4+UtyVb+hs9AXi+j1xVJAg=", + "dev": true, + "requires": { + "estree-walker": "0.2.1", + "minimatch": "3.0.4" + } } } }, @@ -3927,16 +3971,36 @@ "dev": true, "requires": { "rollup-pluginutils": "1.5.2" + }, + "dependencies": { + "rollup-pluginutils": { + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-1.5.2.tgz", + "integrity": "sha1-HhVud4+UtyVb+hs9AXi+j1xVJAg=", + "dev": true, + "requires": { + "estree-walker": "0.2.1", + "minimatch": "3.0.4" + } + } } }, "rollup-pluginutils": { - "version": "1.5.2", - "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-1.5.2.tgz", - "integrity": "sha1-HhVud4+UtyVb+hs9AXi+j1xVJAg=", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-2.0.1.tgz", + "integrity": "sha1-fslbNXP2VDpGpkYb2afFRFJdD8A=", "dev": true, "requires": { - "estree-walker": "0.2.1", - "minimatch": "3.0.4" + "estree-walker": "0.3.1", + "micromatch": "2.3.11" + }, + "dependencies": { + "estree-walker": { + "version": "0.3.1", + "resolved": "https://registry.npmjs.org/estree-walker/-/estree-walker-0.3.1.tgz", + "integrity": "sha1-5rGlHPcpJSTnI3wxLl/mZgwc4ao=", + "dev": true + } } }, "rollup-watch": { @@ -4048,12 +4112,14 @@ "source-map": { "version": "0.5.6", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.5.6.tgz", - "integrity": "sha1-dc449SvwczxafwwRjYEzSiu19BI=" + "integrity": "sha1-dc449SvwczxafwwRjYEzSiu19BI=", + "dev": true }, "source-map-support": { "version": "0.4.15", "resolved": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.4.15.tgz", "integrity": "sha1-AyAt9lwG0r2MfsI2KhkwVv7407E=", + "dev": true, "requires": { "source-map": "0.5.6" } @@ -4133,6 +4199,17 @@ "code-point-at": "1.1.0", "is-fullwidth-code-point": "1.0.0", "strip-ansi": "3.0.1" + }, + "dependencies": { + "strip-ansi": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", + "dev": true, + "requires": { + "ansi-regex": "2.1.1" + } + } } }, "stringstream": { @@ -4142,15 +4219,6 @@ "dev": true, "optional": true }, - "strip-ansi": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", - "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", - "dev": true, - "requires": { - "ansi-regex": "2.1.1" - } - }, "strip-bom": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/strip-bom/-/strip-bom-3.0.0.tgz", diff --git a/package.json b/package.json index f2a44f3d1ba..c797aa6346c 100644 --- a/package.json +++ b/package.json @@ -67,15 +67,15 @@ "rollup-plugin-node-resolve": "^3.0.0", "rollup-plugin-replace": "^1.1.0", "rollup-plugin-string": "^2.0.0", + "rollup-pluginutils": "^2.0.1", "rollup-watch": "^4.3.1", "sander": "^0.6.0", "source-map": "^0.5.6", + "source-map-support": "^0.4.15", "sourcemap-codec": "^1.3.0", "uglify-js": "^3.0.19" }, - "dependencies": { - "source-map-support": "^0.4.0" - }, + "dependencies": {}, "files": [ "dist", "bin/rollup", diff --git a/rollup.config.cli.js b/rollup.config.cli.js index d8dae2a69eb..a6902314a59 100644 --- a/rollup.config.cli.js +++ b/rollup.config.cli.js @@ -12,7 +12,7 @@ export default { plugins: [ string({ include: '**/*.md' }), json(), - buble(), + buble({ target: { node: 4 } }), commonjs({ include: 'node_modules/**' }), @@ -24,6 +24,7 @@ export default { 'fs', 'path', 'module', + 'events', 'source-map-support', 'rollup' ], From 43260b4e20434e9e2d5827e6d3b7e7fe349190fd Mon Sep 17 00:00:00 2001 From: Jeff Jewiss Date: Thu, 20 Jul 2017 00:35:55 +0100 Subject: [PATCH 04/15] adds failing tests for linewrap semi-colon removal --- test/form/line-wrap/_config.js | 3 +++ test/form/line-wrap/_expected/amd.js | 5 +++++ test/form/line-wrap/_expected/cjs.js | 3 +++ test/form/line-wrap/_expected/es.js | 1 + test/form/line-wrap/_expected/iife.js | 5 +++++ test/form/line-wrap/_expected/umd.js | 9 +++++++++ test/form/line-wrap/main.js | 1 + 7 files changed, 27 insertions(+) create mode 100644 test/form/line-wrap/_config.js create mode 100644 test/form/line-wrap/_expected/amd.js create mode 100644 test/form/line-wrap/_expected/cjs.js create mode 100644 test/form/line-wrap/_expected/es.js create mode 100644 test/form/line-wrap/_expected/iife.js create mode 100644 test/form/line-wrap/_expected/umd.js create mode 100644 test/form/line-wrap/main.js diff --git a/test/form/line-wrap/_config.js b/test/form/line-wrap/_config.js new file mode 100644 index 00000000000..9a0504cf967 --- /dev/null +++ b/test/form/line-wrap/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'use a newline for line wraps removing a necessary semicolon (#1275)' +}; diff --git a/test/form/line-wrap/_expected/amd.js b/test/form/line-wrap/_expected/amd.js new file mode 100644 index 00000000000..65f8e046cf7 --- /dev/null +++ b/test/form/line-wrap/_expected/amd.js @@ -0,0 +1,5 @@ +define(function () { 'use strict'; + + for(var x=1;x<2;x++)var d=x|0;console.log(d); + +}); diff --git a/test/form/line-wrap/_expected/cjs.js b/test/form/line-wrap/_expected/cjs.js new file mode 100644 index 00000000000..9398f8f1bbd --- /dev/null +++ b/test/form/line-wrap/_expected/cjs.js @@ -0,0 +1,3 @@ +'use strict'; + +for(var x=1;x<2;x++)var d=x|0;console.log(d); diff --git a/test/form/line-wrap/_expected/es.js b/test/form/line-wrap/_expected/es.js new file mode 100644 index 00000000000..4f2606873e9 --- /dev/null +++ b/test/form/line-wrap/_expected/es.js @@ -0,0 +1 @@ +for(var x=1;x<2;x++)var d=x|0;console.log(d); diff --git a/test/form/line-wrap/_expected/iife.js b/test/form/line-wrap/_expected/iife.js new file mode 100644 index 00000000000..85c3785af90 --- /dev/null +++ b/test/form/line-wrap/_expected/iife.js @@ -0,0 +1,5 @@ +(function () { + 'use strict'; + + for(var x=1;x<2;x++)var d=x|0;console.log(d); +}()); diff --git a/test/form/line-wrap/_expected/umd.js b/test/form/line-wrap/_expected/umd.js new file mode 100644 index 00000000000..c50f8c4a11b --- /dev/null +++ b/test/form/line-wrap/_expected/umd.js @@ -0,0 +1,9 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + (factory()); +}(this, (function () { 'use strict'; + + for(var x=1;x<2;x++)var d=x|0;console.log(d); + +}))); diff --git a/test/form/line-wrap/main.js b/test/form/line-wrap/main.js new file mode 100644 index 00000000000..4f2606873e9 --- /dev/null +++ b/test/form/line-wrap/main.js @@ -0,0 +1 @@ +for(var x=1;x<2;x++)var d=x|0;console.log(d); From 723f6a950840dbb38adfed158e64642d099829c5 Mon Sep 17 00:00:00 2001 From: Jeff Jewiss Date: Thu, 20 Jul 2017 00:41:37 +0100 Subject: [PATCH 05/15] introduce a newline when a semicolon is not needed --- src/ast/nodes/VariableDeclaration.js | 2 +- test/form/line-wrap/_expected/amd.js | 3 ++- test/form/line-wrap/_expected/cjs.js | 3 ++- test/form/line-wrap/_expected/es.js | 3 ++- test/form/line-wrap/_expected/iife.js | 4 +++- test/form/line-wrap/_expected/umd.js | 3 ++- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/ast/nodes/VariableDeclaration.js b/src/ast/nodes/VariableDeclaration.js index 77f4433d67d..2c21130406f 100644 --- a/src/ast/nodes/VariableDeclaration.js +++ b/src/ast/nodes/VariableDeclaration.js @@ -98,7 +98,7 @@ export default class VariableDeclaration extends Node { const needsSemicolon = !forStatement.test( this.parent.type ); if ( this.end > c ) { - code.overwrite( c, this.end, needsSemicolon ? ';' : '' ); + code.overwrite( c, this.end, needsSemicolon ? ';' : '\n' ); } else if ( needsSemicolon ) { this.insertSemicolon( code ); } diff --git a/test/form/line-wrap/_expected/amd.js b/test/form/line-wrap/_expected/amd.js index 65f8e046cf7..61908d3ec40 100644 --- a/test/form/line-wrap/_expected/amd.js +++ b/test/form/line-wrap/_expected/amd.js @@ -1,5 +1,6 @@ define(function () { 'use strict'; - for(var x=1;x<2;x++)var d=x|0;console.log(d); + for(var x=1;x<2;x++)var d=x|0 + console.log(d); }); diff --git a/test/form/line-wrap/_expected/cjs.js b/test/form/line-wrap/_expected/cjs.js index 9398f8f1bbd..f73746adbbf 100644 --- a/test/form/line-wrap/_expected/cjs.js +++ b/test/form/line-wrap/_expected/cjs.js @@ -1,3 +1,4 @@ 'use strict'; -for(var x=1;x<2;x++)var d=x|0;console.log(d); +for(var x=1;x<2;x++)var d=x|0 +console.log(d); diff --git a/test/form/line-wrap/_expected/es.js b/test/form/line-wrap/_expected/es.js index 4f2606873e9..3bf9930a40c 100644 --- a/test/form/line-wrap/_expected/es.js +++ b/test/form/line-wrap/_expected/es.js @@ -1 +1,2 @@ -for(var x=1;x<2;x++)var d=x|0;console.log(d); +for(var x=1;x<2;x++)var d=x|0 +console.log(d); diff --git a/test/form/line-wrap/_expected/iife.js b/test/form/line-wrap/_expected/iife.js index 85c3785af90..d65242fb196 100644 --- a/test/form/line-wrap/_expected/iife.js +++ b/test/form/line-wrap/_expected/iife.js @@ -1,5 +1,7 @@ (function () { 'use strict'; - for(var x=1;x<2;x++)var d=x|0;console.log(d); + for(var x=1;x<2;x++)var d=x|0 + console.log(d); + }()); diff --git a/test/form/line-wrap/_expected/umd.js b/test/form/line-wrap/_expected/umd.js index c50f8c4a11b..b65bbb30135 100644 --- a/test/form/line-wrap/_expected/umd.js +++ b/test/form/line-wrap/_expected/umd.js @@ -4,6 +4,7 @@ (factory()); }(this, (function () { 'use strict'; - for(var x=1;x<2;x++)var d=x|0;console.log(d); + for(var x=1;x<2;x++)var d=x|0 + console.log(d); }))); From a991ea70c2d3018e95476c71c2fe4c0987f02971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Fri, 21 Jul 2017 10:46:59 +0200 Subject: [PATCH 06/15] Fixed namespaced named exports not being returned from the iife closure correctly (fixes #1492) --- src/finalisers/iife.js | 2 +- test/form/module-name-wat/_expected/iife.js | 2 ++ test/form/namespaced-named-exports/_expected/iife.js | 2 ++ test/form/umd-noconflict-namespaced/_expected/iife.js | 2 ++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/finalisers/iife.js b/src/finalisers/iife.js index 8b557d1ea02..34ed22e019d 100644 --- a/src/finalisers/iife.js +++ b/src/finalisers/iife.js @@ -72,7 +72,7 @@ export default function iife ( bundle, magicString, { exportMode, indentString, let wrapperOutro = `\n\n}(${dependencies}));`; - if (possibleVariableAssignment && exportMode === 'named') { + if (!extend && exportMode === 'named') { wrapperOutro = `\n\n${indentString}return exports;${wrapperOutro}`; } diff --git a/test/form/module-name-wat/_expected/iife.js b/test/form/module-name-wat/_expected/iife.js index 5d377d98182..ac64446b1af 100644 --- a/test/form/module-name-wat/_expected/iife.js +++ b/test/form/module-name-wat/_expected/iife.js @@ -8,4 +8,6 @@ this.foo['@scoped/npm-package'].bar['why-would-you-do-this'] = (function (export exports.foo = foo; + return exports; + }({})); diff --git a/test/form/namespaced-named-exports/_expected/iife.js b/test/form/namespaced-named-exports/_expected/iife.js index 4043b2692f9..5376c55e5d7 100644 --- a/test/form/namespaced-named-exports/_expected/iife.js +++ b/test/form/namespaced-named-exports/_expected/iife.js @@ -7,4 +7,6 @@ this.foo.bar.baz = (function (exports) { exports.answer = answer; + return exports; + }({})); diff --git a/test/form/umd-noconflict-namespaced/_expected/iife.js b/test/form/umd-noconflict-namespaced/_expected/iife.js index 40085ca93b5..201f2df1900 100644 --- a/test/form/umd-noconflict-namespaced/_expected/iife.js +++ b/test/form/umd-noconflict-namespaced/_expected/iife.js @@ -16,4 +16,6 @@ this.my.name.spaced.module = (function (exports) { exports.number = number; exports.setting = setting; + return exports; + }({})); From f9dc50205c54392f250150eac8817cefc8c8ac4d Mon Sep 17 00:00:00 2001 From: Lukas Taegert Date: Mon, 24 Jul 2017 08:00:53 +0200 Subject: [PATCH 07/15] Correctly handle variables introduced in switch scopes --- src/ast/nodes/SwitchStatement.js | 14 ++++++++++++++ src/ast/nodes/index.js | 3 ++- test/form/switch-scopes/_config.js | 6 ++++++ test/form/switch-scopes/_expected/amd.js | 13 +++++++++++++ test/form/switch-scopes/_expected/cjs.js | 11 +++++++++++ test/form/switch-scopes/_expected/es.js | 9 +++++++++ test/form/switch-scopes/_expected/iife.js | 14 ++++++++++++++ test/form/switch-scopes/_expected/umd.js | 17 +++++++++++++++++ test/form/switch-scopes/main.js | 23 +++++++++++++++++++++++ 9 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 src/ast/nodes/SwitchStatement.js create mode 100644 test/form/switch-scopes/_config.js create mode 100644 test/form/switch-scopes/_expected/amd.js create mode 100644 test/form/switch-scopes/_expected/cjs.js create mode 100644 test/form/switch-scopes/_expected/es.js create mode 100644 test/form/switch-scopes/_expected/iife.js create mode 100644 test/form/switch-scopes/_expected/umd.js create mode 100644 test/form/switch-scopes/main.js diff --git a/src/ast/nodes/SwitchStatement.js b/src/ast/nodes/SwitchStatement.js new file mode 100644 index 00000000000..ce5ae9e29fe --- /dev/null +++ b/src/ast/nodes/SwitchStatement.js @@ -0,0 +1,14 @@ +import Scope from '../scopes/Scope.js'; +import Statement from './shared/Statement.js'; + +export default class SwitchStatement extends Statement { + initialise ( scope ) { + this.scope = new Scope( { + parent: scope, + isBlockScope: true, + isLexicalBoundary: false + } ); + + super.initialise( this.scope ); + } +} diff --git a/src/ast/nodes/index.js b/src/ast/nodes/index.js index bfda415448a..0d851bb78ed 100644 --- a/src/ast/nodes/index.js +++ b/src/ast/nodes/index.js @@ -28,6 +28,7 @@ import NewExpression from './NewExpression.js'; import ObjectExpression from './ObjectExpression.js'; import ReturnStatement from './ReturnStatement.js'; import Statement from './shared/Statement.js'; +import SwitchStatement from './SwitchStatement.js'; import TemplateLiteral from './TemplateLiteral.js'; import ThisExpression from './ThisExpression.js'; import ThrowStatement from './ThrowStatement.js'; @@ -67,7 +68,7 @@ export default { NewExpression, ObjectExpression, ReturnStatement, - SwitchStatement: Statement, + SwitchStatement, TemplateLiteral, ThisExpression, ThrowStatement, diff --git a/test/form/switch-scopes/_config.js b/test/form/switch-scopes/_config.js new file mode 100644 index 00000000000..47676bff0d9 --- /dev/null +++ b/test/form/switch-scopes/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'correctly handles switch scopes', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/switch-scopes/_expected/amd.js b/test/form/switch-scopes/_expected/amd.js new file mode 100644 index 00000000000..f8af6ec51dd --- /dev/null +++ b/test/form/switch-scopes/_expected/amd.js @@ -0,0 +1,13 @@ +define(function () { 'use strict'; + + const x = globalFunction; + switch ( anotherGlobal ) { + case 2: + x(); + } + + switch ( globalFunction() ) { + case 4: + } + +}); diff --git a/test/form/switch-scopes/_expected/cjs.js b/test/form/switch-scopes/_expected/cjs.js new file mode 100644 index 00000000000..ee73f2e70d7 --- /dev/null +++ b/test/form/switch-scopes/_expected/cjs.js @@ -0,0 +1,11 @@ +'use strict'; + +const x = globalFunction; +switch ( anotherGlobal ) { + case 2: + x(); +} + +switch ( globalFunction() ) { + case 4: +} diff --git a/test/form/switch-scopes/_expected/es.js b/test/form/switch-scopes/_expected/es.js new file mode 100644 index 00000000000..f5cd80e94e6 --- /dev/null +++ b/test/form/switch-scopes/_expected/es.js @@ -0,0 +1,9 @@ +const x = globalFunction; +switch ( anotherGlobal ) { + case 2: + x(); +} + +switch ( globalFunction() ) { + case 4: +} diff --git a/test/form/switch-scopes/_expected/iife.js b/test/form/switch-scopes/_expected/iife.js new file mode 100644 index 00000000000..f52899caedb --- /dev/null +++ b/test/form/switch-scopes/_expected/iife.js @@ -0,0 +1,14 @@ +(function () { + 'use strict'; + + const x = globalFunction; + switch ( anotherGlobal ) { + case 2: + x(); + } + + switch ( globalFunction() ) { + case 4: + } + +}()); diff --git a/test/form/switch-scopes/_expected/umd.js b/test/form/switch-scopes/_expected/umd.js new file mode 100644 index 00000000000..bed407c91ef --- /dev/null +++ b/test/form/switch-scopes/_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'; + + const x = globalFunction; + switch ( anotherGlobal ) { + case 2: + x(); + } + + switch ( globalFunction() ) { + case 4: + } + +}))); diff --git a/test/form/switch-scopes/main.js b/test/form/switch-scopes/main.js new file mode 100644 index 00000000000..092ec07c8b2 --- /dev/null +++ b/test/form/switch-scopes/main.js @@ -0,0 +1,23 @@ +const x = globalFunction; +function y () {} + +switch ( anotherGlobal ) { + case 1: + const x = function () {}; + x(); +} + +switch ( anotherGlobal ) { + case 2: + x(); +} + +switch ( anotherGlobal ) { + case 3: + const y = globalFunction; +} +y(); + +switch ( globalFunction() ) { + case 4: +} From fa497ad771351e66f9b2350ee8d26065e8fb97d4 Mon Sep 17 00:00:00 2001 From: Lukas Taegert Date: Wed, 2 Aug 2017 06:02:04 +0200 Subject: [PATCH 08/15] Always store scope in nodes and clean up scope handling --- src/Bundle.js | 150 +++++++++++----------- src/Module.js | 86 ++++++------- src/ast/Node.js | 44 ++++--- src/ast/nodes/ArrowFunctionExpression.js | 42 ++---- src/ast/nodes/AssignmentExpression.js | 18 +-- src/ast/nodes/BinaryExpression.js | 2 +- src/ast/nodes/BlockStatement.js | 49 +++---- src/ast/nodes/CallExpression.js | 19 ++- src/ast/nodes/CatchClause.js | 21 ++- src/ast/nodes/ClassDeclaration.js | 35 ++--- src/ast/nodes/ClassExpression.js | 28 +--- src/ast/nodes/ConditionalExpression.js | 20 ++- src/ast/nodes/ExportAllDeclaration.js | 2 +- src/ast/nodes/ExportDefaultDeclaration.js | 34 +++-- src/ast/nodes/ExportNamedDeclaration.js | 11 +- src/ast/nodes/ForInStatement.js | 27 ++-- src/ast/nodes/ForOfStatement.js | 27 ++-- src/ast/nodes/ForStatement.js | 26 ++-- src/ast/nodes/FunctionDeclaration.js | 37 ++---- src/ast/nodes/FunctionExpression.js | 35 ++--- src/ast/nodes/Identifier.js | 40 +++--- src/ast/nodes/IfStatement.js | 49 ++++--- src/ast/nodes/ImportDeclaration.js | 2 +- src/ast/nodes/Literal.js | 2 +- src/ast/nodes/MemberExpression.js | 18 +-- src/ast/nodes/NewExpression.js | 4 +- src/ast/nodes/ReturnStatement.js | 7 - src/ast/nodes/TemplateLiteral.js | 2 +- src/ast/nodes/ThisExpression.js | 6 +- src/ast/nodes/UnaryExpression.js | 19 ++- src/ast/nodes/UpdateExpression.js | 17 +-- src/ast/nodes/VariableDeclaration.js | 79 ++++++------ src/ast/nodes/VariableDeclarator.js | 23 ++-- src/ast/nodes/index.js | 3 +- src/ast/nodes/shared/Class.js | 32 +++++ src/ast/nodes/shared/Function.js | 33 +++++ src/ast/nodes/shared/Statement.js | 4 +- 37 files changed, 489 insertions(+), 564 deletions(-) delete mode 100644 src/ast/nodes/ReturnStatement.js create mode 100644 src/ast/nodes/shared/Class.js create mode 100644 src/ast/nodes/shared/Function.js diff --git a/src/Bundle.js b/src/Bundle.js index 884eae76eac..216afad745a 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -27,7 +27,7 @@ export default class Bundle { if ( options.cache ) { options.cache.modules.forEach( module => { this.cachedModules.set( module.id, module ); - }); + } ); } this.plugins = ensureArray( options.plugins ); @@ -35,7 +35,7 @@ export default class Bundle { options = this.plugins.reduce( ( acc, plugin ) => { if ( plugin.options ) return plugin.options( acc ) || acc; return acc; - }, options); + }, options ); if ( !options.entry ) { throw new Error( 'You must supply options.entry to rollup' ); @@ -47,13 +47,13 @@ export default class Bundle { this.treeshake = options.treeshake !== false; - if (options.pureExternalModules === true) { + if ( options.pureExternalModules === true ) { this.isPureExternalModule = () => true; - } else if (typeof options.pureExternalModules === 'function') { + } else if ( typeof options.pureExternalModules === 'function' ) { this.isPureExternalModule = options.pureExternalModules; - } else if (Array.isArray(options.pureExternalModules)) { - const pureExternalModules = new Set(options.pureExternalModules); - this.isPureExternalModule = id => pureExternalModules.has(id); + } else if ( Array.isArray( options.pureExternalModules ) ) { + const pureExternalModules = new Set( options.pureExternalModules ); + this.isPureExternalModule = id => pureExternalModules.has( id ); } else { this.isPureExternalModule = () => false; } @@ -81,7 +81,7 @@ export default class Bundle { // TODO strictly speaking, this only applies with non-ES6, non-default-only bundles [ 'module', 'exports', '_interopDefault' ].forEach( name => { this.scope.findDeclaration( name ); // creates global declaration as side-effect - }); + } ); this.moduleById = new Map(); this.modules = []; @@ -96,7 +96,7 @@ export default class Bundle { const moduleContext = new Map(); Object.keys( optionsModuleContext ).forEach( key => { moduleContext.set( resolve( key ), optionsModuleContext[ key ] ); - }); + } ); this.getModuleContext = id => moduleContext.get( id ) || this.context; } else { this.getModuleContext = () => this.context; @@ -125,22 +125,22 @@ export default class Bundle { return this.resolveId( this.entry, undefined ) .then( id => { if ( id === false ) { - error({ + error( { code: 'UNRESOLVED_ENTRY', message: `Entry module cannot be external` - }); + } ); } if ( id == null ) { - error({ + error( { code: 'UNRESOLVED_ENTRY', message: `Could not resolve entry (${this.entry})` - }); + } ); } this.entryId = id; return this.fetchModule( id, undefined ); - }) + } ) .then( entryModule => { this.entryModule = entryModule; @@ -169,13 +169,13 @@ export default class Bundle { if ( declaration.isNamespace ) { declaration.needsNamespaceBlock = true; } - }); + } ); // mark statements that should appear in the bundle if ( this.treeshake ) { this.modules.forEach( module => { module.run(); - }); + } ); let settled = false; while ( !settled ) { @@ -183,7 +183,7 @@ export default class Bundle { let i = this.dependentExpressions.length; while ( i-- ) { - const expression = this.dependentExpressions[i]; + const expression = this.dependentExpressions[ i ]; let statement = expression; while ( statement.parent && !/Function/.test( statement.parent.type ) ) statement = statement.parent; @@ -192,7 +192,7 @@ export default class Bundle { this.dependentExpressions.splice( i, 1 ); } else if ( expression.isUsedByBundle() ) { settled = false; - statement.run( statement.findScope() ); + statement.run(); this.dependentExpressions.splice( i, 1 ); } } @@ -216,25 +216,25 @@ export default class Bundle { if ( unused.length === 0 ) return; const names = unused.length === 1 ? - `'${unused[0]}' is` : + `'${unused[ 0 ]}' is` : `${unused.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${unused.pop()}' are`; - this.warn({ + this.warn( { code: 'UNUSED_EXTERNAL_IMPORT', message: `${names} imported from external module '${module.id}' but never used` - }); - }); + } ); + } ); // prune unused external imports - this.externalModules = this.externalModules.filter(module => { - return module.used || !this.isPureExternalModule(module.id); - }); + this.externalModules = this.externalModules.filter( module => { + return module.used || !this.isPureExternalModule( module.id ); + } ); this.orderedModules = this.sort(); this.deconflict(); timeEnd( 'phase 4' ); - }); + } ); } deconflict () { @@ -245,7 +245,7 @@ export default class Bundle { function getSafeName ( name ) { while ( used[ name ] ) { - name += `$${used[name]++}`; + name += `$${used[ name ]++}`; } used[ name ] = 1; @@ -265,8 +265,8 @@ export default class Bundle { const safeName = getSafeName( name ); toDeshadow.add( safeName ); declaration.setSafeName( safeName ); - }); - }); + } ); + } ); this.modules.forEach( module => { forOwn( module.scope.declarations, ( declaration ) => { @@ -275,14 +275,14 @@ export default class Bundle { } declaration.name = getSafeName( declaration.name ); - }); + } ); // deconflict reified namespaces const namespace = module.namespace(); if ( namespace.needsNamespaceBlock ) { namespace.name = getSafeName( namespace.name ); } - }); + } ); this.scope.deshadow( toDeshadow ); } @@ -299,17 +299,17 @@ export default class Bundle { msg += `: ${err.message}`; throw new Error( msg ); - }) + } ) .then( source => { if ( typeof source === 'string' ) return source; if ( source && typeof source === 'object' && source.code ) return source; // TODO report which plugin failed - error({ + error( { code: 'BAD_LOADER', message: `Error loading ${relativeId( id )}: plugin load hook should return a string, a { code, map } object, or nothing/null` - }); - }) + } ); + } ) .then( source => { if ( typeof source === 'string' ) { source = { @@ -323,11 +323,11 @@ export default class Bundle { } return transform( this, source, id, this.plugins ); - }) + } ) .then( source => { const { code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds } = source; - const module = new Module({ + const module = new Module( { id, code, originalCode, @@ -336,7 +336,7 @@ export default class Bundle { sourceMapChain, resolvedIds, bundle: this - }); + } ); this.modules.push( module ); this.moduleById.set( id, module ); @@ -344,9 +344,9 @@ export default class Bundle { return this.fetchAllDependencies( module ).then( () => { keys( module.exports ).forEach( name => { if ( name !== 'default' ) { - module.exportsAll[name] = module.id; + module.exportsAll[ name ] = module.id; } - }); + } ); module.exportAllSources.forEach( source => { const id = module.resolvedIds[ source ] || module.resolvedExternalIds[ source ]; const exportAllModule = this.moduleById.get( id ); @@ -354,18 +354,18 @@ export default class Bundle { keys( exportAllModule.exportsAll ).forEach( name => { if ( name in module.exportsAll ) { - this.warn({ + this.warn( { code: 'NAMESPACE_CONFLICT', message: `Conflicting namespaces: ${relativeId( module.id )} re-exports '${name}' from both ${relativeId( module.exportsAll[ name ] )} and ${relativeId( exportAllModule.exportsAll[ name ] )} (will be ignored)` - }); + } ); } else { - module.exportsAll[ name ] = exportAllModule.exportsAll[ name ]; + module.exportsAll[ name ] = exportAllModule.exportsAll[ name ]; } - }); - }); + } ); + } ); return module; - }); - }); + } ); + } ); } fetchAllDependencies ( module ) { @@ -374,24 +374,24 @@ export default class Bundle { return ( resolvedId ? Promise.resolve( resolvedId ) : this.resolveId( source, module.id ) ) .then( resolvedId => { const externalId = resolvedId || ( - isRelative( source ) ? resolve( module.id, '..', source ) : source - ); + isRelative( source ) ? resolve( module.id, '..', source ) : source + ); let isExternal = this.isExternal( externalId ); if ( !resolvedId && !isExternal ) { if ( isRelative( source ) ) { - error({ + error( { code: 'UNRESOLVED_IMPORT', message: `Could not resolve '${source}' from ${relativeId( module.id )}` - }); + } ); } - this.warn({ + this.warn( { code: 'UNRESOLVED_IMPORT', message: `'${source}' is imported by ${relativeId( module.id )}, but could not be resolved – treating it as an external dependency`, url: 'https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency' - }); + } ); isExternal = true; } @@ -412,16 +412,16 @@ export default class Bundle { if ( importDeclaration.source !== source ) return; externalModule.traceExport( importDeclaration.name ); - }); + } ); } else { if ( resolvedId === module.id ) { // need to find the actual import declaration, so we can provide // a useful error message. Bit hoop-jumpy but what can you do const declaration = module.ast.body.find( node => { return node.isImportDeclaration && node.source.value === source; - }); + } ); - module.error({ + module.error( { code: 'CANNOT_IMPORT_SELF', message: `A module cannot import itself` }, declaration.start ); @@ -430,8 +430,8 @@ export default class Bundle { module.resolvedIds[ source ] = resolvedId; return this.fetchModule( resolvedId, module.id ); } - }); - }); + } ); + } ); } getPathRelativeToEntryDirname ( resolvedId ) { @@ -448,10 +448,10 @@ export default class Bundle { render ( options = {} ) { return Promise.resolve().then( () => { if ( options.format === 'es6' ) { - this.warn({ + this.warn( { code: 'DEPRECATED_ES6', message: 'The es6 format is deprecated – use `es` instead' - }); + } ); options.format = 'es'; } @@ -459,7 +459,7 @@ export default class Bundle { // Determine export mode - 'default', 'named', 'none' const exportMode = getExportMode( this, options ); - let magicString = new MagicStringBundle({ separator: '\n\n' }); + let magicString = new MagicStringBundle( { separator: '\n\n' } ); const usedModules = []; timeStart( 'render modules' ); @@ -471,13 +471,13 @@ export default class Bundle { magicString.addSource( source ); usedModules.push( module ); } - }); + } ); if ( !magicString.toString().trim() && this.entryModule.getExports().length === 0 ) { - this.warn({ + this.warn( { code: 'EMPTY_BUNDLE', message: 'Generated an empty bundle' - }); + } ); } timeEnd( 'render modules' ); @@ -504,10 +504,10 @@ export default class Bundle { const finalise = finalisers[ options.format ]; if ( !finalise ) { - error({ + error( { code: 'INVALID_OPTION', message: `You must specify an output type - valid options are ${keys( finalisers ).join( ', ' )}` - }); + } ); } timeStart( 'render format' ); @@ -543,13 +543,13 @@ export default class Bundle { if ( file ) file = resolve( typeof process !== 'undefined' ? process.cwd() : '', file ); if ( this.hasLoaders || find( this.plugins, plugin => plugin.transform || plugin.transformBundle ) ) { - map = magicString.generateMap({}); + map = magicString.generateMap( {} ); if ( typeof map.mappings === 'string' ) { map.mappings = decode( map.mappings ); } map = collapseSourcemaps( this, file, map, usedModules, bundleSourcemapChain ); } else { - map = magicString.generateMap({ file, includeContent: true }); + map = magicString.generateMap( { file, includeContent: true } ); } map.sources = map.sources.map( normalize ); @@ -559,8 +559,8 @@ export default class Bundle { if ( code[ code.length - 1 ] !== '\n' ) code += '\n'; return { code, map }; - }); - }); + } ); + } ); } sort () { @@ -574,7 +574,7 @@ export default class Bundle { this.modules.forEach( module => { stronglyDependsOn[ module.id ] = blank(); dependsOn[ module.id ] = blank(); - }); + } ); this.modules.forEach( module => { function processStrongDependency ( dependency ) { @@ -593,7 +593,7 @@ export default class Bundle { module.strongDependencies.forEach( processStrongDependency ); module.dependencies.forEach( processDependency ); - }); + } ); const visit = module => { if ( seen[ module.id ] ) { @@ -612,7 +612,7 @@ export default class Bundle { if ( hasCycles ) { ordered.forEach( ( a, i ) => { for ( i += 1; i < ordered.length; i += 1 ) { - const b = ordered[i]; + const b = ordered[ i ]; // TODO reinstate this! it no longer works if ( stronglyDependsOn[ a.id ][ b.id ] ) { @@ -629,7 +629,7 @@ export default class Bundle { } visited[ module.id ] = true; for ( let i = 0; i < module.dependencies.length; i += 1 ) { - const dependency = module.dependencies[i]; + const dependency = module.dependencies[ i ]; if ( !visited[ dependency.id ] && findParent( dependency ) ) return true; } }; @@ -641,7 +641,7 @@ export default class Bundle { ); } } - }); + } ); } return ordered; diff --git a/src/Module.js b/src/Module.js index bb868548d59..cdd8526d575 100644 --- a/src/Module.js +++ b/src/Module.js @@ -17,14 +17,14 @@ import ModuleScope from './ast/scopes/ModuleScope.js'; function tryParse ( module, acornOptions ) { try { - return parse( module.code, assign({ + return parse( module.code, assign( { ecmaVersion: 8, sourceType: 'module', - onComment: ( block, text, start, end ) => module.comments.push({ block, text, start, end }), + onComment: ( block, text, start, end ) => module.comments.push( { block, text, start, end } ), preserveParens: false - }, acornOptions )); + }, acornOptions ) ); } catch ( err ) { - module.error({ + module.error( { code: 'PARSE_ERROR', message: err.message.replace( / \(\d+:\d+\)$/, '' ) }, err.pos ); @@ -32,7 +32,7 @@ function tryParse ( module, acornOptions ) { } export default class Module { - constructor ({ id, code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds, resolvedExternalIds, bundle }) { + constructor ( { id, code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds, resolvedExternalIds, bundle } ) { this.code = code; this.id = id; this.bundle = bundle; @@ -79,17 +79,17 @@ export default class Module { this.magicString = new MagicString( code, { filename: this.excludeFromSourcemap ? null : id, // don't include plugin helpers in sourcemap indentExclusionRanges: [] - }); + } ); // remove existing sourceMappingURL comments - this.comments = this.comments.filter(comment => { + this.comments = this.comments.filter( comment => { //only one line comment can contain source maps - const isSourceMapComment = !comment.block && SOURCEMAPPING_URL_RE.test(comment.text); - if (isSourceMapComment) { - this.magicString.remove(comment.start, comment.end ); + const isSourceMapComment = !comment.block && SOURCEMAPPING_URL_RE.test( comment.text ); + if ( isSourceMapComment ) { + this.magicString.remove( comment.start, comment.end ); } return !isSourceMapComment; - }); + } ); this.declarations = blank(); this.type = 'Module'; // TODO only necessary so that Scope knows this should be treated as a function scope... messy @@ -122,7 +122,7 @@ export default class Module { const name = specifier.exported.name; if ( this.exports[ name ] || this.reexports[ name ] ) { - this.error({ + this.error( { code: 'DUPLICATE_EXPORT', message: `A module cannot have multiple exports with the same name ('${name}')` }, specifier.start ); @@ -134,7 +134,7 @@ export default class Module { localName: specifier.local.name, module: null // filled in later }; - }); + } ); } } @@ -145,7 +145,7 @@ export default class Module { const identifier = ( node.declaration.id && node.declaration.id.name ) || node.declaration.name; if ( this.exports.default ) { - this.error({ + this.error( { code: 'DUPLICATE_EXPORT', message: `A module can only have one default export` }, node.start ); @@ -171,8 +171,8 @@ export default class Module { declaration.declarations.forEach( decl => { extractNames( decl.id ).forEach( localName => { this.exports[ localName ] = { localName }; - }); - }); + } ); + } ); } else { // export function foo () {} const localName = declaration.id.name; @@ -188,18 +188,18 @@ export default class Module { const exportedName = specifier.exported.name; if ( this.exports[ exportedName ] || this.reexports[ exportedName ] ) { - this.error({ + this.error( { code: 'DUPLICATE_EXPORT', message: `A module cannot have multiple exports with the same name ('${exportedName}')` }, specifier.start ); } this.exports[ exportedName ] = { localName }; - }); + } ); } else { // TODO is this really necessary? `export {}` is valid JS, and // might be used as a hint that this is indeed a module - this.warn({ + this.warn( { code: 'EMPTY_EXPORT', message: `Empty export declaration` }, node.start ); @@ -216,7 +216,7 @@ export default class Module { const localName = specifier.local.name; if ( this.imports[ localName ] ) { - this.error({ + this.error( { code: 'DUPLICATE_IMPORT', message: `Duplicated import '${localName}'` }, specifier.start ); @@ -227,7 +227,7 @@ export default class Module { const name = isDefault ? 'default' : isNamespace ? '*' : specifier.imported.name; this.imports[ localName ] = { source, specifier, name, module: null }; - }); + } ); } analyse () { @@ -262,13 +262,13 @@ export default class Module { const id = this.resolvedIds[ specifier.source ] || this.resolvedExternalIds[ specifier.source ]; specifier.module = this.bundle.moduleById.get( id ); - }); - }); + } ); + } ); this.exportAllModules = this.exportAllSources.map( source => { const id = this.resolvedIds[ source ] || this.resolvedExternalIds[ source ]; return this.bundle.moduleById.get( id ); - }); + } ); this.sources.forEach( source => { const id = this.resolvedIds[ source ]; @@ -277,12 +277,12 @@ export default class Module { const module = this.bundle.moduleById.get( id ); this.dependencies.push( module ); } - }); + } ); } bindReferences () { for ( const node of this.ast.body ) { - node.bind( this.scope ); + node.bind(); } // if ( this.declarations.default ) { @@ -297,7 +297,7 @@ export default class Module { if ( pos !== undefined ) { props.pos = pos; - const { line, column } = locate( this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps + const { line, column } = locate( this.code, pos, { offsetLine: 1 } ); // TODO trace sourcemaps props.loc = { file: this.id, line, column }; props.frame = getCodeFrame( this.code, line, column ); @@ -311,20 +311,16 @@ export default class Module { return null; } - findScope () { - return this.scope; - } - getExports () { const exports = blank(); keys( this.exports ).forEach( name => { exports[ name ] = true; - }); + } ); keys( this.reexports ).forEach( name => { exports[ name ] = true; - }); + } ); this.exportAllModules.forEach( module => { if ( module.isExternal ) { @@ -334,18 +330,18 @@ export default class Module { module.getExports().forEach( name => { if ( name !== 'default' ) exports[ name ] = true; - }); - }); + } ); + } ); return keys( exports ); } namespace () { - if ( !this.declarations['*'] ) { - this.declarations['*'] = new SyntheticNamespaceDeclaration( this ); + if ( !this.declarations[ '*' ] ) { + this.declarations[ '*' ] = new SyntheticNamespaceDeclaration( this ); } - return this.declarations['*']; + return this.declarations[ '*' ]; } render ( es, legacy ) { @@ -364,8 +360,8 @@ export default class Module { run () { for ( const node of this.ast.body ) { - if ( node.hasEffects( this.scope ) ) { - node.run( this.scope ); + if ( node.hasEffects() ) { + node.run(); } } } @@ -401,7 +397,7 @@ export default class Module { const declaration = otherModule.traceExport( importDeclaration.name ); if ( !declaration ) { - this.error({ + this.error( { code: 'MISSING_EXPORT', message: `'${importDeclaration.name}' is not exported by ${relativeId( otherModule.id )}`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` @@ -416,7 +412,7 @@ export default class Module { traceExport ( name ) { // export * from 'external' - if ( name[0] === '*' ) { + if ( name[ 0 ] === '*' ) { const module = this.bundle.moduleById.get( name.slice( 1 ) ); return module.traceExport( '*' ); } @@ -427,7 +423,7 @@ export default class Module { const declaration = reexportDeclaration.module.traceExport( reexportDeclaration.localName ); if ( !declaration ) { - this.error({ + this.error( { code: 'MISSING_EXPORT', message: `'${reexportDeclaration.localName}' is not exported by ${relativeId( reexportDeclaration.module.id )}`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` @@ -448,7 +444,7 @@ export default class Module { if ( name === 'default' ) return; for ( let i = 0; i < this.exportAllModules.length; i += 1 ) { - const module = this.exportAllModules[i]; + const module = this.exportAllModules[ i ]; const declaration = module.traceExport( name ); if ( declaration ) return declaration; @@ -459,7 +455,7 @@ export default class Module { if ( pos !== undefined ) { warning.pos = pos; - const { line, column } = locate( this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps + const { line, column } = locate( this.code, pos, { offsetLine: 1 } ); // TODO trace sourcemaps warning.loc = { file: this.id, line, column }; warning.frame = getCodeFrame( this.code, line, column ); diff --git a/src/ast/Node.js b/src/ast/Node.js index 1a9cd8fd6d2..9ce677d6a39 100644 --- a/src/ast/Node.js +++ b/src/ast/Node.js @@ -2,8 +2,8 @@ import { locate } from 'locate-character'; import { UNKNOWN } from './values.js'; export default class Node { - bind ( scope ) { - this.eachChild( child => child.bind( this.scope || scope ) ); + bind () { + this.eachChild( child => child.bind() ); } eachChild ( callback ) { @@ -28,11 +28,6 @@ export default class Node { return selector.test( this.type ) ? this : this.parent.findParent( selector ); } - // TODO abolish findScope. if a node needs to store scope, store it - findScope ( functionScope ) { - return this.parent.findScope( functionScope ); - } - gatherPossibleValues ( values ) { //this.eachChild( child => child.gatherPossibleValues( values ) ); values.add( UNKNOWN ); @@ -42,28 +37,41 @@ export default class Node { return UNKNOWN; } - hasEffects ( scope ) { - if ( this.scope ) scope = this.scope; - + hasEffects () { for ( const key of this.keys ) { const value = this[ key ]; if ( value ) { if ( 'length' in value ) { for ( const child of value ) { - if ( child && child.hasEffects( scope ) ) { + if ( child && child.hasEffects( this.scope ) ) { return true; } } - } else if ( value && value.hasEffects( scope ) ) { + } else if ( value.hasEffects( this.scope ) ) { return true; } } } } - initialise ( scope ) { - this.eachChild( child => child.initialise( this.scope || scope ) ); + initialise ( parentScope ) { + this.initialiseScope( parentScope ); + this.initialiseNode( parentScope ); + this.initialiseChildren( parentScope ); + } + + // Override if e.g. some children need to be initialised with the parent scope + initialiseChildren () { + this.eachChild( child => child.initialise( this.scope ) ); + } + + // Override to perform special initialisation steps after the scope is initialised + initialiseNode () {} + + // Overwrite to create a new scope + initialiseScope ( parentScope ) { + this.scope = parentScope; } insertSemicolon ( code ) { @@ -74,7 +82,7 @@ export default class Node { locate () { // useful for debugging - const location = locate( this.module.code, this.start, { offsetLine: 1 }); + const location = locate( this.module.code, this.start, { offsetLine: 1 } ); location.file = this.module.id; location.toString = () => JSON.stringify( location ); @@ -85,13 +93,11 @@ export default class Node { this.eachChild( child => child.render( code, es ) ); } - run ( scope ) { + run () { if ( this.ran ) return; this.ran = true; - this.eachChild( child => { - child.run( this.scope || scope ); - }); + this.eachChild( child => child.run() ); } toString () { diff --git a/src/ast/nodes/ArrowFunctionExpression.js b/src/ast/nodes/ArrowFunctionExpression.js index 8d9a3d2e878..1cd0307e2a1 100644 --- a/src/ast/nodes/ArrowFunctionExpression.js +++ b/src/ast/nodes/ArrowFunctionExpression.js @@ -1,38 +1,12 @@ -import Node from '../Node.js'; +import Function from './shared/Function.js'; import Scope from '../scopes/Scope.js'; -import extractNames from '../utils/extractNames.js'; -export default class ArrowFunctionExpression extends Node { - bind ( scope ) { - super.bind( this.scope || scope ); - } - - findScope ( functionScope ) { - return this.scope || this.parent.findScope( functionScope ); - } - - hasEffects () { - return false; - } - - initialise ( scope ) { - if ( this.body.type === 'BlockStatement' ) { - this.body.createScope( scope ); - this.scope = this.body.scope; - } else { - this.scope = new Scope({ - parent: scope, - isBlockScope: false, - isLexicalBoundary: false - }); - - for ( const param of this.params ) { - for ( const name of extractNames( param ) ) { - this.scope.addDeclaration( name, null, null, true ); // TODO ugh - } - } - } - - super.initialise( this.scope ); +export default class ArrowFunctionExpression extends Function { + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: false, + isLexicalBoundary: false + } ); } } diff --git a/src/ast/nodes/AssignmentExpression.js b/src/ast/nodes/AssignmentExpression.js index bbbb7d0ab96..ba846a3cbef 100644 --- a/src/ast/nodes/AssignmentExpression.js +++ b/src/ast/nodes/AssignmentExpression.js @@ -5,14 +5,14 @@ import isProgramLevel from '../utils/isProgramLevel.js'; import { NUMBER, STRING } from '../values.js'; export default class AssignmentExpression extends Node { - bind ( scope ) { + bind () { const subject = this.left; this.subject = subject; - disallowIllegalReassignment( scope, subject ); + disallowIllegalReassignment( this.scope, subject ); if ( subject.type === 'Identifier' ) { - const declaration = scope.findDeclaration( subject.name ); + const declaration = this.scope.findDeclaration( subject.name ); declaration.isReassigned = true; if ( declaration.possibleValues ) { // TODO this feels hacky @@ -26,22 +26,18 @@ export default class AssignmentExpression extends Node { } } - super.bind( scope ); + super.bind(); } - hasEffects ( scope ) { - const hasEffects = this.isUsedByBundle() || this.right.hasEffects( scope ); + hasEffects () { + const hasEffects = this.isUsedByBundle() || this.right.hasEffects( this.scope ); return hasEffects; } - initialise ( scope ) { - this.scope = scope; - + initialiseNode () { if ( isProgramLevel( this ) ) { this.module.bundle.dependentExpressions.push( this ); } - - super.initialise( scope ); } isUsedByBundle () { diff --git a/src/ast/nodes/BinaryExpression.js b/src/ast/nodes/BinaryExpression.js index f4bc5d72aa9..81c0fa5c0f0 100644 --- a/src/ast/nodes/BinaryExpression.js +++ b/src/ast/nodes/BinaryExpression.js @@ -34,7 +34,7 @@ export default class BinaryExpression extends Node { const rightValue = this.right.getValue(); if ( rightValue === UNKNOWN ) return UNKNOWN; - if (!operators[ this.operator ]) return UNKNOWN; + if ( !operators[ this.operator ] ) return UNKNOWN; return operators[ this.operator ]( leftValue, rightValue ); } diff --git a/src/ast/nodes/BlockStatement.js b/src/ast/nodes/BlockStatement.js index d83d321ef65..1c83e4eb9b1 100644 --- a/src/ast/nodes/BlockStatement.js +++ b/src/ast/nodes/BlockStatement.js @@ -1,43 +1,18 @@ import Statement from './shared/Statement.js'; import Scope from '../scopes/Scope.js'; -import extractNames from '../utils/extractNames.js'; export default class BlockStatement extends Statement { bind () { - for ( const node of this.body ) { - node.bind( this.scope ); - } + this.body.forEach( node => node.bind() ); } - createScope ( parent ) { - this.parentIsFunction = /Function/.test( this.parent.type ); - this.isFunctionBlock = this.parentIsFunction || this.parent.type === 'Module'; - - this.scope = new Scope({ - parent, - isBlockScope: !this.isFunctionBlock, - isLexicalBoundary: this.isFunctionBlock && this.parent.type !== 'ArrowFunctionExpression', - owner: this // TODO is this used anywhere? - }); - - const params = this.parent.params || ( this.parent.type === 'CatchClause' && [ this.parent.param ] ); - - if ( params && params.length ) { - params.forEach( node => { - extractNames( node ).forEach( name => { - this.scope.addDeclaration( name, node, false, true ); - }); - }); - } + initialiseAndReplaceScope ( scope ) { + this.scope = scope; + this.initialiseNode(); + this.initialiseChildren( scope ); } - findScope ( functionScope ) { - return functionScope && !this.isFunctionBlock ? this.parent.findScope( functionScope ) : this.scope; - } - - initialise ( scope ) { - if ( !this.scope ) this.createScope( scope ); // scope can be created early in some cases, e.g for (let i... ) - + initialiseChildren () { let lastNode; for ( const node of this.body ) { node.initialise( this.scope ); @@ -47,13 +22,21 @@ export default class BlockStatement extends Statement { } } + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: true, + isLexicalBoundary: false + } ); + } + render ( code, es ) { - if (this.body.length) { + if ( this.body.length ) { for ( const node of this.body ) { node.render( code, es ); } } else { - Statement.prototype.render.call(this, code, es); + Statement.prototype.render.call( this, code, es ); } } } diff --git a/src/ast/nodes/CallExpression.js b/src/ast/nodes/CallExpression.js index d743838d29d..feb736a00a5 100644 --- a/src/ast/nodes/CallExpression.js +++ b/src/ast/nodes/CallExpression.js @@ -3,19 +3,19 @@ import isProgramLevel from '../utils/isProgramLevel.js'; import callHasEffects from './shared/callHasEffects.js'; export default class CallExpression extends Node { - bind ( scope ) { + bind () { if ( this.callee.type === 'Identifier' ) { - const declaration = scope.findDeclaration( this.callee.name ); + const declaration = this.scope.findDeclaration( this.callee.name ); if ( declaration.isNamespace ) { - this.module.error({ + this.module.error( { code: 'CANNOT_CALL_NAMESPACE', message: `Cannot call a namespace ('${this.callee.name}')` }, this.start ); } if ( this.callee.name === 'eval' && declaration.isGlobal ) { - this.module.warn({ + this.module.warn( { code: 'EVAL', message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, url: 'https://github.com/rollup/rollup/wiki/Troubleshooting#avoiding-eval' @@ -23,21 +23,20 @@ export default class CallExpression extends Node { } } - super.bind( scope ); + super.bind(); } - hasEffects ( scope ) { - return callHasEffects( scope, this.callee, false ); + hasEffects () { + return callHasEffects( this.scope, this.callee, false ); } - initialise ( scope ) { + initialiseNode () { if ( isProgramLevel( this ) ) { this.module.bundle.dependentExpressions.push( this ); } - super.initialise( scope ); } isUsedByBundle () { - return this.hasEffects( this.findScope() ); + return this.hasEffects(); } } diff --git a/src/ast/nodes/CatchClause.js b/src/ast/nodes/CatchClause.js index e4697ecf635..037e736a920 100644 --- a/src/ast/nodes/CatchClause.js +++ b/src/ast/nodes/CatchClause.js @@ -1,10 +1,21 @@ import Node from '../Node.js'; +import Scope from '../scopes/Scope.js'; +import extractNames from '../utils/extractNames.js'; export default class CatchClause extends Node { - initialise ( scope ) { - this.body.createScope( scope ); - this.scope = this.body.scope; + initialiseChildren () { + if ( this.param ) { + this.param.initialise( this.scope ); + extractNames( this.param ).forEach( name => this.scope.addDeclaration( name, null, false, true ) ); + } + this.body.initialiseAndReplaceScope( this.scope ); + } - this.body.initialise( this.scope ); + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: true, + isLexicalBoundary: false + } ); } -} \ No newline at end of file +} diff --git a/src/ast/nodes/ClassDeclaration.js b/src/ast/nodes/ClassDeclaration.js index 594ee62b17f..2d1b98ffae4 100644 --- a/src/ast/nodes/ClassDeclaration.js +++ b/src/ast/nodes/ClassDeclaration.js @@ -1,40 +1,21 @@ -import Node from '../Node.js'; - -// TODO is this basically identical to FunctionDeclaration? -export default class ClassDeclaration extends Node { - activate () { - if ( this.activated ) return; - this.activated = true; - - if ( this.superClass ) this.superClass.run( this.scope ); - this.body.run(); - } - - addReference () { - /* noop? */ - } +import Class from './shared/Class.js'; +export default class ClassDeclaration extends Class { gatherPossibleValues ( values ) { values.add( this ); } - getName () { - return this.name; - } - hasEffects () { return false; } - initialise ( scope ) { - this.scope = scope; - + initialiseChildren ( parentScope ) { if ( this.id ) { this.name = this.id.name; - scope.addDeclaration( this.name, this, false, false ); + parentScope.addDeclaration( this.name, this, false, false ); + this.id.initialise( parentScope ); } - - super.initialise( scope ); + super.initialiseChildren(parentScope); } render ( code, es ) { @@ -45,9 +26,9 @@ export default class ClassDeclaration extends Node { } } - run ( scope ) { + run () { if ( this.parent.type === 'ExportDefaultDeclaration' ) { - super.run( scope ); + super.run(); } } } diff --git a/src/ast/nodes/ClassExpression.js b/src/ast/nodes/ClassExpression.js index 63f1399663b..e3bd24bdc25 100644 --- a/src/ast/nodes/ClassExpression.js +++ b/src/ast/nodes/ClassExpression.js @@ -1,26 +1,12 @@ -import Node from '../Node.js'; -import Scope from '../scopes/Scope.js'; - -export default class ClassExpression extends Node { - bind () { - super.bind( this.scope ); - } - - findScope () { - return this.scope; - } - - initialise () { - this.scope = new Scope({ - isBlockScope: true, - parent: this.parent.findScope( false ) - }); +import Class from './shared/Class.js'; +export default class ClassExpression extends Class { + initialiseChildren (parentScope) { if ( this.id ) { - // function expression IDs belong to the child scope... - this.scope.addDeclaration( this.id.name, this, false, true ); + this.name = this.id.name; + this.scope.addDeclaration( this.name, this, false, false ); + this.id.initialise( this.scope ); } - - super.initialise( this.scope ); + super.initialiseChildren(parentScope); } } diff --git a/src/ast/nodes/ConditionalExpression.js b/src/ast/nodes/ConditionalExpression.js index b8bb8c9864b..3350736974b 100644 --- a/src/ast/nodes/ConditionalExpression.js +++ b/src/ast/nodes/ConditionalExpression.js @@ -2,25 +2,21 @@ import Node from '../Node.js'; import { UNKNOWN } from '../values.js'; export default class ConditionalExpression extends Node { - initialise ( scope ) { + initialiseChildren ( parentScope ) { if ( this.module.bundle.treeshake ) { this.testValue = this.test.getValue(); - if ( this.testValue === UNKNOWN ) { - super.initialise( scope ); - } - - else if ( this.testValue ) { - this.consequent.initialise( scope ); + if ( this.testValue === UNKNOWN ) { + super.initialiseChildren( parentScope ); + } else if ( this.testValue ) { + this.consequent.initialise( this.scope ); this.alternate = null; } else if ( this.alternate ) { - this.alternate.initialise( scope ); + this.alternate.initialise( this.scope ); this.consequent = null; } - } - - else { - super.initialise( scope ); + } else { + super.initialiseChildren( parentScope ); } } diff --git a/src/ast/nodes/ExportAllDeclaration.js b/src/ast/nodes/ExportAllDeclaration.js index a0f53085dc7..26f3101d4fb 100644 --- a/src/ast/nodes/ExportAllDeclaration.js +++ b/src/ast/nodes/ExportAllDeclaration.js @@ -1,7 +1,7 @@ import Node from '../Node.js'; export default class ExportAllDeclaration extends Node { - initialise () { + initialiseNode () { this.isExportDeclaration = true; } diff --git a/src/ast/nodes/ExportDefaultDeclaration.js b/src/ast/nodes/ExportDefaultDeclaration.js index 945f774bcfa..efe17298911 100644 --- a/src/ast/nodes/ExportDefaultDeclaration.js +++ b/src/ast/nodes/ExportDefaultDeclaration.js @@ -3,16 +3,6 @@ import Node from '../Node.js'; const functionOrClassDeclaration = /^(?:Function|Class)Declaration/; export default class ExportDefaultDeclaration extends Node { - initialise ( scope ) { - this.isExportDeclaration = true; - this.isDefault = true; - - this.name = ( this.declaration.id && this.declaration.id.name ) || this.declaration.name || this.module.basename(); - scope.declarations.default = this; - - this.declaration.initialise( scope ); - } - activate () { if ( this.activated ) return; this.activated = true; @@ -25,11 +15,11 @@ export default class ExportDefaultDeclaration extends Node { if ( this.original ) this.original.addReference( reference ); } - bind ( scope ) { + bind () { const name = ( this.declaration.id && this.declaration.id.name ) || this.declaration.name; - if ( name ) this.original = scope.findDeclaration( name ); + if ( name ) this.original = this.scope.findDeclaration( name ); - this.declaration.bind( scope ); + this.declaration.bind(); } gatherPossibleValues ( values ) { @@ -44,6 +34,14 @@ export default class ExportDefaultDeclaration extends Node { return this.name; } + initialiseNode () { + this.isExportDeclaration = true; + this.isDefault = true; + + this.name = ( this.declaration.id && this.declaration.id.name ) || this.declaration.name || this.module.basename(); + this.scope.declarations.default = this; + } + // TODO this is total chaos, tidy it up render ( code, es ) { const treeshake = this.module.bundle.treeshake; @@ -53,7 +51,7 @@ export default class ExportDefaultDeclaration extends Node { let declaration_start; if ( this.declaration ) { const statementStr = code.original.slice( this.start, this.end ); - declaration_start = this.start + statementStr.match(/^\s*export\s+default\s*/)[0].length; + declaration_start = this.start + statementStr.match( /^\s*export\s+default\s*/ )[ 0 ].length; } if ( this.shouldInclude || this.declaration.activated ) { @@ -63,7 +61,7 @@ export default class ExportDefaultDeclaration extends Node { code.remove( this.start, declaration_start ); } else { code.overwrite( this.start, declaration_start, `var ${this.name} = ` ); - if ( code.original[this.end - 1] !== ';' ) code.appendLeft( this.end, ';' ); + if ( code.original[ this.end - 1 ] !== ';' ) code.appendLeft( this.end, ';' ); } } @@ -92,7 +90,7 @@ export default class ExportDefaultDeclaration extends Node { const hasEffects = this.declaration.hasEffects( this.module.scope ); code.remove( this.start, hasEffects ? declaration_start : this.next || this.end ); } - } else if (name === this.declaration.name) { + } else if ( name === this.declaration.name ) { code.remove( this.start, this.next || this.end ); } else { code.overwrite( this.start, declaration_start, `${this.module.bundle.varOrConst} ${name} = ` ); @@ -101,9 +99,9 @@ export default class ExportDefaultDeclaration extends Node { } } - run ( scope ) { + run () { this.shouldInclude = true; - super.run( scope ); + super.run(); // special case (TODO is this correct?) if ( functionOrClassDeclaration.test( this.declaration.type ) && !this.declaration.id ) { diff --git a/src/ast/nodes/ExportNamedDeclaration.js b/src/ast/nodes/ExportNamedDeclaration.js index c9e4af11bc7..74d0a104056 100644 --- a/src/ast/nodes/ExportNamedDeclaration.js +++ b/src/ast/nodes/ExportNamedDeclaration.js @@ -1,15 +1,12 @@ import Node from '../Node.js'; export default class ExportNamedDeclaration extends Node { - initialise ( scope ) { - this.scope = scope; - this.isExportDeclaration = true; - - if ( this.declaration ) this.declaration.initialise( scope ); + bind () { + if ( this.declaration ) this.declaration.bind(); } - bind ( scope ) { - if ( this.declaration ) this.declaration.bind( scope ); + initialiseNode () { + this.isExportDeclaration = true; } render ( code, es ) { diff --git a/src/ast/nodes/ForInStatement.js b/src/ast/nodes/ForInStatement.js index 1872b963b21..3f0954b8f5c 100644 --- a/src/ast/nodes/ForInStatement.js +++ b/src/ast/nodes/ForInStatement.js @@ -4,19 +4,20 @@ import Scope from '../scopes/Scope.js'; import { STRING } from '../values.js'; export default class ForInStatement extends Statement { - initialise ( scope ) { - if ( this.body.type === 'BlockStatement' ) { - this.body.createScope( scope ); - this.scope = this.body.scope; - } else { - this.scope = new Scope({ - parent: scope, - isBlockScope: true, - isLexicalBoundary: false - }); - } - - super.initialise( this.scope ); + initialiseChildren () { + this.left.initialise( this.scope ); + this.right.initialise( this.scope ); + this.body.initialiseAndReplaceScope ? + this.body.initialiseAndReplaceScope( this.scope ) : + this.body.initialise( this.scope ); assignTo( this.left, this.scope, STRING ); } + + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: true, + isLexicalBoundary: false + } ); + } } diff --git a/src/ast/nodes/ForOfStatement.js b/src/ast/nodes/ForOfStatement.js index a5225f847b7..8c745b7103a 100644 --- a/src/ast/nodes/ForOfStatement.js +++ b/src/ast/nodes/ForOfStatement.js @@ -4,19 +4,20 @@ import Scope from '../scopes/Scope.js'; import { UNKNOWN } from '../values.js'; export default class ForOfStatement extends Statement { - initialise ( scope ) { - if ( this.body.type === 'BlockStatement' ) { - this.body.createScope( scope ); - this.scope = this.body.scope; - } else { - this.scope = new Scope({ - parent: scope, - isBlockScope: true, - isLexicalBoundary: false - }); - } - - super.initialise( this.scope ); + initialiseChildren () { + this.left.initialise( this.scope ); + this.right.initialise( this.scope ); + this.body.initialiseAndReplaceScope ? + this.body.initialiseAndReplaceScope( this.scope ) : + this.body.initialise( this.scope ); assignTo( this.left, this.scope, UNKNOWN ); } + + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: true, + isLexicalBoundary: false + } ); + } } diff --git a/src/ast/nodes/ForStatement.js b/src/ast/nodes/ForStatement.js index 11e98e5c18c..94132dfa65d 100644 --- a/src/ast/nodes/ForStatement.js +++ b/src/ast/nodes/ForStatement.js @@ -2,22 +2,20 @@ import Statement from './shared/Statement.js'; import Scope from '../scopes/Scope.js'; export default class ForStatement extends Statement { - initialise ( scope ) { - if ( this.body.type === 'BlockStatement' ) { - this.body.createScope( scope ); - this.scope = this.body.scope; - } else { - this.scope = new Scope({ - parent: scope, - isBlockScope: true, - isLexicalBoundary: false - }); - } - - // can't use super, because we need to control the order + initialiseChildren () { if ( this.init ) this.init.initialise( this.scope ); if ( this.test ) this.test.initialise( this.scope ); if ( this.update ) this.update.initialise( this.scope ); - this.body.initialise( this.scope ); + this.body.initialiseAndReplaceScope ? + this.body.initialiseAndReplaceScope( this.scope ) : + this.body.initialise( this.scope ); + } + + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: true, + isLexicalBoundary: false + } ); } } diff --git a/src/ast/nodes/FunctionDeclaration.js b/src/ast/nodes/FunctionDeclaration.js index 660be951a11..2ce79eda407 100644 --- a/src/ast/nodes/FunctionDeclaration.js +++ b/src/ast/nodes/FunctionDeclaration.js @@ -1,24 +1,15 @@ -import Node from '../Node.js'; +import Function from './shared/Function.js'; -export default class FunctionDeclaration extends Node { +export default class FunctionDeclaration extends Function { activate () { if ( this.activated ) return; this.activated = true; - const scope = this.body.scope; - this.params.forEach( param => param.run( scope ) ); // in case of assignment patterns + this.params.forEach( param => param.run() ); // in case of assignment patterns this.body.run(); } - addReference () { - /* noop? */ - } - - bind ( scope ) { - if ( this.id ) this.id.bind( scope ); - this.params.forEach( param => param.bind( this.body.scope ) ); - this.body.bind( scope ); - } + addReference () {} gatherPossibleValues ( values ) { values.add( this ); @@ -28,21 +19,13 @@ export default class FunctionDeclaration extends Node { return this.name; } - hasEffects () { - return false; - } - - initialise ( scope ) { + initialiseChildren ( parentScope ) { if ( this.id ) { this.name = this.id.name; // may be overridden by bundle.deconflict - scope.addDeclaration( this.name, this, false, false ); - this.id.initialise( scope ); + parentScope.addDeclaration( this.name, this, false, false ); + this.id.initialise( parentScope ); } - - this.body.createScope( scope ); - - this.params.forEach( param => param.initialise( this.body.scope ) ); - this.body.initialise(); + super.initialiseChildren( parentScope ); } render ( code, es ) { @@ -53,9 +36,9 @@ export default class FunctionDeclaration extends Node { } } - run ( scope ) { + run () { if ( this.parent.type === 'ExportDefaultDeclaration' ) { - super.run( scope ); + super.run(); } } } diff --git a/src/ast/nodes/FunctionExpression.js b/src/ast/nodes/FunctionExpression.js index 60f89f3f69a..ea0f32ab12d 100644 --- a/src/ast/nodes/FunctionExpression.js +++ b/src/ast/nodes/FunctionExpression.js @@ -1,43 +1,26 @@ -import Node from '../Node.js'; +import Function from './shared/Function.js'; -export default class FunctionExpression extends Node { +export default class FunctionExpression extends Function { activate () { if ( this.activated ) return; this.activated = true; - const scope = this.body.scope; - this.params.forEach( param => param.run( scope ) ); // in case of assignment patterns + this.params.forEach( param => param.run() ); // in case of assignment patterns this.body.run(); } - addReference () { - /* noop? */ - } - - bind () { - if ( this.id ) this.id.bind( this.body.scope ); - this.params.forEach( param => param.bind( this.body.scope ) ); - this.body.bind(); - } + addReference () {} getName () { return this.name; } - hasEffects () { - return false; - } - - initialise ( scope ) { - this.name = this.id && this.id.name; // may be overridden by bundle.deconflict - this.body.createScope( scope ); - + initialiseChildren ( parentScope ) { if ( this.id ) { - this.id.initialise( this.body.scope ); - this.body.scope.addDeclaration( this.id.name, this, false, false ); + this.name = this.id.name; // may be overridden by bundle.deconflict + this.scope.addDeclaration( this.name, this, false, false ); + this.id.initialise( this.scope ); } - - this.params.forEach( param => param.initialise( this.body.scope ) ); - this.body.initialise(); + super.initialiseChildren( parentScope ); } } diff --git a/src/ast/nodes/Identifier.js b/src/ast/nodes/Identifier.js index d458f5d5615..b3857b58e37 100644 --- a/src/ast/nodes/Identifier.js +++ b/src/ast/nodes/Identifier.js @@ -1,7 +1,7 @@ -import Node from '../Node.js'; -import isReference from 'is-reference'; +import Node from "../Node.js"; +import isReference from "is-reference"; -function isAssignmentPatternLhs ( node, parent ) { +function isAssignmentPatternLhs (node, parent) { // special case: `({ foo = 42 }) => {...}` // `foo` actually has two different parents, the Property of the // ObjectPattern, and the AssignmentPattern. In one case it's a @@ -9,42 +9,42 @@ function isAssignmentPatternLhs ( node, parent ) { // `({ foo: foo = 42 }) => {...}`. But unlike a regular shorthand // property, the `foo` node appears at different levels of the tree return ( - parent.type === 'Property' && + parent.type === "Property" && parent.shorthand && - parent.value.type === 'AssignmentPattern' && + parent.value.type === "AssignmentPattern" && parent.value.left === node ); } export default class Identifier extends Node { - bind ( scope ) { - if ( isReference( this, this.parent ) || isAssignmentPatternLhs( this, this.parent ) ) { - this.declaration = scope.findDeclaration( this.name ); - this.declaration.addReference( this ); // TODO necessary? + bind () { + if (isReference(this, this.parent) || isAssignmentPatternLhs(this, this.parent)) { + this.declaration = this.scope.findDeclaration(this.name); + this.declaration.addReference(this); // TODO necessary? } } - gatherPossibleValues ( values ) { - if ( isReference( this, this.parent ) ) { - values.add( this ); + gatherPossibleValues (values) { + if (isReference(this, this.parent)) { + values.add(this); } } - render ( code, es ) { - if ( this.declaration ) { - const name = this.declaration.getName( es ); - if ( name !== this.name ) { - code.overwrite( this.start, this.end, name, { storeName: true, contentOnly: false } ); + render (code, es) { + if (this.declaration) { + const name = this.declaration.getName(es); + if (name !== this.name) { + code.overwrite(this.start, this.end, name, { storeName: true, contentOnly: false }); // special case - if ( this.parent.type === 'Property' && this.parent.shorthand ) { - code.appendLeft( this.start, `${this.name}: ` ); + if (this.parent.type === "Property" && this.parent.shorthand) { + code.appendLeft(this.start, `${this.name}: `); } } } } run () { - if ( this.declaration ) this.declaration.activate(); + if (this.declaration) this.declaration.activate(); } } diff --git a/src/ast/nodes/IfStatement.js b/src/ast/nodes/IfStatement.js index 5bf8c0f6c63..3feedf97ede 100644 --- a/src/ast/nodes/IfStatement.js +++ b/src/ast/nodes/IfStatement.js @@ -3,14 +3,14 @@ import extractNames from '../utils/extractNames.js'; import { UNKNOWN } from '../values.js'; // Statement types which may contain if-statements as direct children. -const statementsWithIfStatements = new Set([ +const statementsWithIfStatements = new Set( [ 'DoWhileStatement', 'ForInStatement', 'ForOfStatement', 'ForStatement', 'IfStatement', 'WhileStatement' -]); +] ); function handleVarDeclarations ( node, scope ) { const hoistedVars = []; @@ -23,8 +23,8 @@ function handleVarDeclarations ( node, scope ) { extractNames( declarator.id ).forEach( name => { if ( !~hoistedVars.indexOf( name ) ) hoistedVars.push( name ); - }); - }); + } ); + } ); } else if ( !/Function/.test( node.type ) ) { @@ -39,32 +39,27 @@ function handleVarDeclarations ( node, scope ) { // TODO DRY this out export default class IfStatement extends Statement { - initialise ( scope ) { - this.scope = scope; - this.testValue = this.test.getValue(); - + initialiseChildren ( parentScope ) { if ( this.module.bundle.treeshake ) { - if ( this.testValue === UNKNOWN ) { - super.initialise( scope ); - } - - else if ( this.testValue ) { - this.consequent.initialise( scope ); - - if ( this.alternate ) this.hoistedVars = handleVarDeclarations( this.alternate, scope ); - this.alternate = null; - } - - else { - if ( this.alternate ) this.alternate.initialise( scope ); + this.testValue = this.test.getValue(); - this.hoistedVars = handleVarDeclarations( this.consequent, scope ); + if ( this.testValue === UNKNOWN ) { + super.initialiseChildren( parentScope ); + } else if ( this.testValue ) { + this.consequent.initialise( this.scope ); + if ( this.alternate ) { + this.hoistedVars = handleVarDeclarations( this.alternate, this.scope ); + this.alternate = null; + } + } else { + if ( this.alternate ) { + this.alternate.initialise( this.scope ); + } + this.hoistedVars = handleVarDeclarations( this.consequent, this.scope ); this.consequent = null; } - } - - else { - super.initialise( scope ); + } else { + super.initialiseChildren( parentScope ); } } @@ -85,7 +80,7 @@ export default class IfStatement extends Statement { .map( name => { const declaration = this.scope.findDeclaration( name ); return declaration.activated ? declaration.getName() : null; - }) + } ) .filter( Boolean ); if ( names.length > 0 ) { diff --git a/src/ast/nodes/ImportDeclaration.js b/src/ast/nodes/ImportDeclaration.js index 0edc68b2111..261aeed2d12 100644 --- a/src/ast/nodes/ImportDeclaration.js +++ b/src/ast/nodes/ImportDeclaration.js @@ -6,7 +6,7 @@ export default class ImportDeclaration extends Node { // TODO do the inter-module binding setup here? } - initialise () { + initialiseNode () { this.isImportDeclaration = true; } diff --git a/src/ast/nodes/Literal.js b/src/ast/nodes/Literal.js index 3b54f14294f..3ba7962a62f 100644 --- a/src/ast/nodes/Literal.js +++ b/src/ast/nodes/Literal.js @@ -11,7 +11,7 @@ export default class Literal extends Node { render ( code ) { if ( typeof this.value === 'string' ) { - code.indentExclusionRanges.push([ this.start + 1, this.end - 1 ]); + code.indentExclusionRanges.push( [ this.start + 1, this.end - 1 ] ); } } } diff --git a/src/ast/nodes/MemberExpression.js b/src/ast/nodes/MemberExpression.js index 5ab2954444d..3716cdc11f3 100644 --- a/src/ast/nodes/MemberExpression.js +++ b/src/ast/nodes/MemberExpression.js @@ -11,7 +11,7 @@ class Keypath { while ( node.type === 'MemberExpression' ) { const prop = node.property; - if ( node.computed ) { + if ( node.computed ) { if ( prop.type !== 'Literal' || typeof prop.value !== 'string' || !validProp.test( prop.value ) ) { this.computed = true; return; @@ -27,23 +27,23 @@ class Keypath { } export default class MemberExpression extends Node { - bind ( scope ) { + bind () { // if this resolves to a namespaced declaration, prepare // to replace it // TODO this code is a bit inefficient const keypath = new Keypath( this ); if ( !keypath.computed && keypath.root.type === 'Identifier' ) { - let declaration = scope.findDeclaration( keypath.root.name ); + let declaration = this.scope.findDeclaration( keypath.root.name ); while ( declaration.isNamespace && keypath.parts.length ) { const exporterId = declaration.module.id; - const part = keypath.parts[0]; + const part = keypath.parts[ 0 ]; declaration = declaration.module.traceExport( part.name || part.value ); if ( !declaration ) { - this.module.warn({ + this.module.warn( { code: 'MISSING_EXPORT', message: `'${part.name || part.value}' is not exported by '${relativeId( exporterId )}'`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` @@ -56,7 +56,7 @@ export default class MemberExpression extends Node { } if ( keypath.parts.length ) { - super.bind( scope ); + super.bind(); return; // not a namespaced declaration } @@ -68,7 +68,7 @@ export default class MemberExpression extends Node { } else { - super.bind( scope ); + super.bind(); } } @@ -89,8 +89,8 @@ export default class MemberExpression extends Node { super.render( code, es ); } - run ( scope ) { + run () { if ( this.declaration ) this.declaration.activate(); - super.run( scope ); + super.run(); } } diff --git a/src/ast/nodes/NewExpression.js b/src/ast/nodes/NewExpression.js index d8fc3b159f4..699a393dddf 100644 --- a/src/ast/nodes/NewExpression.js +++ b/src/ast/nodes/NewExpression.js @@ -2,7 +2,7 @@ import Node from '../Node.js'; import callHasEffects from './shared/callHasEffects.js'; export default class NewExpression extends Node { - hasEffects ( scope ) { - return callHasEffects( scope, this.callee, true ); + hasEffects () { + return callHasEffects( this.scope, this.callee, true ); } } diff --git a/src/ast/nodes/ReturnStatement.js b/src/ast/nodes/ReturnStatement.js deleted file mode 100644 index bff5ae126d6..00000000000 --- a/src/ast/nodes/ReturnStatement.js +++ /dev/null @@ -1,7 +0,0 @@ -import Node from '../Node.js'; - -export default class ReturnStatement extends Node { - // hasEffects () { - // return true; - // } -} diff --git a/src/ast/nodes/TemplateLiteral.js b/src/ast/nodes/TemplateLiteral.js index b25ef48c86a..0b24cdecc65 100644 --- a/src/ast/nodes/TemplateLiteral.js +++ b/src/ast/nodes/TemplateLiteral.js @@ -2,7 +2,7 @@ import Node from '../Node.js'; export default class TemplateLiteral extends Node { render ( code, es ) { - code.indentExclusionRanges.push([ this.start, this.end ]); + code.indentExclusionRanges.push( [ this.start, this.end ] ); super.render( code, es ); } } diff --git a/src/ast/nodes/ThisExpression.js b/src/ast/nodes/ThisExpression.js index 235a372008b..ece9a32f853 100644 --- a/src/ast/nodes/ThisExpression.js +++ b/src/ast/nodes/ThisExpression.js @@ -1,13 +1,13 @@ import Node from '../Node.js'; export default class ThisExpression extends Node { - initialise ( scope ) { - const lexicalBoundary = scope.findLexicalBoundary(); + initialiseNode () { + const lexicalBoundary = this.scope.findLexicalBoundary(); if ( lexicalBoundary.isModuleScope ) { this.alias = this.module.context; if ( this.alias === 'undefined' ) { - this.module.warn({ + this.module.warn( { code: 'THIS_IS_UNDEFINED', message: `The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#this-is-undefined` diff --git a/src/ast/nodes/UnaryExpression.js b/src/ast/nodes/UnaryExpression.js index 9be1c2c3689..9193157e796 100644 --- a/src/ast/nodes/UnaryExpression.js +++ b/src/ast/nodes/UnaryExpression.js @@ -2,18 +2,18 @@ import Node from '../Node.js'; import { UNKNOWN } from '../values.js'; const operators = { - "-": value => -value, - "+": value => +value, - "!": value => !value, - "~": value => ~value, + '-': value => -value, + '+': value => +value, + '!': value => !value, + '~': value => ~value, typeof: value => typeof value, void: () => undefined, delete: () => UNKNOWN }; export default class UnaryExpression extends Node { - bind ( scope ) { - if ( this.value === UNKNOWN ) super.bind( scope ); + bind () { + if ( this.value === UNKNOWN ) super.bind(); } getValue () { @@ -23,12 +23,11 @@ export default class UnaryExpression extends Node { return operators[ this.operator ]( argumentValue ); } - hasEffects ( scope ) { - return this.operator === 'delete' || this.argument.hasEffects( scope ); + hasEffects () { + return this.operator === 'delete' || this.argument.hasEffects(); } - initialise ( scope ) { + initialiseNode () { this.value = this.getValue(); - if ( this.value === UNKNOWN ) super.initialise( scope ); } } diff --git a/src/ast/nodes/UpdateExpression.js b/src/ast/nodes/UpdateExpression.js index 9fcea9dc50f..2b77db9a417 100644 --- a/src/ast/nodes/UpdateExpression.js +++ b/src/ast/nodes/UpdateExpression.js @@ -4,14 +4,14 @@ import isUsedByBundle from './shared/isUsedByBundle.js'; import { NUMBER } from '../values.js'; export default class UpdateExpression extends Node { - bind ( scope ) { + bind () { const subject = this.argument; this.subject = subject; - disallowIllegalReassignment( scope, this.argument ); + disallowIllegalReassignment( this.scope, this.argument ); if ( subject.type === 'Identifier' ) { - const declaration = scope.findDeclaration( subject.name ); + const declaration = this.scope.findDeclaration( subject.name ); declaration.isReassigned = true; if ( declaration.possibleValues ) { @@ -19,18 +19,15 @@ export default class UpdateExpression extends Node { } } - super.bind( scope ); + super.bind(); } - hasEffects ( scope ) { - return isUsedByBundle( scope, this.subject ); + hasEffects () { + return isUsedByBundle( this.scope, this.subject ); } - initialise ( scope ) { - this.scope = scope; - + initialiseNode () { this.module.bundle.dependentExpressions.push( this ); - super.initialise( scope ); } isUsedByBundle () { diff --git a/src/ast/nodes/VariableDeclaration.js b/src/ast/nodes/VariableDeclaration.js index 77f4433d67d..3726705a43b 100644 --- a/src/ast/nodes/VariableDeclaration.js +++ b/src/ast/nodes/VariableDeclaration.js @@ -1,15 +1,15 @@ -import Node from '../Node.js'; -import extractNames from '../utils/extractNames.js'; +import Node from "../Node.js"; +import extractNames from "../utils/extractNames.js"; -function getSeparator ( code, start ) { +function getSeparator (code, start) { let c = start; - while ( c > 0 && code[ c - 1 ] !== '\n' ) { + while (c > 0 && code[c - 1] !== "\n") { c -= 1; - if ( code[c] === ';' || code[c] === '{' ) return '; '; + if (code[c] === ";" || code[c] === "{") return "; "; } - const lineStart = code.slice( c, start ).match( /^\s*/ )[0]; + const lineStart = code.slice(c, start).match(/^\s*/)[0]; return `;\n${lineStart}`; } @@ -17,90 +17,83 @@ function getSeparator ( code, start ) { const forStatement = /^For(?:Of|In)?Statement/; export default class VariableDeclaration extends Node { - initialise ( scope ) { - this.scope = scope; - super.initialise( scope ); - } - - render ( code, es ) { + render (code, es) { const treeshake = this.module.bundle.treeshake; let shouldSeparate = false; let separator; - if ( this.scope.isModuleScope && !forStatement.test( this.parent.type ) ) { + if (this.scope.isModuleScope && !forStatement.test(this.parent.type)) { shouldSeparate = true; - separator = getSeparator( this.module.code, this.start ); + separator = getSeparator(this.module.code, this.start); } let c = this.start; let empty = true; - for ( let i = 0; i < this.declarations.length; i += 1 ) { + for (let i = 0; i < this.declarations.length; i += 1) { const declarator = this.declarations[i]; - const prefix = empty ? '' : separator; // TODO indentation + const prefix = empty ? "" : separator; // TODO indentation - if ( declarator.id.type === 'Identifier' ) { - const proxy = declarator.proxies.get( declarator.id.name ); + if (declarator.id.type === "Identifier") { + const proxy = declarator.proxies.get(declarator.id.name); const isExportedAndReassigned = !es && proxy.exportName && proxy.isReassigned; - if ( isExportedAndReassigned ) { - if ( declarator.init ) { - if ( shouldSeparate ) code.overwrite( c, declarator.start, prefix ); + if (isExportedAndReassigned) { + if (declarator.init) { + if (shouldSeparate) code.overwrite(c, declarator.start, prefix); c = declarator.end; empty = false; } - } else if ( !treeshake || proxy.activated ) { - if ( shouldSeparate ) code.overwrite( c, declarator.start, `${prefix}${this.kind} ` ); // TODO indentation + } else if (!treeshake || proxy.activated) { + if (shouldSeparate) code.overwrite(c, declarator.start, `${prefix}${this.kind} `); // TODO indentation c = declarator.end; empty = false; } - } - - else { + } else { const exportAssignments = []; let activated = false; - extractNames( declarator.id ).forEach( name => { - const proxy = declarator.proxies.get( name ); + extractNames(declarator.id).forEach(name => { + const proxy = declarator.proxies.get(name); const isExportedAndReassigned = !es && proxy.exportName && proxy.isReassigned; - if ( isExportedAndReassigned ) { + if (isExportedAndReassigned) { // code.overwrite( c, declarator.start, prefix ); // c = declarator.end; // empty = false; - exportAssignments.push( 'TODO' ); - } else if ( declarator.activated ) { + exportAssignments.push("TODO"); + } else if (declarator.activated) { activated = true; } }); - if ( !treeshake || activated ) { - if ( shouldSeparate ) code.overwrite( c, declarator.start, `${prefix}${this.kind} ` ); // TODO indentation + if (!treeshake || activated) { + if (shouldSeparate) code.overwrite(c, declarator.start, `${prefix}${this.kind} `); // TODO indentation c = declarator.end; empty = false; } - if ( exportAssignments.length ) { - throw new Error( 'TODO' ); + if (exportAssignments.length) { + throw new Error("TODO"); } } - declarator.render( code, es ); + declarator.render(code, es); } - if ( treeshake && empty ) { - code.remove( this.leadingCommentStart || this.start, this.next || this.end ); + if (treeshake && empty) { + code.remove(this.leadingCommentStart || this.start, this.next || this.end); } else { // always include a semi-colon (https://github.com/rollup/rollup/pull/1013), // unless it's a var declaration in a loop head - const needsSemicolon = !forStatement.test( this.parent.type ); + const needsSemicolon = !forStatement.test(this.parent.type); - if ( this.end > c ) { - code.overwrite( c, this.end, needsSemicolon ? ';' : '' ); - } else if ( needsSemicolon ) { - this.insertSemicolon( code ); + if (this.end > c) { + code.overwrite(c, this.end, needsSemicolon ? ";" : ""); + } else if (needsSemicolon) { + this.insertSemicolon(code); } } } diff --git a/src/ast/nodes/VariableDeclarator.js b/src/ast/nodes/VariableDeclarator.js index 753d38402b2..f3225ccb257 100644 --- a/src/ast/nodes/VariableDeclarator.js +++ b/src/ast/nodes/VariableDeclarator.js @@ -47,7 +47,7 @@ export default class VariableDeclarator extends Node { if ( this.activated ) return; this.activated = true; - this.run( this.findScope() ); + this.run(); // if declaration is inside a block, ensure that the block // is marked for inclusion @@ -60,27 +60,22 @@ export default class VariableDeclarator extends Node { } } - hasEffects ( scope ) { - return this.init && this.init.hasEffects( scope ); + hasEffects () { + return this.init && this.init.hasEffects(); } - initialise ( scope ) { + initialiseNode () { this.proxies = new Map(); - - const lexicalBoundary = scope.findLexicalBoundary(); - - const init = this.init ? - ( this.id.type === 'Identifier' ? this.init : UNKNOWN ) : // TODO maybe UNKNOWN is unnecessary + const lexicalBoundary = this.scope.findLexicalBoundary(); + const init = this.init ? ( this.id.type === 'Identifier' ? this.init : UNKNOWN ) : // TODO maybe UNKNOWN is unnecessary null; extractNames( this.id ).forEach( name => { const proxy = new DeclaratorProxy( name, this, lexicalBoundary.isModuleScope, init ); this.proxies.set( name, proxy ); - scope.addDeclaration( name, proxy, this.parent.kind === 'var' ); - }); - - super.initialise( scope ); + this.scope.addDeclaration( name, proxy, this.parent.kind === 'var' ); + } ); } render ( code, es ) { @@ -94,7 +89,7 @@ export default class VariableDeclarator extends Node { code.remove( this.start, this.end ); } } - }); + } ); super.render( code, es ); } diff --git a/src/ast/nodes/index.js b/src/ast/nodes/index.js index bfda415448a..6cfc0b43c13 100644 --- a/src/ast/nodes/index.js +++ b/src/ast/nodes/index.js @@ -26,7 +26,6 @@ import LogicalExpression from './LogicalExpression.js'; import MemberExpression from './MemberExpression.js'; import NewExpression from './NewExpression.js'; import ObjectExpression from './ObjectExpression.js'; -import ReturnStatement from './ReturnStatement.js'; import Statement from './shared/Statement.js'; import TemplateLiteral from './TemplateLiteral.js'; import ThisExpression from './ThisExpression.js'; @@ -66,7 +65,7 @@ export default { MemberExpression, NewExpression, ObjectExpression, - ReturnStatement, + ReturnStatement: Statement, SwitchStatement: Statement, TemplateLiteral, ThisExpression, diff --git a/src/ast/nodes/shared/Class.js b/src/ast/nodes/shared/Class.js new file mode 100644 index 00000000000..fa550396b5c --- /dev/null +++ b/src/ast/nodes/shared/Class.js @@ -0,0 +1,32 @@ +import Node from '../../Node.js'; +import Scope from '../../scopes/Scope.js'; + +export default class ClassExpression extends Node { + activate () { + if ( this.activated ) return; + this.activated = true; + + if ( this.superClass ) this.superClass.run(); + this.body.run(); + } + + addReference () {} + + getName () { + return this.name; + } + + initialiseChildren () { + if ( this.superClass ) { + this.superClass.initialise( this.scope ); + } + this.body.initialise( this.scope ); + } + + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: true + } ); + } +} diff --git a/src/ast/nodes/shared/Function.js b/src/ast/nodes/shared/Function.js new file mode 100644 index 00000000000..21dae785507 --- /dev/null +++ b/src/ast/nodes/shared/Function.js @@ -0,0 +1,33 @@ +import Node from '../../Node.js'; +import Scope from '../../scopes/Scope.js'; +import extractNames from '../../utils/extractNames.js'; + +export default class Function extends Node { + bind () { + if ( this.id ) this.id.bind(); + this.params.forEach( param => param.bind() ); + this.body.bind(); + } + + hasEffects () { + return false; + } + + initialiseChildren () { + this.params.forEach( param => { + param.initialise( this.scope ); + extractNames( param ).forEach( name => this.scope.addDeclaration( name, null, false, true ) ); + } ); + this.body.initialiseAndReplaceScope ? + this.body.initialiseAndReplaceScope( this.scope ) : + this.body.initialise( this.scope ); + } + + initialiseScope ( parentScope ) { + this.scope = new Scope( { + parent: parentScope, + isBlockScope: false, + isLexicalBoundary: true + } ); + } +} diff --git a/src/ast/nodes/shared/Statement.js b/src/ast/nodes/shared/Statement.js index 5667652071c..5a18476ec2b 100644 --- a/src/ast/nodes/shared/Statement.js +++ b/src/ast/nodes/shared/Statement.js @@ -9,8 +9,8 @@ export default class Statement extends Node { } } - run ( scope ) { + run () { this.shouldInclude = true; - super.run( scope ); + super.run(); } } From 6c80cbb538a689219d062986bd0f43fbcd24a8ff Mon Sep 17 00:00:00 2001 From: Lukas Taegert Date: Mon, 7 Aug 2017 07:52:02 +0200 Subject: [PATCH 09/15] Remove a few scope parameters previously missed --- src/ast/Node.js | 4 ++-- src/ast/nodes/AssignmentExpression.js | 2 +- src/ast/nodes/ClassDeclaration.js | 2 +- src/ast/nodes/ExportDefaultDeclaration.js | 2 +- src/ast/nodes/shared/callHasEffects.js | 13 ++++++------- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/ast/Node.js b/src/ast/Node.js index 9ce677d6a39..b9e1f405c2a 100644 --- a/src/ast/Node.js +++ b/src/ast/Node.js @@ -44,11 +44,11 @@ export default class Node { if ( value ) { if ( 'length' in value ) { for ( const child of value ) { - if ( child && child.hasEffects( this.scope ) ) { + if ( child && child.hasEffects() ) { return true; } } - } else if ( value.hasEffects( this.scope ) ) { + } else if ( value.hasEffects() ) { return true; } } diff --git a/src/ast/nodes/AssignmentExpression.js b/src/ast/nodes/AssignmentExpression.js index ba846a3cbef..6a9ef91d7a2 100644 --- a/src/ast/nodes/AssignmentExpression.js +++ b/src/ast/nodes/AssignmentExpression.js @@ -30,7 +30,7 @@ export default class AssignmentExpression extends Node { } hasEffects () { - const hasEffects = this.isUsedByBundle() || this.right.hasEffects( this.scope ); + const hasEffects = this.isUsedByBundle() || this.right.hasEffects(); return hasEffects; } diff --git a/src/ast/nodes/ClassDeclaration.js b/src/ast/nodes/ClassDeclaration.js index 2d1b98ffae4..6d9863273df 100644 --- a/src/ast/nodes/ClassDeclaration.js +++ b/src/ast/nodes/ClassDeclaration.js @@ -15,7 +15,7 @@ export default class ClassDeclaration extends Class { parentScope.addDeclaration( this.name, this, false, false ); this.id.initialise( parentScope ); } - super.initialiseChildren(parentScope); + super.initialiseChildren( parentScope ); } render ( code, es ) { diff --git a/src/ast/nodes/ExportDefaultDeclaration.js b/src/ast/nodes/ExportDefaultDeclaration.js index efe17298911..433b9b25139 100644 --- a/src/ast/nodes/ExportDefaultDeclaration.js +++ b/src/ast/nodes/ExportDefaultDeclaration.js @@ -87,7 +87,7 @@ export default class ExportDefaultDeclaration extends Node { if ( functionOrClassDeclaration.test( this.declaration.type ) ) { code.remove( this.leadingCommentStart || this.start, this.next || this.end ); } else { - const hasEffects = this.declaration.hasEffects( this.module.scope ); + const hasEffects = this.declaration.hasEffects(); code.remove( this.start, hasEffects ? declaration_start : this.next || this.end ); } } else if ( name === this.declaration.name ) { diff --git a/src/ast/nodes/shared/callHasEffects.js b/src/ast/nodes/shared/callHasEffects.js index 09d58ccfce9..3fc7e6bbccd 100644 --- a/src/ast/nodes/shared/callHasEffects.js +++ b/src/ast/nodes/shared/callHasEffects.js @@ -9,21 +9,21 @@ function isES5Function ( node ) { return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration'; } -function hasEffectsNew ( node, scope ) { +function hasEffectsNew ( node ) { let inner = node; if ( inner.type === 'ExpressionStatement' ) { inner = inner.expression; if ( inner.type === 'AssignmentExpression' ) { - if ( inner.right.hasEffects( scope ) ) { + if ( inner.right.hasEffects() ) { return true; } else { inner = inner.left; if ( inner.type === 'MemberExpression' ) { - if ( inner.computed && inner.property.hasEffects( scope ) ) { + if ( inner.computed && inner.property.hasEffects() ) { return true; } else { @@ -38,7 +38,7 @@ function hasEffectsNew ( node, scope ) { } } - return node.hasEffects( scope ); + return node.hasEffects(); } function fnHasEffects ( fn, isNew ) { @@ -46,11 +46,10 @@ function fnHasEffects ( fn, isNew ) { currentlyCalling.add( fn ); // handle body-less arrow functions - const scope = fn.body.scope || fn.scope; const body = fn.body.type === 'BlockStatement' ? fn.body.body : [ fn.body ]; for ( const node of body ) { - if ( isNew ? hasEffectsNew( node, scope ) : node.hasEffects( scope ) ) { + if ( isNew ? hasEffectsNew( node ) : node.hasEffects() ) { currentlyCalling.delete( fn ); return true; } @@ -61,7 +60,7 @@ function fnHasEffects ( fn, isNew ) { } export default function callHasEffects ( scope, callee, isNew ) { - const values = new Set([ callee ]); + const values = new Set( [ callee ] ); for ( const node of values ) { if ( node === UNKNOWN ) return true; // err on side of caution From c35648fab5287755741148a48cb00c6c8dba3cad Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 8 Aug 2017 22:19:14 -0400 Subject: [PATCH 10/15] update some tests --- .../custom-path-resolver-async/_config.js | 2 ++ .../custom-path-resolver-sync/_config.js | 2 ++ .../_config.js | 2 ++ test/function/empty-exports/_config.js | 23 ------------------- test/function/empty-exports/main.js | 1 - .../namespace-missing-export/_config.js | 3 +++ test/function/unused-import/_config.js | 2 ++ test/mocha.opts | 1 - 8 files changed, 11 insertions(+), 25 deletions(-) delete mode 100644 test/function/empty-exports/_config.js delete mode 100644 test/function/empty-exports/main.js diff --git a/test/function/custom-path-resolver-async/_config.js b/test/function/custom-path-resolver-async/_config.js index c471e027b8f..27b46dc9f9d 100644 --- a/test/function/custom-path-resolver-async/_config.js +++ b/test/function/custom-path-resolver-async/_config.js @@ -23,6 +23,8 @@ module.exports = { warnings: [ { code: 'UNRESOLVED_IMPORT', + importer: 'main.js', + source: 'path', message: `'path' is imported by main.js, but could not be resolved – treating it as an external dependency`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` } diff --git a/test/function/custom-path-resolver-sync/_config.js b/test/function/custom-path-resolver-sync/_config.js index f32cb9ab7b3..49c926ef1fe 100644 --- a/test/function/custom-path-resolver-sync/_config.js +++ b/test/function/custom-path-resolver-sync/_config.js @@ -16,6 +16,8 @@ module.exports = { warnings: [ { code: 'UNRESOLVED_IMPORT', + importer: 'main.js', + source: 'path', message: `'path' is imported by main.js, but could not be resolved – treating it as an external dependency`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` } diff --git a/test/function/does-not-hang-on-missing-module/_config.js b/test/function/does-not-hang-on-missing-module/_config.js index aeb5386224e..6540af6f5f8 100644 --- a/test/function/does-not-hang-on-missing-module/_config.js +++ b/test/function/does-not-hang-on-missing-module/_config.js @@ -5,6 +5,8 @@ module.exports = { warnings: [ { code: 'UNRESOLVED_IMPORT', + importer: 'main.js', + source: 'unlessYouCreatedThisFileForSomeReason', message: `'unlessYouCreatedThisFileForSomeReason' is imported by main.js, but could not be resolved – treating it as an external dependency`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` } diff --git a/test/function/empty-exports/_config.js b/test/function/empty-exports/_config.js deleted file mode 100644 index e928a59b106..00000000000 --- a/test/function/empty-exports/_config.js +++ /dev/null @@ -1,23 +0,0 @@ -module.exports = { - description: 'warns on export {}, but does not fail', - warnings: [ - { - code: 'EMPTY_EXPORT', - message: 'Empty export declaration', - pos: 0, - loc: { - file: require( 'path' ).resolve( __dirname, 'main.js' ), - line: 1, - column: 0 - }, - frame: ` - 1: export {}; - ^ - ` - }, - { - code: 'EMPTY_BUNDLE', - message: 'Generated an empty bundle' - } - ] -}; diff --git a/test/function/empty-exports/main.js b/test/function/empty-exports/main.js deleted file mode 100644 index cb0ff5c3b54..00000000000 --- a/test/function/empty-exports/main.js +++ /dev/null @@ -1 +0,0 @@ -export {}; diff --git a/test/function/namespace-missing-export/_config.js b/test/function/namespace-missing-export/_config.js index 485a1ba17bd..6848cc28686 100644 --- a/test/function/namespace-missing-export/_config.js +++ b/test/function/namespace-missing-export/_config.js @@ -2,6 +2,9 @@ module.exports = { warnings: [ { code: 'MISSING_EXPORT', + exporter: 'empty.js', + importer: 'main.js', + missing: 'foo', message: `'foo' is not exported by 'empty.js'`, pos: 61, loc: { diff --git a/test/function/unused-import/_config.js b/test/function/unused-import/_config.js index 0f99f594b94..a60eaf85acf 100644 --- a/test/function/unused-import/_config.js +++ b/test/function/unused-import/_config.js @@ -5,6 +5,8 @@ module.exports = { warnings: [ { code: 'UNRESOLVED_IMPORT', + importer: 'main.js', + source: 'external', message: `'external' is imported by main.js, but could not be resolved – treating it as an external dependency`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` }, diff --git a/test/mocha.opts b/test/mocha.opts index fcb33f02a08..0339aeebdc1 100644 --- a/test/mocha.opts +++ b/test/mocha.opts @@ -1,3 +1,2 @@ ---bail --compilers js:buble/register test/test.js \ No newline at end of file From 1f31de4a60eba8afd679f055d6835caca2fb13fa Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 9 Aug 2017 15:42:51 -0400 Subject: [PATCH 11/15] implement rollup.watch --- .gitignore | 1 + src/rollup.js | 186 +-------------------- src/rollup/index.js | 184 +++++++++++++++++++++ src/watch/chokidar.js | 11 ++ src/watch/fileWatchers.js | 88 ++++++++++ src/watch/index.js | 153 +++++++++++++++++ test/test.js | 254 +++++++++++++++++++++++++++++ test/watch/samples/basic/main.js | 1 + test/watch/samples/ignored/bar.js | 1 + test/watch/samples/ignored/foo.js | 1 + test/watch/samples/ignored/main.js | 4 + 11 files changed, 700 insertions(+), 184 deletions(-) create mode 100644 src/rollup/index.js create mode 100644 src/watch/chokidar.js create mode 100644 src/watch/fileWatchers.js create mode 100644 src/watch/index.js create mode 100644 test/watch/samples/basic/main.js create mode 100644 test/watch/samples/ignored/bar.js create mode 100644 test/watch/samples/ignored/foo.js create mode 100644 test/watch/samples/ignored/main.js diff --git a/.gitignore b/.gitignore index f5abf8d2c85..e19d49a9cf5 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ coverage .commithash .idea bin/rollup +test/_tmp \ No newline at end of file diff --git a/src/rollup.js b/src/rollup.js index 12eb5d44d6c..7845ad0145c 100644 --- a/src/rollup.js +++ b/src/rollup.js @@ -1,184 +1,2 @@ -import { timeStart, timeEnd, flushTime } from './utils/flushTime.js'; -import { basename } from './utils/path.js'; -import { writeFile } from './utils/fs.js'; -import { assign, keys } from './utils/object.js'; -import { mapSequence } from './utils/promise.js'; -import validateKeys from './utils/validateKeys.js'; -import error from './utils/error.js'; -import { SOURCEMAPPING_URL } from './utils/sourceMappingURL.js'; -import Bundle from './Bundle.js'; - -export const VERSION = '<@VERSION@>'; - -const ALLOWED_KEYS = [ - 'acorn', - 'amd', - 'banner', - 'cache', - 'context', - 'dest', - 'entry', - 'exports', - 'extend', - 'external', - 'footer', - 'format', - 'globals', - 'indent', - 'interop', - 'intro', - 'legacy', - 'moduleContext', - 'moduleName', - 'noConflict', - 'onwarn', - 'outro', - 'paths', - 'plugins', - 'preferConst', - 'pureExternalModules', - 'sourceMap', - 'sourceMapFile', - 'targets', - 'treeshake', - 'useStrict', - 'watch' -]; - -function checkAmd ( options ) { - if ( options.moduleId ) { - if ( options.amd ) throw new Error( 'Cannot have both options.amd and options.moduleId' ); - - options.amd = { id: options.moduleId }; - delete options.moduleId; - - const msg = `options.moduleId is deprecated in favour of options.amd = { id: moduleId }`; - if ( options.onwarn ) { - options.onwarn( msg ); - } else { - console.warn( msg ); // eslint-disable-line no-console - } - } -} - -function checkOptions ( options ) { - if ( !options ) { - throw new Error( 'You must supply an options object to rollup' ); - } - - if ( options.transform || options.load || options.resolveId || options.resolveExternal ) { - throw new Error( 'The `transform`, `load`, `resolveId` and `resolveExternal` options are deprecated in favour of a unified plugin API. See https://github.com/rollup/rollup/wiki/Plugins for details' ); - } - - checkAmd (options); - - const err = validateKeys( keys(options), ALLOWED_KEYS ); - if ( err ) throw err; -} - -const throwAsyncGenerateError = { - get () { - throw new Error( `bundle.generate(...) now returns a Promise instead of a { code, map } object` ); - } -}; - -export function rollup ( options ) { - try { - checkOptions( options ); - const bundle = new Bundle( options ); - - timeStart( '--BUILD--' ); - - return bundle.build().then( () => { - timeEnd( '--BUILD--' ); - - function generate ( options = {} ) { - if ( !options.format ) { - bundle.warn({ // TODO make this an error - code: 'MISSING_FORMAT', - message: `No format option was supplied – defaulting to 'es'`, - url: `https://github.com/rollup/rollup/wiki/JavaScript-API#format` - }); - - options.format = 'es'; - } - - checkAmd( options ); - - timeStart( '--GENERATE--' ); - - const promise = Promise.resolve() - .then( () => bundle.render( options ) ) - .then( rendered => { - timeEnd( '--GENERATE--' ); - - bundle.plugins.forEach( plugin => { - if ( plugin.ongenerate ) { - plugin.ongenerate( assign({ - bundle: result - }, options ), rendered); - } - }); - - flushTime(); - - return rendered; - }); - - Object.defineProperty( promise, 'code', throwAsyncGenerateError ); - Object.defineProperty( promise, 'map', throwAsyncGenerateError ); - - return promise; - } - - const result = { - imports: bundle.externalModules.map( module => module.id ), - exports: keys( bundle.entryModule.exports ), - modules: bundle.orderedModules.map( module => module.toJSON() ), - - generate, - write: options => { - if ( !options || !options.dest ) { - error({ - code: 'MISSING_OPTION', - message: 'You must supply options.dest to bundle.write' - }); - } - - const dest = options.dest; - return generate( options ).then( output => { - let { code, map } = output; - - const promises = []; - - if ( options.sourceMap ) { - let url; - - if ( options.sourceMap === 'inline' ) { - url = map.toUrl(); - } else { - url = `${basename( dest )}.map`; - promises.push( writeFile( dest + '.map', map.toString() ) ); - } - - code += `//# ${SOURCEMAPPING_URL}=${url}\n`; - } - - promises.push( writeFile( dest, code ) ); - return Promise.all( promises ).then( () => { - return mapSequence( bundle.plugins.filter( plugin => plugin.onwrite ), plugin => { - return Promise.resolve( plugin.onwrite( assign({ - bundle: result - }, options ), output)); - }); - }); - }); - } - }; - - return result; - }); - } catch ( err ) { - return Promise.reject( err ); - } -} +export { default as rollup } from './rollup/index.js'; +export { default as watch } from './watch/index.js'; \ No newline at end of file diff --git a/src/rollup/index.js b/src/rollup/index.js new file mode 100644 index 00000000000..7213c1ea1f8 --- /dev/null +++ b/src/rollup/index.js @@ -0,0 +1,184 @@ +import { timeStart, timeEnd, flushTime } from '../utils/flushTime.js'; +import { basename } from '../utils/path.js'; +import { writeFile } from '../utils/fs.js'; +import { assign, keys } from '../utils/object.js'; +import { mapSequence } from '../utils/promise.js'; +import validateKeys from '../utils/validateKeys.js'; +import error from '../utils/error.js'; +import { SOURCEMAPPING_URL } from '../utils/sourceMappingURL.js'; +import Bundle from '../Bundle.js'; + +export const VERSION = '<@VERSION@>'; + +const ALLOWED_KEYS = [ + 'acorn', + 'amd', + 'banner', + 'cache', + 'context', + 'dest', + 'entry', + 'exports', + 'extend', + 'external', + 'footer', + 'format', + 'globals', + 'indent', + 'interop', + 'intro', + 'legacy', + 'moduleContext', + 'moduleName', + 'noConflict', + 'onwarn', + 'outro', + 'paths', + 'plugins', + 'preferConst', + 'pureExternalModules', + 'sourceMap', + 'sourceMapFile', + 'targets', + 'treeshake', + 'useStrict', + 'watch' +]; + +function checkAmd ( options ) { + if ( options.moduleId ) { + if ( options.amd ) throw new Error( 'Cannot have both options.amd and options.moduleId' ); + + options.amd = { id: options.moduleId }; + delete options.moduleId; + + const msg = `options.moduleId is deprecated in favour of options.amd = { id: moduleId }`; + if ( options.onwarn ) { + options.onwarn( msg ); + } else { + console.warn( msg ); // eslint-disable-line no-console + } + } +} + +function checkOptions ( options ) { + if ( !options ) { + throw new Error( 'You must supply an options object to rollup' ); + } + + if ( options.transform || options.load || options.resolveId || options.resolveExternal ) { + throw new Error( 'The `transform`, `load`, `resolveId` and `resolveExternal` options are deprecated in favour of a unified plugin API. See https://github.com/rollup/rollup/wiki/Plugins for details' ); + } + + checkAmd (options); + + const err = validateKeys( keys(options), ALLOWED_KEYS ); + if ( err ) throw err; +} + +const throwAsyncGenerateError = { + get () { + throw new Error( `bundle.generate(...) now returns a Promise instead of a { code, map } object` ); + } +}; + +export default function rollup ( options ) { + try { + checkOptions( options ); + const bundle = new Bundle( options ); + + timeStart( '--BUILD--' ); + + return bundle.build().then( () => { + timeEnd( '--BUILD--' ); + + function generate ( options = {} ) { + if ( !options.format ) { + bundle.warn({ // TODO make this an error + code: 'MISSING_FORMAT', + message: `No format option was supplied – defaulting to 'es'`, + url: `https://github.com/rollup/rollup/wiki/JavaScript-API#format` + }); + + options.format = 'es'; + } + + checkAmd( options ); + + timeStart( '--GENERATE--' ); + + const promise = Promise.resolve() + .then( () => bundle.render( options ) ) + .then( rendered => { + timeEnd( '--GENERATE--' ); + + bundle.plugins.forEach( plugin => { + if ( plugin.ongenerate ) { + plugin.ongenerate( assign({ + bundle: result + }, options ), rendered); + } + }); + + flushTime(); + + return rendered; + }); + + Object.defineProperty( promise, 'code', throwAsyncGenerateError ); + Object.defineProperty( promise, 'map', throwAsyncGenerateError ); + + return promise; + } + + const result = { + imports: bundle.externalModules.map( module => module.id ), + exports: keys( bundle.entryModule.exports ), + modules: bundle.orderedModules.map( module => module.toJSON() ), + + generate, + write: options => { + if ( !options || !options.dest ) { + error({ + code: 'MISSING_OPTION', + message: 'You must supply options.dest to bundle.write' + }); + } + + const dest = options.dest; + return generate( options ).then( output => { + let { code, map } = output; + + const promises = []; + + if ( options.sourceMap ) { + let url; + + if ( options.sourceMap === 'inline' ) { + url = map.toUrl(); + } else { + url = `${basename( dest )}.map`; + promises.push( writeFile( dest + '.map', map.toString() ) ); + } + + code += `//# ${SOURCEMAPPING_URL}=${url}\n`; + } + + promises.push( writeFile( dest, code ) ); + return Promise.all( promises ).then( () => { + return mapSequence( bundle.plugins.filter( plugin => plugin.onwrite ), plugin => { + return Promise.resolve( plugin.onwrite( assign({ + bundle: result + }, options ), output)); + }); + }); + }); + } + }; + + return result; + }); + } catch ( err ) { + return Promise.reject( err ); + } +} diff --git a/src/watch/chokidar.js b/src/watch/chokidar.js new file mode 100644 index 00000000000..18ad0a5540f --- /dev/null +++ b/src/watch/chokidar.js @@ -0,0 +1,11 @@ +import relative from 'require-relative'; + +let chokidar; + +try { + chokidar = relative( 'chokidar', process.cwd() ); +} catch (err) { + chokidar = null; +} + +export default chokidar; \ No newline at end of file diff --git a/src/watch/fileWatchers.js b/src/watch/fileWatchers.js new file mode 100644 index 00000000000..0070f6572af --- /dev/null +++ b/src/watch/fileWatchers.js @@ -0,0 +1,88 @@ + +import * as fs from 'fs'; +import chokidar from './chokidar.js'; + +const opts = { encoding: 'utf-8', persistent: true }; + +const watchers = new Map(); + +export function addTask(id, task) { + if (!watchers.has(id)) { + const watcher = new FileWatcher(id, () => { + watchers.delete(id); + }); + + if (watcher.fileExists) { + watchers.set(id, watcher); + } else { + return; + } + } + + watchers.get(id).tasks.add(task); +} + +export function deleteTask(id, target) { + const watcher = watchers.get(id); + watcher.tasks.delete(target); + + if (watcher.tasks.size === 0) { + watcher.close(); + watchers.delete(id); + } +} + +export default class FileWatcher { + constructor(id, chokidarOptions, dispose) { + this.tasks = new Set(); + + let data; + + try { + fs.statSync(id); + this.fileExists = true; + } catch (err) { + if (err.code === 'ENOENT') { + // can't watch files that don't exist (e.g. injected + // by plugins somehow) + this.fileExists = false; + return; + } else { + throw err; + } + } + + const handleWatchEvent = event => { + if (event === 'rename' || event === 'unlink') { + this.fsWatcher.close(); + dispose(); + this.trigger(); + } else { + // this is necessary because we get duplicate events... + const contents = fs.readFileSync(id, 'utf-8'); + if (contents !== data) { + data = contents; + this.trigger(); + } + } + }; + + if (chokidarOptions) { + this.fsWatcher = chokidar + .watch(id, chokidarOptions) + .on('all', handleWatchEvent); + } else { + this.fsWatcher = fs.watch(id, opts, handleWatchEvent); + } + } + + close() { + this.fsWatcher.close(); + } + + trigger() { + this.tasks.forEach(task => { + task.makeDirty(); + }); + } +} diff --git a/src/watch/index.js b/src/watch/index.js new file mode 100644 index 00000000000..52cb823e61a --- /dev/null +++ b/src/watch/index.js @@ -0,0 +1,153 @@ +import path from 'path'; +import EventEmitter from 'events'; +import rollup from '../rollup/index.js'; +import ensureArray from '../utils/ensureArray.js'; +import { mapSequence } from '../utils/promise.js'; +import { addTask, deleteTask } from './fileWatchers.js'; + +const DELAY = 100; + +class Watcher extends EventEmitter { + constructor(configs) { + super(); + + this.dirty = true; + this.running = false; + this.tasks = ensureArray(configs).map(config => new Task(this, config)); + + process.nextTick(() => { + this.run(); + }); + } + + close() { + this.tasks.forEach(task => { + task.close(); + }); + } + + error(error) { + this.emit('event', { + code: 'ERROR', + error + }); + } + + makeDirty() { + if (this.dirty) return; + this.dirty = true; + + if (!this.running) { + setTimeout(() => { + this.run(); + }, DELAY); + } + } + + run() { + this.running = true; + this.dirty = false; + + // TODO + this.emit('event', { + code: 'BUILD_START' + }); + + mapSequence(this.tasks, task => { + return task.run().catch(error => { + this.emit('event', { + code: 'ERROR', + error + }); + }); + }).then(() => { + this.running = false; + + this.emit('event', { + code: 'BUILD_END' + }); + + if (this.dirty) this.run(); + }); + } +} + +class Task { + constructor(watcher, config) { + this.cache = null; + this.watcher = watcher; + this.config = config; + + this.closed = false; + this.watched = new Set(); + + this.dests = new Set( + (config.dest ? [config.dest] : config.targets.map(t => t.dest)).map(dest => path.resolve(dest)) + ); + } + + close() { + this.closed = true; + this.watched.forEach(id => { + deleteTask(id, this); + }); + } + + makeDirty() { + if (!this.dirty) { + this.dirty = true; + this.watcher.makeDirty(); + } + } + + run() { + this.dirty = false; + + const config = Object.assign(this.config, { + cache: this.cache + }); + + return rollup(config).then(bundle => { + if (this.closed) return; + + this.cache = bundle; + + const watched = new Set(); + + bundle.modules.forEach(module => { + if (this.dests.has(module.id)) { + throw new Error('Cannot import the generated bundle'); + } + + watched.add(module.id); + addTask(module.id, this); + }); + + this.watched.forEach(id => { + if (!watched.has(id)) deleteTask(id, this); + }); + + this.watched = watched; + + if (this.config.dest) { + return bundle.write({ + format: this.config.format, + dest: this.config.dest + }); + } + + return Promise.all( + this.config.targets.map(target => { + return bundle.write({ + format: target.format, + dest: target.dest + }); + }) + ); + }); + } +} + +export default function watch(configs) { + return new Watcher(configs); +} \ No newline at end of file diff --git a/test/test.js b/test/test.js index 1afe24fc399..a1a071caece 100644 --- a/test/test.js +++ b/test/test.js @@ -14,6 +14,8 @@ const FORM = path.resolve( __dirname, 'form' ); const SOURCEMAPS = path.resolve( __dirname, 'sourcemaps' ); const CLI = path.resolve( __dirname, 'cli' ); +const cwd = process.cwd(); + const PROFILES = [ { format: 'amd' }, { format: 'cjs' }, @@ -100,6 +102,12 @@ function compareError ( actual, expected ) { assert.deepEqual( actual, expected ); } +function wait ( ms ) { + return new Promise( fulfil => { + setTimeout( fulfil, ms ); + }); +} + describe( 'rollup', function () { this.timeout( 10000 ); @@ -951,4 +959,250 @@ describe( 'rollup', function () { }); }); }); + + describe.only( 'rollup.watch', () => { + beforeEach( () => { + process.chdir(cwd); + return sander.rimraf( 'test/_tmp' ); + }); + + function run ( file ) { + const resolved = require.resolve( file ); + delete require.cache[ resolved ]; + return require( resolved ); + } + + function sequence ( watcher, events ) { + return new Promise( ( fulfil, reject ) => { + function go ( event ) { + const next = events.shift(); + + if ( !next ) { + fulfil(); + } + + else if ( typeof next === 'string' ) { + watcher.once( 'event', event => { + if ( event.code !== next ) { + reject( new Error( `Expected ${next} error, got ${event.code}` ) ); + } else { + go( event ); + } + }); + } + + else { + Promise.resolve() + .then( () => wait( 100 ) ) // gah, this appears to be necessary to fix random errors + .then( () => next( event ) ) + .then( go ) + .catch( reject ); + } + } + + go(); + }); + } + + describe( 'fs.watch', () => { + runTests( false ); + }); + + if ( !process.env.CI ) { + describe( 'chokidar', () => { + runTests( true ); + }); + } + + function runTests ( chokidar ) { + it( 'watches a file', () => { + return sander.copydir( 'test/watch/samples/basic' ).to( 'test/_tmp/input' ).then( () => { + const watcher = rollup.watch({ + entry: 'test/_tmp/input/main.js', + dest: 'test/_tmp/output/bundle.js', + format: 'cjs', + watch: { chokidar } + }); + + return sequence( watcher, [ + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 42 ); + sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); + }, + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 43 ); + watcher.close(); + } + ]); + }); + }); + + it( 'recovers from an error', () => { + return sander.copydir( 'test/watch/samples/basic' ).to( 'test/_tmp/input' ).then( () => { + const watcher = rollup.watch({ + entry: 'test/_tmp/input/main.js', + dest: 'test/_tmp/output/bundle.js', + format: 'cjs', + watch: { chokidar } + }); + + return sequence( watcher, [ + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 42 ); + sander.writeFileSync( 'test/_tmp/input/main.js', 'export nope;' ); + }, + 'BUILD_START', + 'ERROR', + () => { + sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); + }, + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 43 ); + watcher.close(); + } + ]); + }); + }); + + it( 'recovers from an error even when erroring file was "renamed" (#38)', () => { + return sander.copydir( 'test/watch/samples/basic' ).to( 'test/_tmp/input' ).then( () => { + const watcher = rollup.watch({ + entry: 'test/_tmp/input/main.js', + dest: 'test/_tmp/output/bundle.js', + format: 'cjs', + watch: { chokidar } + }); + + return sequence( watcher, [ + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 42 ); + sander.unlinkSync( 'test/_tmp/input/main.js' ); + sander.writeFileSync( 'test/_tmp/input/main.js', 'export nope;' ); + }, + 'BUILD_START', + 'ERROR', + () => { + sander.unlinkSync( 'test/_tmp/input/main.js' ); + sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); + }, + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 43 ); + watcher.close(); + } + ]); + }); + }); + + it( 'refuses to watch the output file (#15)', () => { + return sander.copydir( 'test/watch/samples/basic' ).to( 'test/_tmp/input' ).then( () => { + const watcher = rollup.watch({ + entry: 'test/_tmp/input/main.js', + dest: 'test/_tmp/output/bundle.js', + format: 'cjs', + watch: { chokidar } + }); + + return sequence( watcher, [ + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 42 ); + sander.writeFileSync( 'test/_tmp/input/main.js', `import '../output/bundle.js'` ); + }, + 'BUILD_START', + 'ERROR', + event => { + assert.equal( event.error.message, 'Cannot import the generated bundle' ); + sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); + }, + 'BUILD_START', + 'BUILD_END', + () => { + assert.equal( run( './_tmp/output/bundle.js' ), 43 ); + watcher.close(); + } + ]); + }); + }); + + it( 'ignores files that are not specified in options.watch.include, if given', () => { + return sander.copydir( 'test/watch/samples/ignored' ).to( 'test/_tmp/input' ).then( () => { + const watcher = rollup.watch({ + entry: 'test/_tmp/input/main.js', + dest: 'test/_tmp/output/bundle.js', + format: 'cjs', + watch: { + chokidar, + include: ['test/_tmp/input/+(main|foo).js'] + } + }); + + return sequence( watcher, [ + 'BUILD_START', + 'BUILD_END', + () => { + assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-1', bar: 'bar-1' }); + sander.writeFileSync( 'test/_tmp/input/foo.js', `export default 'foo-2';` ); + }, + 'BUILD_START', + 'BUILD_END', + () => { + assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-2', bar: 'bar-1' }); + sander.writeFileSync( 'test/_tmp/input/bar.js', `export default 'bar-2';` ); + }, + () => { + assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-2', bar: 'bar-1' }); + watcher.close(); + } + ]); + }); + }); + + it( 'ignores files that are specified in options.watch.exclude, if given', () => { + return sander.copydir( 'test/watch/samples/ignored' ).to( 'test/_tmp/input' ).then( () => { + const watcher = rollup.watch({ + entry: 'test/_tmp/input/main.js', + dest: 'test/_tmp/output/bundle.js', + format: 'cjs', + watch: { + chokidar, + exclude: ['test/_tmp/input/bar.js'] + } + }); + + return sequence( watcher, [ + 'BUILD_START', + 'BUILD_END', + () => { + assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-1', bar: 'bar-1' }); + sander.writeFileSync( 'test/_tmp/input/foo.js', `export default 'foo-2';` ); + }, + 'BUILD_START', + 'BUILD_END', + () => { + assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-2', bar: 'bar-1' }); + sander.writeFileSync( 'test/_tmp/input/bar.js', `export default 'bar-2';` ); + }, + () => { + assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-2', bar: 'bar-1' }); + watcher.close(); + } + ]); + }); + }); + } + }); + }); diff --git a/test/watch/samples/basic/main.js b/test/watch/samples/basic/main.js new file mode 100644 index 00000000000..7a4e8a723a4 --- /dev/null +++ b/test/watch/samples/basic/main.js @@ -0,0 +1 @@ +export default 42; diff --git a/test/watch/samples/ignored/bar.js b/test/watch/samples/ignored/bar.js new file mode 100644 index 00000000000..e36cec404c9 --- /dev/null +++ b/test/watch/samples/ignored/bar.js @@ -0,0 +1 @@ +export default 'bar-1'; \ No newline at end of file diff --git a/test/watch/samples/ignored/foo.js b/test/watch/samples/ignored/foo.js new file mode 100644 index 00000000000..8402d2657cb --- /dev/null +++ b/test/watch/samples/ignored/foo.js @@ -0,0 +1 @@ +export default 'foo-1'; \ No newline at end of file diff --git a/test/watch/samples/ignored/main.js b/test/watch/samples/ignored/main.js new file mode 100644 index 00000000000..d42f9080e17 --- /dev/null +++ b/test/watch/samples/ignored/main.js @@ -0,0 +1,4 @@ +import foo from './foo.js'; +import bar from './bar.js'; + +export { foo, bar }; \ No newline at end of file From f4d9765c381b5d5c20d1336bd9724112459defe6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 10 Aug 2017 19:31:11 -0400 Subject: [PATCH 12/15] use rollup.watch in CLI, update tests, various other things --- .eslintrc => .eslintrc.json | 1 - bin/src/logging.js | 39 ++-- bin/src/run/batchWarnings.js | 212 +++++++++++------ bin/src/run/build.js | 50 ++-- bin/src/run/createWatcher.js | 217 ------------------ bin/src/run/index.js | 29 ++- bin/src/run/mergeOptions.js | 21 +- bin/src/run/watch.js | 88 ++++--- package-lock.json | 31 +++ package.json | 1 + rollup.config.js | 7 +- src/Bundle.js | 16 +- src/Module.js | 1 + src/ast/scopes/ModuleScope.js | 2 + src/finalisers/shared/getGlobalNameMaker.js | 2 + src/finalisers/shared/warnOnBuiltins.js | 3 +- src/rollup/index.js | 8 +- src/utils/collapseSourcemaps.js | 1 + src/utils/error.js | 3 +- src/utils/transform.js | 24 +- src/watch/fileWatchers.js | 23 +- src/watch/index.js | 144 +++++++----- .../namespace-missing-export/_config.js | 3 + test/function/unused-import/_config.js | 2 + test/function/warn-on-eval/_config.js | 3 + .../warn-on-namespace-conflict/_config.js | 8 + .../warn-on-top-level-this/_config.js | 2 + .../warn-on-unused-missing-imports/_config.js | 3 + test/test.js | 115 ++++++---- 29 files changed, 529 insertions(+), 530 deletions(-) rename .eslintrc => .eslintrc.json (95%) delete mode 100644 bin/src/run/createWatcher.js diff --git a/.eslintrc b/.eslintrc.json similarity index 95% rename from .eslintrc rename to .eslintrc.json index d9150f9de08..e35395ba7e3 100644 --- a/.eslintrc +++ b/.eslintrc.json @@ -5,7 +5,6 @@ "semi": [ 2, "always" ], "keyword-spacing": [ 2, { "before": true, "after": true } ], "space-before-blocks": [ 2, "always" ], - "space-before-function-paren": [ 2, "always" ], "no-mixed-spaces-and-tabs": [ 2, "smart-tabs" ], "no-cond-assign": 0, "no-unused-vars": 2, diff --git a/bin/src/logging.js b/bin/src/logging.js index 69d336e0a20..0207c137732 100644 --- a/bin/src/logging.js +++ b/bin/src/logging.js @@ -2,42 +2,35 @@ import chalk from 'chalk'; import relativeId from '../../src/utils/relativeId.js'; if ( !process.stderr.isTTY ) chalk.enabled = false; -const warnSymbol = process.stderr.isTTY ? `⚠️ ` : `Warning: `; -const errorSymbol = process.stderr.isTTY ? `🚨 ` : `Error: `; // log to stderr to keep `rollup main.js > bundle.js` from breaking export const stderr = console.error.bind( console ); // eslint-disable-line no-console -function log ( object, symbol ) { - let description = object.message || object; - if (object.name) description = object.name + ': ' + description; - const message = (object.plugin ? `(${object.plugin} plugin) ${description}` : description) || object;; +export function handleError ( err, recover ) { + let description = err.message || err; + if (err.name) description = `${err.name}: ${description}`; + const message = (err.plugin ? `(${err.plugin} plugin) ${description}` : description) || err; - stderr( `${symbol}${chalk.bold( message )}` ); + stderr( chalk.bold.red( `[!] ${chalk.bold( message )}` ) ); - // TODO should this be "object.url || (object.file && object.loc.file) || object.id"? - if ( object.url ) { - stderr( chalk.cyan( object.url ) ); + // TODO should this be "err.url || (err.file && err.loc.file) || err.id"? + if ( err.url ) { + stderr( chalk.cyan( err.url ) ); } - if ( object.loc ) { - stderr( `${relativeId( object.loc.file || object.id )} (${object.loc.line}:${object.loc.column})` ); - } else if ( object.id ) { - stderr( relativeId( object.id ) ); + if ( err.loc ) { + stderr( `${relativeId( err.loc.file || err.id )} (${err.loc.line}:${err.loc.column})` ); + } else if ( err.id ) { + stderr( relativeId( err.id ) ); } - if ( object.frame ) { - stderr( chalk.dim( object.frame ) ); + if ( err.frame ) { + stderr( chalk.dim( err.frame ) ); + } else if ( err.stack ) { + stderr( chalk.dim( err.stack ) ); } stderr( '' ); -} -export function handleWarning ( warning ) { - log( warning, warnSymbol ); -} - -export function handleError ( err, recover ) { - log( err, errorSymbol ); if ( !recover ) process.exit( 1 ); } diff --git a/bin/src/run/batchWarnings.js b/bin/src/run/batchWarnings.js index 8ed81e5dada..1fbb69d9d5a 100644 --- a/bin/src/run/batchWarnings.js +++ b/bin/src/run/batchWarnings.js @@ -1,5 +1,5 @@ import chalk from 'chalk'; -import { handleWarning, stderr } from '../logging.js'; +import { stderr } from '../logging.js'; import relativeId from '../../../src/utils/relativeId.js'; export default function batchWarnings () { @@ -7,11 +7,20 @@ export default function batchWarnings () { let count = 0; return { + get count() { + return count; + }, + add: warning => { if ( typeof warning === 'string' ) { warning = { code: 'UNKNOWN', message: warning }; } + if ( warning.code in immediateHandlers ) { + immediateHandlers[ warning.code ]( warning ); + return; + } + if ( !allWarnings.has( warning.code ) ) allWarnings.set( warning.code, [] ); allWarnings.get( warning.code ).push( warning ); @@ -23,24 +32,37 @@ export default function batchWarnings () { const codes = Array.from( allWarnings.keys() ) .sort( ( a, b ) => { - if ( handlers[a] && handlers[b] ) { - return handlers[a].priority - handlers[b].priority; + if ( deferredHandlers[a] && deferredHandlers[b] ) { + return deferredHandlers[a].priority - deferredHandlers[b].priority; } - if ( handlers[a] ) return -1; - if ( handlers[b] ) return 1; + if ( deferredHandlers[a] ) return -1; + if ( deferredHandlers[b] ) return 1; return allWarnings.get( b ).length - allWarnings.get( a ).length; }); codes.forEach( code => { - const handler = handlers[ code ]; + const handler = deferredHandlers[ code ]; const warnings = allWarnings.get( code ); if ( handler ) { handler.fn( warnings ); } else { warnings.forEach( warning => { - handleWarning( warning ); + stderr( `${chalk.bold.yellow('(!)')} ${chalk.bold.yellow( warning.message )}` ); + + if ( warning.url ) info( warning.url ); + + const id = warning.loc && warning.loc.file || warning.id; + if ( id ) { + const loc = warning.loc ? + `${relativeId( id )}: (${warning.loc.line}:${warning.loc.column})` : + relativeId( id ); + + stderr( chalk.bold( relativeId( loc ) ) ); + } + + if ( warning.frame ) info( warning.frame ); }); } }); @@ -50,14 +72,34 @@ export default function batchWarnings () { }; } +const immediateHandlers = { + MISSING_NODE_BUILTINS: warning => { + title( `Missing shims for Node.js built-ins` ); + + const detail = warning.modules.length === 1 ? + `'${warning.modules[0]}'` : + `${warning.modules.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${warning.modules.slice( -1 )}'`; + stderr( `Creating a browser bundle that depends on ${detail}. You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins` ); + }, + + MIXED_EXPORTS: () => { + title( 'Mixing named and default exports' ); + stderr( `Consumers of your bundle will have to use bundle['default'] to access the default export, which may not be what you want. Use \`exports: 'named'\` to disable this warning` ); + }, + + EMPTY_BUNDLE: () => { + title( `Generated an empty bundle` ); + } +}; + // TODO select sensible priorities -const handlers = { +const deferredHandlers = { UNUSED_EXTERNAL_IMPORT: { priority: 1, fn: warnings => { - group( 'Unused external imports' ); + title( 'Unused external imports' ); warnings.forEach( warning => { - stderr( `${warning.message}` ); + stderr( `${warning.names} imported from external module '${warning.source}' but never used` ); }); } }, @@ -65,7 +107,7 @@ const handlers = { UNRESOLVED_IMPORT: { priority: 1, fn: warnings => { - group( 'Unresolved dependencies' ); + title( 'Unresolved dependencies' ); info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency' ); const dependencies = new Map(); @@ -84,7 +126,7 @@ const handlers = { MISSING_EXPORT: { priority: 1, fn: warnings => { - group( 'Missing exports' ); + title( 'Missing exports' ); info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module' ); warnings.forEach( warning => { @@ -98,110 +140,136 @@ const handlers = { THIS_IS_UNDEFINED: { priority: 1, fn: warnings => { - group( '`this` has been rewritten to `undefined`' ); + title( '`this` has been rewritten to `undefined`' ); info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#this-is-undefined' ); - - const modules = new Map(); - warnings.forEach( warning => { - if ( !modules.has( warning.loc.file ) ) modules.set( warning.loc.file, [] ); - modules.get( warning.loc.file ).push( warning ); - }); - - const allIds = Array.from( modules.keys() ); - const ids = allIds.length > 5 ? allIds.slice( 0, 3 ) : allIds; - - ids.forEach( id => { - const occurrences = modules.get( id ); - - stderr( chalk.bold( relativeId( id ) ) ); - stderr( chalk.grey( occurrences[0].frame ) ); - - if ( occurrences.length > 1 ) { - stderr( `...and ${occurrences.length - 1} other ${occurrences.length > 2 ? 'occurrences' : 'occurrence'}` ); - } - }); - - if ( allIds.length > ids.length ) { - stderr( `\n...and ${allIds.length - ids.length} other files` ); - } + showTruncatedWarnings(warnings); } }, EVAL: { priority: 1, fn: warnings => { - + title( 'Use of eval is strongly discouraged' ); + info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#avoiding-eval' ); + showTruncatedWarnings(warnings); } }, NON_EXISTENT_EXPORT: { priority: 1, fn: warnings => { - + title( `Import of non-existent ${warnings.length > 1 ? 'exports' : 'export'}` ); + showTruncatedWarnings(warnings); } }, NAMESPACE_CONFLICT: { priority: 1, fn: warnings => { - + title( `Conflicting re-exports` ); + warnings.forEach(warning => { + stderr( `${chalk.bold(relativeId(warning.reexporter))} re-exports '${warning.name}' from both ${relativeId(warning.sources[0])} and ${relativeId(warning.sources[1])} (will be ignored)` ); + }); } }, - DEPRECATED_ES6: { + MISSING_GLOBAL_NAME: { priority: 1, fn: warnings => { - + title( `Missing global variable ${warnings.length > 1 ? 'names' : 'name'}` ); + stderr( `Use options.globals to specify browser global variable names corresponding to external modules` ); + warnings.forEach(warning => { + stderr(`${chalk.bold(warning.source)} (guessing '${warning.guess}')`); + }); } }, - EMPTY_BUNDLE: { + SOURCEMAP_BROKEN: { priority: 1, fn: warnings => { - - } - }, + title( `Broken sourcemap` ); + info( 'https://github.com/rollup/rollup/wiki/Troubleshooting#sourcemap-is-likely-to-be-incorrect' ); - MISSING_GLOBAL_NAME: { - priority: 1, - fn: warnings => { - - } - }, + const plugins = Array.from( new Set( warnings.map( w => w.plugin ).filter( Boolean ) ) ); + const detail = plugins.length === 0 ? '' : plugins.length > 1 ? + ` (such as ${plugins.slice(0, -1).map(p => `'${p}'`).join(', ')} and '${plugins.slice(-1)}')` : + ` (such as '${plugins[0]}')`; - MISSING_NODE_BUILTINS: { - priority: 1, - fn: warnings => { - + stderr( `Plugins that transform code${detail} should generate accompanying sourcemaps` ); } }, - MISSING_FORMAT: { + PLUGIN_WARNING: { priority: 1, fn: warnings => { - - } - }, // TODO make this an error + const nestedByPlugin = nest(warnings, 'plugin'); - SOURCEMAP_BROKEN: { - priority: 1, - fn: warnings => { - - } - }, + nestedByPlugin.forEach(({ key: plugin, items }) => { + const nestedByMessage = nest(items, 'message'); - MIXED_EXPORTS: { - priority: 1, - fn: warnings => { - + let lastUrl; + + nestedByMessage.forEach(({ key: message, items }) => { + title( `${plugin} plugin: ${message}` ); + items.forEach(warning => { + if ( warning.url !== lastUrl ) info( lastUrl = warning.url ); + + const loc = warning.loc ? + `${relativeId( warning.id )}: (${warning.loc.line}:${warning.loc.column})` : + relativeId( warning.id ); + + stderr( chalk.bold( relativeId( loc ) ) ); + if ( warning.frame ) info( warning.frame ); + }); + }); + }); } } }; -function group ( title ) { - stderr( `${chalk.bold.yellow('(!)')} ${chalk.bold.yellow( title )}` ); +function title ( str ) { + stderr( `${chalk.bold.yellow('(!)')} ${chalk.bold.yellow( str )}` ); } function info ( url ) { stderr( chalk.grey( url ) ); +} + +function nest(array, prop) { + const nested = []; + const lookup = new Map(); + + array.forEach(item => { + const key = item[prop]; + if (!lookup.has(key)) { + lookup.set(key, { + key, + items: [] + }); + + nested.push(lookup.get(key)); + } + + lookup.get(key).items.push(item); + }); + + return nested; +} + +function showTruncatedWarnings(warnings) { + const nestedByModule = nest(warnings, 'id'); + + const sliced = nestedByModule.length > 5 ? nestedByModule.slice(0, 3) : nestedByModule; + sliced.forEach(({ key: id, items }) => { + stderr( chalk.bold( relativeId( id ) ) ); + stderr( chalk.grey( items[0].frame ) ); + + if ( items.length > 1 ) { + stderr( `...and ${items.length - 1} other ${items.length > 2 ? 'occurrences' : 'occurrence'}` ); + } + }); + + if ( nestedByModule.length > sliced.length ) { + stderr( `\n...and ${nestedByModule.length - sliced.length} other files` ); + } } \ No newline at end of file diff --git a/bin/src/run/build.js b/bin/src/run/build.js index 055bdae386b..0ef94446991 100644 --- a/bin/src/run/build.js +++ b/bin/src/run/build.js @@ -1,47 +1,45 @@ import * as rollup from 'rollup'; import chalk from 'chalk'; +import ms from 'pretty-ms'; import { handleError, stderr } from '../logging.js'; +import relativeId from '../../../src/utils/relativeId.js'; +import { mapSequence } from '../../../src/utils/promise.js'; import SOURCEMAPPING_URL from '../sourceMappingUrl.js'; -export default function build ( options, warnings ) { +export default function build ( options, warnings, silent ) { + const useStdout = !options.targets && !options.dest; + const targets = options.targets ? options.targets : [{ dest: options.dest, format: options.format }]; + const start = Date.now(); - stderr( chalk.green( `\n${chalk.bold( options.entry )} → ${chalk.bold( options.dest )}...` ) ); + const dests = useStdout ? [ 'stdout' ] : targets.map( t => relativeId( t.dest ) ); + if ( !silent ) stderr( chalk.cyan( `\n${chalk.bold( options.entry )} → ${chalk.bold( dests.join( ', ' ) )}...` ) ); return rollup.rollup( options ) .then( bundle => { - if ( options.dest ) { - return bundle.write( options ); - } - - if ( options.targets ) { - let result = null; - - options.targets.forEach( target => { - result = bundle.write( assign( clone( options ), target ) ); - }); + if ( useStdout ) { + if ( options.sourceMap && options.sourceMap !== 'inline' ) { + handleError({ + code: 'MISSING_OUTPUT_OPTION', + message: 'You must specify an --output (-o) option when creating a file with a sourcemap' + }); + } - return result; - } + return bundle.generate(options).then( ({ code, map }) => { + if ( options.sourceMap === 'inline' ) { + code += `\n//# ${SOURCEMAPPING_URL}=${map.toUrl()}\n`; + } - if ( options.sourceMap && options.sourceMap !== 'inline' ) { - handleError({ - code: 'MISSING_OUTPUT_OPTION', - message: 'You must specify an --output (-o) option when creating a file with a sourcemap' + process.stdout.write( code ); }); } - return bundle.generate(options).then( ({ code, map }) => { - if ( options.sourceMap === 'inline' ) { - code += `\n//# ${SOURCEMAPPING_URL}=${map.toUrl()}\n`; - } - - process.stdout.write( code ); + return mapSequence( targets, target => { + return bundle.write( assign( clone( options ), target ) ); }); }) .then( () => { warnings.flush(); - stderr( chalk.green( `${chalk.bold( options.dest )} created in ${Date.now() - start}ms\n` ) ); - // stderr( `${chalk.blue( '----------' )}\n` ); + if ( !silent ) stderr( chalk.green( `created ${chalk.bold( dests.join( ', ' ) )} in ${chalk.bold(ms( Date.now() - start))}` ) ); }) .catch( handleError ); } diff --git a/bin/src/run/createWatcher.js b/bin/src/run/createWatcher.js deleted file mode 100644 index 4ec02fa5ff0..00000000000 --- a/bin/src/run/createWatcher.js +++ /dev/null @@ -1,217 +0,0 @@ -import EventEmitter from 'events'; -import relative from 'require-relative'; -import path from 'path'; -import * as fs from 'fs'; -import createFilter from 'rollup-pluginutils/src/createFilter.js'; -import sequence from '../utils/sequence.js'; - -const opts = { encoding: 'utf-8', persistent: true }; - -let chokidar; - -try { - chokidar = relative( 'chokidar', process.cwd() ); -} catch (err) { - chokidar = null; -} - -class FileWatcher { - constructor ( file, data, callback, chokidarOptions, dispose ) { - const handleWatchEvent = (event) => { - if ( event === 'rename' || event === 'unlink' ) { - this.fsWatcher.close(); - dispose(); - callback(); - } else { - // this is necessary because we get duplicate events... - const contents = fs.readFileSync( file, 'utf-8' ); - if ( contents !== data ) { - data = contents; - callback(); - } - } - }; - - try { - if (chokidarOptions) { - this.fsWatcher = chokidar.watch(file, chokidarOptions).on('all', handleWatchEvent); - } else { - this.fsWatcher = fs.watch( file, opts, handleWatchEvent); - } - - this.fileExists = true; - } catch ( err ) { - if ( err.code === 'ENOENT' ) { - // can't watch files that don't exist (e.g. injected - // by plugins somehow) - this.fileExists = false; - } else { - throw err; - } - } - } - - close () { - this.fsWatcher.close(); - } -} - -export default function watch ( rollup, options ) { - const watchOptions = options.watch || {}; - - if ( 'useChokidar' in watchOptions ) watchOptions.chokidar = watchOptions.useChokidar; - let chokidarOptions = 'chokidar' in watchOptions ? watchOptions.chokidar : !!chokidar; - if ( chokidarOptions ) { - chokidarOptions = Object.assign( chokidarOptions === true ? {} : chokidarOptions, { - ignoreInitial: true - }); - } - - if ( chokidarOptions && !chokidar ) { - throw new Error( `options.watch.chokidar was provided, but chokidar could not be found. Have you installed it?` ); - } - - const watcher = new EventEmitter(); - - const filter = createFilter( watchOptions.include, watchOptions.exclude ); - const dests = options.dest ? [ path.resolve( options.dest ) ] : options.targets.map( target => path.resolve( target.dest ) ); - let filewatchers = new Map(); - - let rebuildScheduled = false; - let building = false; - let watching = false; - let closed = false; - - let timeout; - let cache; - - function triggerRebuild () { - clearTimeout( timeout ); - rebuildScheduled = true; - - timeout = setTimeout( () => { - if ( !building ) build(); - }, 50 ); - } - - function addFileWatchersForModules ( modules ) { - modules.forEach( module => { - let id = module.id; - - // skip plugin helper modules and unwatched files - if ( /\0/.test( id ) ) return; - if ( !filter( id ) ) return; - - try { - id = fs.realpathSync( id ); - } catch ( err ) { - return; - } - - if ( ~dests.indexOf( id ) ) { - throw new Error( 'Cannot import the generated bundle' ); - } - - if ( !filewatchers.has( id ) ) { - const watcher = new FileWatcher( id, module.originalCode, triggerRebuild, chokidarOptions, () => { - filewatchers.delete( id ); - }); - - if ( watcher.fileExists ) filewatchers.set( id, watcher ); - } - }); - } - - function build () { - if ( building || closed ) return; - - rebuildScheduled = false; - - let start = Date.now(); - let initial = !watching; - if ( cache ) options.cache = cache; - - watcher.emit( 'event', { code: 'BUILD_START' }); - - building = true; - - return rollup.rollup( options ) - .then( bundle => { - // Save off bundle for re-use later - cache = bundle; - - if ( !closed ) { - addFileWatchersForModules(bundle.modules); - } - - // Now we're watching - watching = true; - - if ( options.targets ) { - return sequence( options.targets, target => { - const mergedOptions = Object.assign( {}, options, target ); - return bundle.write( mergedOptions ); - }); - } - - return bundle.write( options ); - }) - .then( () => { - watcher.emit( 'event', { - code: 'BUILD_END', - duration: Date.now() - start, - initial - }); - }, error => { - try { - //If build failed, make sure we are still watching those files from the most recent successful build. - addFileWatchersForModules( cache.modules ); - } - catch (e) { - //Ignore if they tried to import the output. We are already inside of a catch (probably caused by that). - } - watcher.emit( 'event', { - code: 'ERROR', - error - }); - }) - .then( () => { - building = false; - if ( rebuildScheduled && !closed ) build(); - }); - } - - // build on next tick, so consumers can listen for BUILD_START - process.nextTick( build ); - - function close () { - if ( closed ) return; - for ( const fw of filewatchers.values() ) { - fw.close(); - } - - process.removeListener('SIGINT', close); - process.removeListener('SIGTERM', close); - process.removeListener('uncaughtException', close); - process.stdin.removeListener('end', close); - - watcher.removeAllListeners(); - closed = true; - } - - watcher.close = close; - - // ctrl-c - process.on('SIGINT', close); - - // killall node - process.on('SIGTERM', close); - - // on error - process.on('uncaughtException', close); - - // in case we ever support stdin! - process.stdin.on('end', close); - - return watcher; -} diff --git a/bin/src/run/index.js b/bin/src/run/index.js index 06ac892c806..5091e9fcef2 100644 --- a/bin/src/run/index.js +++ b/bin/src/run/index.js @@ -1,10 +1,12 @@ import path from 'path'; +import chalk from 'chalk'; import { realpathSync } from 'fs'; import * as rollup from 'rollup'; import relative from 'require-relative'; -import { handleWarning, handleError } from '../logging.js'; +import { handleError, stderr } from '../logging.js'; import mergeOptions from './mergeOptions.js'; import batchWarnings from './batchWarnings.js'; +import relativeId from '../../../src/utils/relativeId.js'; import sequence from '../utils/sequence.js'; import build from './build.js'; import watch from './watch.js'; @@ -78,7 +80,10 @@ export default function runRollup ( command ) { onwarn: warnings.add }) .then( bundle => { - warnings.flush(); + if ( !command.silent && warnings.count > 0 ) { + stderr( chalk.bold( `loaded ${relativeId( config )} with warnings` ) ); + warnings.flush(); + } return bundle.generate({ format: 'cjs' @@ -118,11 +123,23 @@ export default function runRollup ( command ) { function execute ( configs, command ) { if ( command.watch ) { process.env.ROLLUP_WATCH = 'true'; - watch( configs, command ); + watch( configs, command, command.silent ); } else { - return sequence( config => { - const { options, warnings } = mergeOptions( config, command ); - return build( options, warnings ); + return sequence( configs, config => { + const options = mergeOptions( config, command ); + + const warnings = batchWarnings(); + + const onwarn = options.onwarn; + if ( onwarn ) { + options.onwarn = warning => { + onwarn( warning, warnings.add ); + }; + } else { + options.onwarn = warnings.add; + } + + return build( options, warnings, command.silent ); }); } } \ No newline at end of file diff --git a/bin/src/run/mergeOptions.js b/bin/src/run/mergeOptions.js index c56e08e98f8..8b12514258f 100644 --- a/bin/src/run/mergeOptions.js +++ b/bin/src/run/mergeOptions.js @@ -54,21 +54,8 @@ export default function mergeOptions ( config, command ) { options.extend = command.extend; } - const warnings = batchWarnings(); - - if ( command.silent ) { + if (command.silent) { options.onwarn = () => {}; - } else { - options.onwarn = warnings.add; - - const onwarn = options.onwarn; - if ( onwarn ) { - options.onwarn = warning => { - onwarn( warning, warnings.add ); - }; - } else { - options.onwarn = warnings.add; - } } options.external = external; @@ -80,5 +67,9 @@ export default function mergeOptions ( config, command ) { } }); - return { options, warnings }; + const targets = options.dest ? [{ dest: options.dest, format: options.format }] : options.targets; + options.targets = targets; + delete options.dest; + + return options; } \ No newline at end of file diff --git a/bin/src/run/watch.js b/bin/src/run/watch.js index 9bd5ff90f33..751e98c83ab 100644 --- a/bin/src/run/watch.js +++ b/bin/src/run/watch.js @@ -1,44 +1,74 @@ import * as rollup from 'rollup'; import chalk from 'chalk'; -import createWatcher from './createWatcher.js'; +import ms from 'pretty-ms'; import mergeOptions from './mergeOptions.js'; +import batchWarnings from './batchWarnings.js'; +import relativeId from '../../../src/utils/relativeId.js'; import { handleError, stderr } from '../logging.js'; -export default function watch ( configs, command ) { - configs.forEach( config => { - const { options, warnings } = mergeOptions( config, command ); +export default function watch(configs, command, silent) { + process.stderr.write('\x1b[?1049h'); // alternate screen buffer - if ( !options.entry || ( !options.dest && !options.targets ) ) { - handleError({ - code: 'WATCHER_MISSING_INPUT_OR_OUTPUT', - message: 'must specify --input and --output when using rollup --watch' - }); + const warnings = batchWarnings(); + + configs = configs.map(options => { + const merged = mergeOptions(options, command); + + const onwarn = merged.onwarn; + if ( onwarn ) { + merged.onwarn = warning => { + onwarn( warning, warnings.add ); + }; + } else { + merged.onwarn = warnings.add; } - const watcher = createWatcher( rollup, options ); + return merged; + }); + + const watcher = rollup.watch(configs); - watcher.on( 'event', event => { - switch ( event.code ) { - case 'STARTING': // TODO this isn't emitted by newer versions of rollup-watch - stderr( 'checking rollup-watch version...' ); - break; + watcher.on('event', event => { + switch (event.code) { + case 'FATAL': + process.stderr.write('\x1b[?1049l'); // reset screen buffer + handleError(event.error, true); + process.exit(1); + break; - case 'BUILD_START': - stderr( `${chalk.blue.bold( options.entry )} -> ${chalk.blue.bold( options.dest )}...` ); - break; + case 'ERROR': + warnings.flush(); + handleError(event.error, true); + break; - case 'BUILD_END': - warnings.flush(); - stderr( `created ${chalk.blue.bold( options.dest )} in ${event.duration}ms. Watching for changes...` ); - break; + case 'START': + stderr(`\x1B[2J\x1B[0f${chalk.underline( 'rollup.watch' )}`); // clear, move to top-left + break; - case 'ERROR': - handleError( event.error, true ); - break; + case 'BUNDLE_START': + if ( !silent ) stderr( chalk.cyan( `\n${chalk.bold( event.input )} → ${chalk.bold( event.output.map( relativeId ).join( ', ' ) )}...` ) ); + break; - default: - stderr( 'unknown event', event ); - } - }); + case 'BUNDLE_END': + warnings.flush(); + if ( !silent ) stderr( chalk.green( `created ${chalk.bold( event.output.map( relativeId ).join( ', ' ) )} in ${chalk.bold(ms(event.duration))}` ) ); + break; + + case 'END': + if ( !silent ) stderr( `\nwaiting for changes...` ); + } }); + + let closed = false; + const close = () => { + if (!closed) { + process.stderr.write('\x1b[?1049l'); // reset screen buffer + closed = true; + watcher.close(); + } + }; + process.on('SIGINT', close); // ctrl-c + process.on('SIGTERM', close); // killall node + process.on('uncaughtException', close); // on error + process.stdin.on('end', close); // in case we ever support stdin! } \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 500e3f2c595..20a9568c5fc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2439,6 +2439,12 @@ "integrity": "sha1-y8NcYu7uc/Gat7EKgBURQBr8D5A=", "dev": true }, + "irregular-plurals": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/irregular-plurals/-/irregular-plurals-1.3.0.tgz", + "integrity": "sha512-njf5A+Mxb3kojuHd1DzISjjIl+XhyzovXEOyPPSzdQozq/Lf2tN27mOrAAsxEPZxpn6I4MGzs1oo9TxXxPFpaA==", + "dev": true + }, "is-arrayish": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/is-arrayish/-/is-arrayish-0.2.1.tgz", @@ -3399,6 +3405,12 @@ "error-ex": "1.3.1" } }, + "parse-ms": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/parse-ms/-/parse-ms-1.0.1.tgz", + "integrity": "sha1-VjRtR0nXjyNDDKDHE4UK75GqNh0=", + "dev": true + }, "path-exists": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-2.1.0.tgz", @@ -3465,6 +3477,15 @@ "find-up": "1.1.2" } }, + "plur": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/plur/-/plur-2.1.2.tgz", + "integrity": "sha1-dIJFLBoPUI4+NE6uwxLJHCncZVo=", + "dev": true, + "requires": { + "irregular-plurals": "1.3.0" + } + }, "pluralize": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/pluralize/-/pluralize-1.2.1.tgz", @@ -3483,6 +3504,16 @@ "integrity": "sha1-gV7R9uvGWSb4ZbMQwHE7yzMVzks=", "dev": true }, + "pretty-ms": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/pretty-ms/-/pretty-ms-3.0.0.tgz", + "integrity": "sha1-8cwgKLZvX6c2rCPvV3A9fmhKsQE=", + "dev": true, + "requires": { + "parse-ms": "1.0.1", + "plur": "2.1.2" + } + }, "process-nextick-args": { "version": "1.0.7", "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-1.0.7.tgz", diff --git a/package.json b/package.json index c797aa6346c..e22c47bcaff 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "magic-string": "^0.21.3", "minimist": "^1.2.0", "mocha": "^3.0.0", + "pretty-ms": "^3.0.0", "remap-istanbul": "^0.9.5", "require-relative": "^0.8.7", "rollup": "^0.42.0", diff --git a/rollup.config.js b/rollup.config.js index cc0b9ba4a65..9ae4776bdd6 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -1,6 +1,7 @@ import { readFileSync } from 'fs'; import buble from 'rollup-plugin-buble'; import nodeResolve from 'rollup-plugin-node-resolve'; +import commonjs from 'rollup-plugin-commonjs'; import replace from 'rollup-plugin-replace'; var pkg = JSON.parse( readFileSync( 'package.json', 'utf-8' ) ); @@ -36,8 +37,10 @@ export default { include: 'src/rollup.js', delimiters: [ '<@', '@>' ], sourceMap: true, - values: { 'VERSION': pkg.version } - }) + values: { VERSION: pkg.version } + }), + + commonjs() ], external: [ 'fs', diff --git a/src/Bundle.js b/src/Bundle.js index d6e9ace3b04..7cb6a5784c6 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -217,10 +217,12 @@ export default class Bundle { const names = unused.length === 1 ? `'${unused[0]}' is` : - `${unused.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${unused.pop()}' are`; + `${unused.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${unused.slice( -1 )}' are`; this.warn({ code: 'UNUSED_EXTERNAL_IMPORT', + source: module.id, + names: unused, message: `${names} imported from external module '${module.id}' but never used` }); }); @@ -356,6 +358,9 @@ export default class Bundle { if ( name in module.exportsAll ) { this.warn({ code: 'NAMESPACE_CONFLICT', + reexporter: module.id, + name, + sources: [ module.exportsAll[ name ], exportAllModule.exportsAll[ name ] ], message: `Conflicting namespaces: ${relativeId( module.id )} re-exports '${name}' from both ${relativeId( module.exportsAll[ name ] )} and ${relativeId( exportAllModule.exportsAll[ name ] )} (will be ignored)` }); } else { @@ -449,15 +454,6 @@ export default class Bundle { render ( options = {} ) { return Promise.resolve().then( () => { - if ( options.format === 'es6' ) { - this.warn({ - code: 'DEPRECATED_ES6', - message: 'The es6 format is deprecated – use `es` instead' - }); - - options.format = 'es'; - } - // Determine export mode - 'default', 'named', 'none' const exportMode = getExportMode( this, options ); diff --git a/src/Module.js b/src/Module.js index 9ba3f205dc3..72a88aa50ee 100644 --- a/src/Module.js +++ b/src/Module.js @@ -456,6 +456,7 @@ export default class Module { warning.frame = getCodeFrame( this.code, line, column ); } + warning.id = this.id; this.bundle.warn( warning ); } } diff --git a/src/ast/scopes/ModuleScope.js b/src/ast/scopes/ModuleScope.js index 9a1f9ee75a7..1da09aae483 100644 --- a/src/ast/scopes/ModuleScope.js +++ b/src/ast/scopes/ModuleScope.js @@ -29,6 +29,8 @@ export default class ModuleScope extends Scope { if ( !declaration ) { this.module.warn({ code: 'NON_EXISTENT_EXPORT', + name: specifier.name, + source: specifier.module.id, message: `Non-existent export '${specifier.name}' is imported from ${relativeId( specifier.module.id )}` }, specifier.specifier.start ); return; diff --git a/src/finalisers/shared/getGlobalNameMaker.js b/src/finalisers/shared/getGlobalNameMaker.js index ee3475653ed..aa208f5b745 100644 --- a/src/finalisers/shared/getGlobalNameMaker.js +++ b/src/finalisers/shared/getGlobalNameMaker.js @@ -8,6 +8,8 @@ export default function getGlobalNameMaker ( globals, bundle, fallback = null ) if ( Object.keys( module.declarations ).length > 0 ) { bundle.warn({ code: 'MISSING_GLOBAL_NAME', + source: module.id, + guess: module.name, message: `No name was provided for external module '${module.id}' in options.globals – guessing '${module.name}'` }); diff --git a/src/finalisers/shared/warnOnBuiltins.js b/src/finalisers/shared/warnOnBuiltins.js index c4d63f6f4de..f4aa878cc04 100644 --- a/src/finalisers/shared/warnOnBuiltins.js +++ b/src/finalisers/shared/warnOnBuiltins.js @@ -33,10 +33,11 @@ export default function warnOnBuiltins ( bundle ) { const detail = externalBuiltins.length === 1 ? `module ('${externalBuiltins[0]}')` : - `modules (${externalBuiltins.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${externalBuiltins.pop()}')`; + `modules (${externalBuiltins.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${externalBuiltins.slice( -1 )}')`; bundle.warn({ code: 'MISSING_NODE_BUILTINS', + modules: externalBuiltins, message: `Creating a browser bundle that depends on Node.js built-in ${detail}. You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins` }); } diff --git a/src/rollup/index.js b/src/rollup/index.js index 7213c1ea1f8..edc868d8fad 100644 --- a/src/rollup/index.js +++ b/src/rollup/index.js @@ -93,10 +93,14 @@ export default function rollup ( options ) { timeEnd( '--BUILD--' ); function generate ( options = {} ) { + if ( options.format === 'es6' ) { + throw new Error( 'The `es6` output format is deprecated – use `es` instead' ); + } + if ( !options.format ) { - bundle.warn({ // TODO make this an error + error({ // TODO make this an error code: 'MISSING_FORMAT', - message: `No format option was supplied – defaulting to 'es'`, + message: `You must supply an output format`, url: `https://github.com/rollup/rollup/wiki/JavaScript-API#format` }); diff --git a/src/utils/collapseSourcemaps.js b/src/utils/collapseSourcemaps.js index b77ef9def6a..db4cebb1c06 100644 --- a/src/utils/collapseSourcemaps.js +++ b/src/utils/collapseSourcemaps.js @@ -135,6 +135,7 @@ export default function collapseSourcemaps ( bundle, file, map, modules, bundleS if ( map.missing ) { bundle.warn({ code: 'SOURCEMAP_BROKEN', + plugin: map.plugin, message: `Sourcemap is likely to be incorrect: a plugin${map.plugin ? ` ('${map.plugin}')` : ``} was used to transform files, but didn't generate a sourcemap for the transformation. Consult the plugin documentation for help`, url: `https://github.com/rollup/rollup/wiki/Troubleshooting#sourcemap-is-likely-to-be-incorrect` }); diff --git a/src/utils/error.js b/src/utils/error.js index c7d46d07fe3..e5cd9768b25 100644 --- a/src/utils/error.js +++ b/src/utils/error.js @@ -4,8 +4,7 @@ export default function error ( props ) { // (Object.keys below does not update these values because they // are properties on the prototype chain) // basically if props is a SyntaxError it will not be overriden as a generic Error - let constructor = Error; - if (props instanceof Error) constructor = props.constructor; + const constructor = (props instanceof Error) ? props.constructor : Error; const err = new constructor( props.message ); Object.keys( props ).forEach( key => { diff --git a/src/utils/transform.js b/src/utils/transform.js index 0c0d9de5a71..b22acdec3a8 100644 --- a/src/utils/transform.js +++ b/src/utils/transform.js @@ -26,7 +26,8 @@ export default function transform ( bundle, source, id, plugins ) { object = { message: object }; } - if ( !object.code ) object.code = code; + if ( object.code ) object.pluginCode = object.code; + object.code = code; if ( pos !== undefined ) { if ( pos.line !== undefined && pos.column !== undefined ) { @@ -42,21 +43,24 @@ export default function transform ( bundle, source, id, plugins ) { } } + object.plugin = plugin.name; + object.id = id; + return object; } - let err; + let throwing; const context = { warn: ( warning, pos ) => { warning = augment( warning, pos, 'PLUGIN_WARNING' ); - warning.plugin = plugin.name; - warning.id = id; bundle.warn( warning ); }, - error ( e, pos ) { - err = augment( e, pos, 'PLUGIN_ERROR' ); + error ( err, pos ) { + err = augment( err, pos, 'PLUGIN_ERROR' ); + throwing = true; + error( err ); } }; @@ -65,13 +69,12 @@ export default function transform ( bundle, source, id, plugins ) { try { transformed = plugin.transform.call( context, previous, id ); } catch ( err ) { - context.error( err ); + if ( !throwing ) context.error( err ); + error( err ); } return Promise.resolve( transformed ) .then( result => { - if ( err ) throw err; - if ( result == null ) return previous; if ( typeof result === 'string' ) { @@ -97,8 +100,7 @@ export default function transform ( bundle, source, id, plugins ) { return result.code; }) .catch( err => { - err.plugin = plugin.name; - err.id = id; + err = augment( err, undefined, 'PLUGIN_ERROR' ); error( err ); }); }); diff --git a/src/watch/fileWatchers.js b/src/watch/fileWatchers.js index 0070f6572af..910bdaf7ffe 100644 --- a/src/watch/fileWatchers.js +++ b/src/watch/fileWatchers.js @@ -6,29 +6,34 @@ const opts = { encoding: 'utf-8', persistent: true }; const watchers = new Map(); -export function addTask(id, task) { - if (!watchers.has(id)) { - const watcher = new FileWatcher(id, () => { - watchers.delete(id); +export function addTask(id, task, chokidarOptions, chokidarOptionsHash) { + if (!watchers.has(chokidarOptionsHash)) watchers.set(chokidarOptionsHash, new Map()); + const group = watchers.get(chokidarOptionsHash); + + if (!group.has(id)) { + const watcher = new FileWatcher(id, chokidarOptions, () => { + group.delete(id); }); if (watcher.fileExists) { - watchers.set(id, watcher); + group.set(id, watcher); } else { return; } } - watchers.get(id).tasks.add(task); + group.get(id).tasks.add(task); } -export function deleteTask(id, target) { - const watcher = watchers.get(id); +export function deleteTask(id, target, chokidarOptionsHash) { + const group = watchers.get(chokidarOptionsHash); + + const watcher = group.get(id); watcher.tasks.delete(target); if (watcher.tasks.size === 0) { watcher.close(); - watchers.delete(id); + group.delete(id); } } diff --git a/src/watch/index.js b/src/watch/index.js index 52cb823e61a..58975948bc6 100644 --- a/src/watch/index.js +++ b/src/watch/index.js @@ -1,9 +1,11 @@ import path from 'path'; import EventEmitter from 'events'; +import createFilter from 'rollup-pluginutils/src/createFilter.js'; import rollup from '../rollup/index.js'; import ensureArray from '../utils/ensureArray.js'; import { mapSequence } from '../utils/promise.js'; import { addTask, deleteTask } from './fileWatchers.js'; +import chokidar from './chokidar.js'; const DELAY = 100; @@ -14,9 +16,10 @@ class Watcher extends EventEmitter { this.dirty = true; this.running = false; this.tasks = ensureArray(configs).map(config => new Task(this, config)); + this.succeeded = false; process.nextTick(() => { - this.run(); + this._run(); }); } @@ -26,125 +29,154 @@ class Watcher extends EventEmitter { }); } - error(error) { - this.emit('event', { - code: 'ERROR', - error - }); - } - - makeDirty() { + _makeDirty() { if (this.dirty) return; this.dirty = true; if (!this.running) { setTimeout(() => { - this.run(); + this._run(); }, DELAY); } } - run() { + _run() { this.running = true; this.dirty = false; - // TODO this.emit('event', { - code: 'BUILD_START' + code: 'START' }); mapSequence(this.tasks, task => { return task.run().catch(error => { this.emit('event', { - code: 'ERROR', + code: this.succeeded ? 'ERROR' : 'FATAL', error }); }); }).then(() => { this.running = false; + if (this.dirty) { + this._run(); + } else { + this.emit('event', { + code: 'END' + }); - this.emit('event', { - code: 'BUILD_END' - }); - - if (this.dirty) this.run(); + this.succeeded = true; + } }); } } class Task { - constructor(watcher, config) { + constructor(watcher, options) { this.cache = null; this.watcher = watcher; - this.config = config; + this.options = options; this.closed = false; this.watched = new Set(); - this.dests = new Set( - (config.dest ? [config.dest] : config.targets.map(t => t.dest)).map(dest => path.resolve(dest)) - ); + this.targets = options.targets ? options.targets : [{ dest: options.dest, format: options.format }]; + + this.dests = (this.targets.map(t => t.dest)).map(dest => path.resolve(dest)); + + const watchOptions = options.watch || {}; + if ('useChokidar' in watchOptions) watchOptions.chokidar = watchOptions.useChokidar; + let chokidarOptions = 'chokidar' in watchOptions ? watchOptions.chokidar : !!chokidar; + if (chokidarOptions) { + chokidarOptions = Object.assign( + chokidarOptions === true ? {} : chokidarOptions, + { + ignoreInitial: true + } + ); + } + + if (chokidarOptions && !chokidar) { + throw new Error(`options.watch.chokidar was provided, but chokidar could not be found. Have you installed it?`); + } + + this.chokidarOptions = chokidarOptions; + this.chokidarOptionsHash = JSON.stringify(chokidarOptions); + + this.filter = createFilter(watchOptions.include, watchOptions.exclude); } close() { this.closed = true; this.watched.forEach(id => { - deleteTask(id, this); + deleteTask(id, this, this.chokidarOptionsHash); }); } makeDirty() { if (!this.dirty) { this.dirty = true; - this.watcher.makeDirty(); + this.watcher._makeDirty(); } } run() { this.dirty = false; - const config = Object.assign(this.config, { + const options = Object.assign(this.options, { cache: this.cache }); - return rollup(config).then(bundle => { - if (this.closed) return; + const start = Date.now(); - this.cache = bundle; + this.watcher.emit('event', { + code: 'BUNDLE_START', + input: this.options.entry, + output: this.dests + }); - const watched = new Set(); + return rollup(options) + .then(bundle => { + if (this.closed) return; - bundle.modules.forEach(module => { - if (this.dests.has(module.id)) { - throw new Error('Cannot import the generated bundle'); - } + this.cache = bundle; - watched.add(module.id); - addTask(module.id, this); - }); + const watched = new Set(); - this.watched.forEach(id => { - if (!watched.has(id)) deleteTask(id, this); - }); + bundle.modules.forEach(module => { + if (!this.filter(module.id)) return; - this.watched = watched; + if (~this.dests.indexOf(module.id)) { + throw new Error('Cannot import the generated bundle'); + } - if (this.config.dest) { - return bundle.write({ - format: this.config.format, - dest: this.config.dest + watched.add(module.id); + addTask(module.id, this, this.chokidarOptions, this.chokidarOptionsHash); }); - } - return Promise.all( - this.config.targets.map(target => { - return bundle.write({ - format: target.format, - dest: target.dest - }); - }) - ); - }); + this.watched.forEach(id => { + if (!watched.has(id)) deleteTask(id, this, this.chokidarOptionsHash); + }); + + this.watched = watched; + + return Promise.all( + this.targets.map(target => { + return bundle.write({ + format: target.format, + dest: target.dest, + moduleName: this.options.moduleName + }); + }) + ); + }) + .then(() => { + this.watcher.emit('event', { + code: 'BUNDLE_END', + input: this.options.entry, + output: this.dests, + duration: Date.now() - start + }); + }); } } diff --git a/test/function/namespace-missing-export/_config.js b/test/function/namespace-missing-export/_config.js index 6848cc28686..92a7eaac809 100644 --- a/test/function/namespace-missing-export/_config.js +++ b/test/function/namespace-missing-export/_config.js @@ -1,9 +1,12 @@ +const path = require('path'); + module.exports = { warnings: [ { code: 'MISSING_EXPORT', exporter: 'empty.js', importer: 'main.js', + id: path.resolve( __dirname, 'main.js' ), missing: 'foo', message: `'foo' is not exported by 'empty.js'`, pos: 61, diff --git a/test/function/unused-import/_config.js b/test/function/unused-import/_config.js index a60eaf85acf..8b7820d5b41 100644 --- a/test/function/unused-import/_config.js +++ b/test/function/unused-import/_config.js @@ -12,6 +12,8 @@ module.exports = { }, { code: 'UNUSED_EXTERNAL_IMPORT', + source: 'external', + names: ['unused', 'notused', 'neverused'], message: `'unused', 'notused' and 'neverused' are imported from external module 'external' but never used` }, { diff --git a/test/function/warn-on-eval/_config.js b/test/function/warn-on-eval/_config.js index a959bceb30a..bb1f0b648f9 100644 --- a/test/function/warn-on-eval/_config.js +++ b/test/function/warn-on-eval/_config.js @@ -1,8 +1,11 @@ +const path = require('path'); + module.exports = { description: 'warns about use of eval', warnings: [ { code: 'EVAL', + id: path.resolve(__dirname, 'main.js'), message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, pos: 13, loc: { diff --git a/test/function/warn-on-namespace-conflict/_config.js b/test/function/warn-on-namespace-conflict/_config.js index 90d8fd73b19..e6d488dc515 100644 --- a/test/function/warn-on-namespace-conflict/_config.js +++ b/test/function/warn-on-namespace-conflict/_config.js @@ -1,8 +1,16 @@ +const path = require('path'); + module.exports = { description: 'warns on duplicate export * from', warnings: [ { code: 'NAMESPACE_CONFLICT', + name: 'foo', + reexporter: path.resolve(__dirname, 'main.js'), + sources: [ + path.resolve(__dirname, 'foo.js'), + path.resolve(__dirname, 'deep.js') + ], message: `Conflicting namespaces: main.js re-exports 'foo' from both foo.js and deep.js (will be ignored)` } ] diff --git a/test/function/warn-on-top-level-this/_config.js b/test/function/warn-on-top-level-this/_config.js index 4d9033e6670..02163ba4b8a 100644 --- a/test/function/warn-on-top-level-this/_config.js +++ b/test/function/warn-on-top-level-this/_config.js @@ -1,3 +1,4 @@ +const path = require( 'path' ); const assert = require( 'assert' ); module.exports = { @@ -5,6 +6,7 @@ module.exports = { warnings: [ { code: 'THIS_IS_UNDEFINED', + id: path.resolve(__dirname, 'main.js'), message: `The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten`, pos: 81, loc: { diff --git a/test/function/warn-on-unused-missing-imports/_config.js b/test/function/warn-on-unused-missing-imports/_config.js index 08cc6bac16f..15d06b8a043 100644 --- a/test/function/warn-on-unused-missing-imports/_config.js +++ b/test/function/warn-on-unused-missing-imports/_config.js @@ -6,6 +6,9 @@ module.exports = { warnings: [ { code: 'NON_EXISTENT_EXPORT', + id: path.resolve(__dirname, 'main.js'), + source: path.resolve(__dirname, 'foo.js'), + name: 'b', message: `Non-existent export 'b' is imported from foo.js`, pos: 12, loc: { diff --git a/test/test.js b/test/test.js index a1a071caece..24f787a71ac 100644 --- a/test/test.js +++ b/test/test.js @@ -168,7 +168,7 @@ describe( 'rollup', function () { }); }); - it( 'warns on missing format option', () => { + it( 'throws on missing format option', () => { const warnings = []; return rollup.rollup({ @@ -176,14 +176,9 @@ describe( 'rollup', function () { plugins: [ loader({ x: `console.log( 42 );` }) ], onwarn: warning => warnings.push( warning ) }).then( bundle => { - bundle.generate(); - compareWarnings( warnings, [ - { - code: 'MISSING_FORMAT', - message: `No format option was supplied – defaulting to 'es'`, - url: `https://github.com/rollup/rollup/wiki/JavaScript-API#format` - } - ]); + assert.throws(() => { + bundle.generate(); + }, /You must supply an output format/ ); }); }); }); @@ -261,9 +256,7 @@ describe( 'rollup', function () { }); }); - it( 'warns on es6 format', () => { - let warned; - + it( 'throws on es6 format', () => { return rollup.rollup({ entry: 'x', plugins: [{ @@ -271,14 +264,11 @@ describe( 'rollup', function () { load: () => { return '// empty'; } - }], - onwarn: msg => { - if ( /The es6 format is deprecated/.test( msg ) ) warned = true; - } + }] }).then( bundle => { - return bundle.generate({ format: 'es6' }); - }).then( () => { - assert.ok( warned ); + assert.throws(() => { + return bundle.generate({ format: 'es6' }); + }, /The `es6` output format is deprecated – use `es` instead/); }); }); }); @@ -864,7 +854,8 @@ describe( 'rollup', function () { ] }).then( bundle => { return bundle.write({ - dest + dest, + format: 'es' }); }).then( () => { return sander.unlink( dest ); @@ -960,7 +951,7 @@ describe( 'rollup', function () { }); }); - describe.only( 'rollup.watch', () => { + describe( 'rollup.watch', () => { beforeEach( () => { process.chdir(cwd); return sander.rimraf( 'test/_tmp' ); @@ -1025,14 +1016,18 @@ describe( 'rollup', function () { }); return sequence( watcher, [ - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 42 ); sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); }, - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 43 ); watcher.close(); @@ -1051,19 +1046,25 @@ describe( 'rollup', function () { }); return sequence( watcher, [ - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 42 ); sander.writeFileSync( 'test/_tmp/input/main.js', 'export nope;' ); }, - 'BUILD_START', + 'START', + 'BUNDLE_START', 'ERROR', + 'END', () => { sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); }, - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 43 ); watcher.close(); @@ -1082,21 +1083,26 @@ describe( 'rollup', function () { }); return sequence( watcher, [ - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 42 ); sander.unlinkSync( 'test/_tmp/input/main.js' ); sander.writeFileSync( 'test/_tmp/input/main.js', 'export nope;' ); }, - 'BUILD_START', + 'START', + 'BUNDLE_START', 'ERROR', () => { sander.unlinkSync( 'test/_tmp/input/main.js' ); sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); }, - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 43 ); watcher.close(); @@ -1115,20 +1121,25 @@ describe( 'rollup', function () { }); return sequence( watcher, [ - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 42 ); sander.writeFileSync( 'test/_tmp/input/main.js', `import '../output/bundle.js'` ); }, - 'BUILD_START', + 'START', + 'BUNDLE_START', 'ERROR', event => { assert.equal( event.error.message, 'Cannot import the generated bundle' ); sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); }, - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.equal( run( './_tmp/output/bundle.js' ), 43 ); watcher.close(); @@ -1150,14 +1161,18 @@ describe( 'rollup', function () { }); return sequence( watcher, [ - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-1', bar: 'bar-1' }); sander.writeFileSync( 'test/_tmp/input/foo.js', `export default 'foo-2';` ); }, - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-2', bar: 'bar-1' }); sander.writeFileSync( 'test/_tmp/input/bar.js', `export default 'bar-2';` ); @@ -1183,14 +1198,18 @@ describe( 'rollup', function () { }); return sequence( watcher, [ - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-1', bar: 'bar-1' }); sander.writeFileSync( 'test/_tmp/input/foo.js', `export default 'foo-2';` ); }, - 'BUILD_START', - 'BUILD_END', + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', () => { assert.deepEqual( run( './_tmp/output/bundle.js' ), { foo: 'foo-2', bar: 'bar-1' }); sander.writeFileSync( 'test/_tmp/input/bar.js', `export default 'bar-2';` ); From 2a231b6c5a3156e8ab00f4eb112e705960a20f44 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 10 Aug 2017 20:23:17 -0400 Subject: [PATCH 13/15] handle renamed files in watch mode --- src/watch/fileWatchers.js | 2 +- src/watch/index.js | 42 ++++++++++++++++++++++++++------------- test/test.js | 3 +-- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/watch/fileWatchers.js b/src/watch/fileWatchers.js index 910bdaf7ffe..85b5b2c61ed 100644 --- a/src/watch/fileWatchers.js +++ b/src/watch/fileWatchers.js @@ -60,8 +60,8 @@ export default class FileWatcher { const handleWatchEvent = event => { if (event === 'rename' || event === 'unlink') { this.fsWatcher.close(); - dispose(); this.trigger(); + dispose(); } else { // this is necessary because we get duplicate events... const contents = fs.readFileSync(id, 'utf-8'); diff --git a/src/watch/index.js b/src/watch/index.js index 58975948bc6..10b07f9c96a 100644 --- a/src/watch/index.js +++ b/src/watch/index.js @@ -48,25 +48,27 @@ class Watcher extends EventEmitter { code: 'START' }); - mapSequence(this.tasks, task => { - return task.run().catch(error => { + mapSequence(this.tasks, task => task.run()) + .then(() => { + this.succeeded = true; + this.emit('event', { - code: this.succeeded ? 'ERROR' : 'FATAL', - error + code: 'END' }); - }); - }).then(() => { - this.running = false; - if (this.dirty) { - this._run(); - } else { + }) + .catch(error => { this.emit('event', { - code: 'END' + code: this.succeeded ? 'ERROR' : 'FATAL', + error }); + }) + .then(() => { + this.running = false; - this.succeeded = true; - } - }); + if (this.dirty) { + this._run(); + } + }); } } @@ -76,6 +78,7 @@ class Task { this.watcher = watcher; this.options = options; + this.dirty = true; this.closed = false; this.watched = new Set(); @@ -120,6 +123,7 @@ class Task { } run() { + if (!this.dirty) return; this.dirty = false; const options = Object.assign(this.options, { @@ -176,6 +180,16 @@ class Task { output: this.dests, duration: Date.now() - start }); + }) + .catch(error => { + if (this.cache) { + this.cache.modules.forEach(module => { + // this is necessary to ensure that any 'renamed' files + // continue to be watched following an error + addTask(module.id, this, this.chokidarOptions, this.chokidarOptionsHash); + }); + } + throw error; }); } } diff --git a/test/test.js b/test/test.js index 24f787a71ac..cddd9d540f4 100644 --- a/test/test.js +++ b/test/test.js @@ -975,7 +975,7 @@ describe( 'rollup', function () { else if ( typeof next === 'string' ) { watcher.once( 'event', event => { if ( event.code !== next ) { - reject( new Error( `Expected ${next} error, got ${event.code}` ) ); + reject( new Error( `Expected ${next} event, got ${event.code}` ) ); } else { go( event ); } @@ -1057,7 +1057,6 @@ describe( 'rollup', function () { 'START', 'BUNDLE_START', 'ERROR', - 'END', () => { sander.writeFileSync( 'test/_tmp/input/main.js', 'export default 43;' ); }, From 9e481d890d333210de3364379b0b1004ee2169f7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 10 Aug 2017 21:17:47 -0400 Subject: [PATCH 14/15] fix intermittent test failures --- src/watch/fileWatchers.js | 10 ++++++---- src/watch/index.js | 24 ++++++++++++++++-------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/watch/fileWatchers.js b/src/watch/fileWatchers.js index 85b5b2c61ed..15f69e4cbb1 100644 --- a/src/watch/fileWatchers.js +++ b/src/watch/fileWatchers.js @@ -29,11 +29,13 @@ export function deleteTask(id, target, chokidarOptionsHash) { const group = watchers.get(chokidarOptionsHash); const watcher = group.get(id); - watcher.tasks.delete(target); + if (watcher) { + watcher.tasks.delete(target); - if (watcher.tasks.size === 0) { - watcher.close(); - group.delete(id); + if (watcher.tasks.size === 0) { + watcher.close(); + group.delete(id); + } } } diff --git a/src/watch/index.js b/src/watch/index.js index 10b07f9c96a..4706d836d84 100644 --- a/src/watch/index.js +++ b/src/watch/index.js @@ -147,14 +147,8 @@ class Task { const watched = new Set(); bundle.modules.forEach(module => { - if (!this.filter(module.id)) return; - - if (~this.dests.indexOf(module.id)) { - throw new Error('Cannot import the generated bundle'); - } - watched.add(module.id); - addTask(module.id, this, this.chokidarOptions, this.chokidarOptionsHash); + this.watchFile(module.id); }); this.watched.forEach(id => { @@ -182,16 +176,30 @@ class Task { }); }) .catch(error => { + if (this.closed) return; + if (this.cache) { this.cache.modules.forEach(module => { // this is necessary to ensure that any 'renamed' files // continue to be watched following an error - addTask(module.id, this, this.chokidarOptions, this.chokidarOptionsHash); + this.watchFile(module.id); }); } throw error; }); } + + watchFile(id) { + if (!this.filter(id)) return; + + if (~this.dests.indexOf(id)) { + throw new Error('Cannot import the generated bundle'); + } + + // this is necessary to ensure that any 'renamed' files + // continue to be watched following an error + addTask(id, this, this.chokidarOptions, this.chokidarOptionsHash); + } } export default function watch(configs) { From c63782ef6725735600ae83c5ca4dcb4dec787eae Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 11 Aug 2017 08:33:35 -0400 Subject: [PATCH 15/15] preserve semicolon if var decl is body of for loop --- src/ast/nodes/VariableDeclaration.js | 4 ++-- .../_config.js | 0 test/form/for-loop-body-var-declaration/_expected/amd.js | 5 +++++ test/form/for-loop-body-var-declaration/_expected/cjs.js | 3 +++ .../_expected/es.js} | 0 test/form/for-loop-body-var-declaration/_expected/iife.js | 6 ++++++ .../_expected/umd.js | 3 +-- test/form/for-loop-body-var-declaration/main.js | 1 + test/form/line-wrap/_expected/amd.js | 6 ------ test/form/line-wrap/_expected/cjs.js | 4 ---- test/form/line-wrap/_expected/es.js | 2 -- test/form/line-wrap/_expected/iife.js | 7 ------- 12 files changed, 18 insertions(+), 23 deletions(-) rename test/form/{line-wrap => for-loop-body-var-declaration}/_config.js (100%) create mode 100644 test/form/for-loop-body-var-declaration/_expected/amd.js create mode 100644 test/form/for-loop-body-var-declaration/_expected/cjs.js rename test/form/{line-wrap/main.js => for-loop-body-var-declaration/_expected/es.js} (100%) create mode 100644 test/form/for-loop-body-var-declaration/_expected/iife.js rename test/form/{line-wrap => for-loop-body-var-declaration}/_expected/umd.js (82%) create mode 100644 test/form/for-loop-body-var-declaration/main.js delete mode 100644 test/form/line-wrap/_expected/amd.js delete mode 100644 test/form/line-wrap/_expected/cjs.js delete mode 100644 test/form/line-wrap/_expected/es.js delete mode 100644 test/form/line-wrap/_expected/iife.js diff --git a/src/ast/nodes/VariableDeclaration.js b/src/ast/nodes/VariableDeclaration.js index 2c21130406f..ccdea620059 100644 --- a/src/ast/nodes/VariableDeclaration.js +++ b/src/ast/nodes/VariableDeclaration.js @@ -95,10 +95,10 @@ export default class VariableDeclaration extends Node { } else { // always include a semi-colon (https://github.com/rollup/rollup/pull/1013), // unless it's a var declaration in a loop head - const needsSemicolon = !forStatement.test( this.parent.type ); + const needsSemicolon = !forStatement.test( this.parent.type ) || this === this.parent.body; if ( this.end > c ) { - code.overwrite( c, this.end, needsSemicolon ? ';' : '\n' ); + code.overwrite( c, this.end, needsSemicolon ? ';' : '' ); } else if ( needsSemicolon ) { this.insertSemicolon( code ); } diff --git a/test/form/line-wrap/_config.js b/test/form/for-loop-body-var-declaration/_config.js similarity index 100% rename from test/form/line-wrap/_config.js rename to test/form/for-loop-body-var-declaration/_config.js diff --git a/test/form/for-loop-body-var-declaration/_expected/amd.js b/test/form/for-loop-body-var-declaration/_expected/amd.js new file mode 100644 index 00000000000..65f8e046cf7 --- /dev/null +++ b/test/form/for-loop-body-var-declaration/_expected/amd.js @@ -0,0 +1,5 @@ +define(function () { 'use strict'; + + for(var x=1;x<2;x++)var d=x|0;console.log(d); + +}); diff --git a/test/form/for-loop-body-var-declaration/_expected/cjs.js b/test/form/for-loop-body-var-declaration/_expected/cjs.js new file mode 100644 index 00000000000..9398f8f1bbd --- /dev/null +++ b/test/form/for-loop-body-var-declaration/_expected/cjs.js @@ -0,0 +1,3 @@ +'use strict'; + +for(var x=1;x<2;x++)var d=x|0;console.log(d); diff --git a/test/form/line-wrap/main.js b/test/form/for-loop-body-var-declaration/_expected/es.js similarity index 100% rename from test/form/line-wrap/main.js rename to test/form/for-loop-body-var-declaration/_expected/es.js diff --git a/test/form/for-loop-body-var-declaration/_expected/iife.js b/test/form/for-loop-body-var-declaration/_expected/iife.js new file mode 100644 index 00000000000..1619ca1df29 --- /dev/null +++ b/test/form/for-loop-body-var-declaration/_expected/iife.js @@ -0,0 +1,6 @@ +(function () { + 'use strict'; + + for(var x=1;x<2;x++)var d=x|0;console.log(d); + +}()); diff --git a/test/form/line-wrap/_expected/umd.js b/test/form/for-loop-body-var-declaration/_expected/umd.js similarity index 82% rename from test/form/line-wrap/_expected/umd.js rename to test/form/for-loop-body-var-declaration/_expected/umd.js index b65bbb30135..c50f8c4a11b 100644 --- a/test/form/line-wrap/_expected/umd.js +++ b/test/form/for-loop-body-var-declaration/_expected/umd.js @@ -4,7 +4,6 @@ (factory()); }(this, (function () { 'use strict'; - for(var x=1;x<2;x++)var d=x|0 - console.log(d); + for(var x=1;x<2;x++)var d=x|0;console.log(d); }))); diff --git a/test/form/for-loop-body-var-declaration/main.js b/test/form/for-loop-body-var-declaration/main.js new file mode 100644 index 00000000000..4f2606873e9 --- /dev/null +++ b/test/form/for-loop-body-var-declaration/main.js @@ -0,0 +1 @@ +for(var x=1;x<2;x++)var d=x|0;console.log(d); diff --git a/test/form/line-wrap/_expected/amd.js b/test/form/line-wrap/_expected/amd.js deleted file mode 100644 index 61908d3ec40..00000000000 --- a/test/form/line-wrap/_expected/amd.js +++ /dev/null @@ -1,6 +0,0 @@ -define(function () { 'use strict'; - - for(var x=1;x<2;x++)var d=x|0 - console.log(d); - -}); diff --git a/test/form/line-wrap/_expected/cjs.js b/test/form/line-wrap/_expected/cjs.js deleted file mode 100644 index f73746adbbf..00000000000 --- a/test/form/line-wrap/_expected/cjs.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -for(var x=1;x<2;x++)var d=x|0 -console.log(d); diff --git a/test/form/line-wrap/_expected/es.js b/test/form/line-wrap/_expected/es.js deleted file mode 100644 index 3bf9930a40c..00000000000 --- a/test/form/line-wrap/_expected/es.js +++ /dev/null @@ -1,2 +0,0 @@ -for(var x=1;x<2;x++)var d=x|0 -console.log(d); diff --git a/test/form/line-wrap/_expected/iife.js b/test/form/line-wrap/_expected/iife.js deleted file mode 100644 index d65242fb196..00000000000 --- a/test/form/line-wrap/_expected/iife.js +++ /dev/null @@ -1,7 +0,0 @@ -(function () { - 'use strict'; - - for(var x=1;x<2;x++)var d=x|0 - console.log(d); - -}());