From 313b108f41dc801ef2b5c393e5bdc5b33d0c7c20 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 23 Sep 2015 16:41:46 -0400 Subject: [PATCH 1/6] remove some unused code, add some additional tests --- src/Module.js | 13 ------------- src/Statement.js | 18 ------------------ test/test.js | 30 ++++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/Module.js b/src/Module.js index a32c021..67f22fa 100644 --- a/src/Module.js +++ b/src/Module.js @@ -586,19 +586,6 @@ export default class Module { magicString.remove( statement.start, statement.next ); return; } - - // skip `export var foo;` if foo is exported - if ( isEmptyExportedVarDeclaration( statement.node.declaration, this.exports, toExport ) ) { - magicString.remove( statement.start, statement.next ); - return; - } - } - - // skip empty var declarations for exported bindings - // (otherwise we're left with `exports.foo;`, which is useless) - if ( isEmptyExportedVarDeclaration( statement.node, this.exports, toExport ) ) { - magicString.remove( statement.start, statement.next ); - return; } // split up/remove var declarations as necessary diff --git a/src/Statement.js b/src/Statement.js index 1a165b4..abd0671 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -459,24 +459,6 @@ export default class Statement { magicString.overwrite( node.start, id.end, bundleExports[ name ], true ); id._skip = true; } - - // otherwise, we insert the `exports.foo = foo` after the declaration - else { - const exportInitialisers = node.declarations - .map( declarator => declarator.id.name ) - .filter( name => !!bundleExports[ name ] ) - .map( name => `\n${bundleExports[name]} = ${name};` ) - .join( '' ); - - if ( exportInitialisers ) { - // TODO clean this up - try { - magicString.insert( node.end, exportInitialisers ); - } catch ( err ) { - magicString.append( exportInitialisers ); - } - } - } } } diff --git a/test/test.js b/test/test.js index a871ccd..624c4a0 100644 --- a/test/test.js +++ b/test/test.js @@ -45,6 +45,36 @@ describe( 'rollup', function () { it( 'has a rollup method', function () { assert.equal( typeof rollup.rollup, 'function' ); }); + + it( 'fails without options or options.entry', function () { + assert.throws( function () { + rollup.rollup(); + }, /must supply options\.entry/ ); + + assert.throws( function () { + rollup.rollup({}); + }, /must supply options\.entry/ ); + }); + }); + + describe( 'bundle.write()', function () { + it( 'fails without options or options.dest', function () { + return rollup.rollup({ + entry: 'x', + resolveId: function () { return 'test'; }, + load: function () { + return '// empty'; + } + }).then( function ( bundle ) { + assert.throws( function () { + bundle.write(); + }, /must supply options\.dest/ ); + + assert.throws( function () { + bundle.write({}); + }, /must supply options\.dest/ ); + }); + }); }); describe( 'function', function () { From 79f368d35ac1e2fe3f8afcfc5eeea5231ebc47f6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 25 Sep 2015 16:22:19 -0400 Subject: [PATCH 2/6] generate html and lcov coverage reports --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index f1ad39b..8ed39a7 100644 --- a/package.json +++ b/package.json @@ -11,9 +11,9 @@ "pretest": "npm run build", "test": "mocha", "pretest-coverage": "npm run build", - "test-coverage": "istanbul cover --report json node_modules/.bin/_mocha -- -u exports -R spec test/test.js", - "posttest-coverage": "remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.json -b dist", - "ci": "npm run test-coverage && codecov < coverage/coverage-remapped.json", + "test-coverage": "rm -rf coverage/* && istanbul cover --report json node_modules/.bin/_mocha -- -u exports -R spec test/test.js", + "posttest-coverage": "remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.json -b dist && remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.lcov -t lcovonly -b dist && remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped -t html -b dist", + "ci": "npm run test-coverage && codecov < coverage/coverage-remapped.lcov", "build": "gobble build -f dist", "prepublish": "npm test", "lint": "eslint src" From bdda8e6b3af8e774321f43e608a3687fa50c5803 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 25 Sep 2015 16:22:50 -0400 Subject: [PATCH 3/6] test for non-renamining of property keys --- test/function/property-keys-not-renamed/_config.js | 3 +++ test/function/property-keys-not-renamed/main.js | 7 +++++++ test/function/property-keys-not-renamed/one.js | 11 +++++++++++ test/function/property-keys-not-renamed/three.js | 11 +++++++++++ test/function/property-keys-not-renamed/two.js | 11 +++++++++++ 5 files changed, 43 insertions(+) create mode 100644 test/function/property-keys-not-renamed/_config.js create mode 100644 test/function/property-keys-not-renamed/main.js create mode 100644 test/function/property-keys-not-renamed/one.js create mode 100644 test/function/property-keys-not-renamed/three.js create mode 100644 test/function/property-keys-not-renamed/two.js diff --git a/test/function/property-keys-not-renamed/_config.js b/test/function/property-keys-not-renamed/_config.js new file mode 100644 index 0000000..d4e1f82 --- /dev/null +++ b/test/function/property-keys-not-renamed/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'does not rename property keys' +}; diff --git a/test/function/property-keys-not-renamed/main.js b/test/function/property-keys-not-renamed/main.js new file mode 100644 index 0000000..9134b8e --- /dev/null +++ b/test/function/property-keys-not-renamed/main.js @@ -0,0 +1,7 @@ +import one from './one'; +import two from './two'; +import three from './three'; + +assert.equal( one(), 'one' ); +assert.equal( two(), 'two' ); +assert.equal( three(), 'three' ); diff --git a/test/function/property-keys-not-renamed/one.js b/test/function/property-keys-not-renamed/one.js new file mode 100644 index 0000000..124f65a --- /dev/null +++ b/test/function/property-keys-not-renamed/one.js @@ -0,0 +1,11 @@ +const obj = { + foo: foo +}; + +function foo () { + return 'one'; +} + +export default function () { + return obj.foo(); +} diff --git a/test/function/property-keys-not-renamed/three.js b/test/function/property-keys-not-renamed/three.js new file mode 100644 index 0000000..f6ef70a --- /dev/null +++ b/test/function/property-keys-not-renamed/three.js @@ -0,0 +1,11 @@ +const obj = { + foo: foo +}; + +function foo () { + return 'three'; +} + +export default function () { + return obj.foo(); +} diff --git a/test/function/property-keys-not-renamed/two.js b/test/function/property-keys-not-renamed/two.js new file mode 100644 index 0000000..44874f0 --- /dev/null +++ b/test/function/property-keys-not-renamed/two.js @@ -0,0 +1,11 @@ +const obj = { + foo: foo +}; + +function foo () { + return 'two'; +} + +export default function () { + return obj.foo(); +} From 4faa4ea298edfd68a9c326dee7c2110755970224 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 25 Sep 2015 16:25:59 -0400 Subject: [PATCH 4/6] test for module importing itself --- test/function/cannot-import-self/_config.js | 8 ++++++++ test/function/cannot-import-self/main.js | 1 + 2 files changed, 9 insertions(+) create mode 100644 test/function/cannot-import-self/_config.js create mode 100644 test/function/cannot-import-self/main.js diff --git a/test/function/cannot-import-self/_config.js b/test/function/cannot-import-self/_config.js new file mode 100644 index 0000000..2413d03 --- /dev/null +++ b/test/function/cannot-import-self/_config.js @@ -0,0 +1,8 @@ +var assert = require( 'assert' ); + +module.exports = { + description: 'prevents a module importing itself', + error: function ( err ) { + assert.ok( /A module cannot import itself/.test( err.message ) ); + } +}; diff --git a/test/function/cannot-import-self/main.js b/test/function/cannot-import-self/main.js new file mode 100644 index 0000000..48e08e6 --- /dev/null +++ b/test/function/cannot-import-self/main.js @@ -0,0 +1 @@ +import me from './main'; From 1034d0b6fe114459833f62af658e039fdc7c4932 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 25 Sep 2015 16:47:54 -0400 Subject: [PATCH 5/6] test for bad export types --- .../export-type-mismatch-b/_config.js | 11 +++ test/function/export-type-mismatch-b/main.js | 1 + .../export-type-mismatch-c/_config.js | 11 +++ test/function/export-type-mismatch-c/main.js | 1 + test/function/export-type-mismatch/_config.js | 11 +++ test/function/export-type-mismatch/main.js | 1 + test/test.js | 71 +++++++++---------- 7 files changed, 68 insertions(+), 39 deletions(-) create mode 100644 test/function/export-type-mismatch-b/_config.js create mode 100644 test/function/export-type-mismatch-b/main.js create mode 100644 test/function/export-type-mismatch-c/_config.js create mode 100644 test/function/export-type-mismatch-c/main.js create mode 100644 test/function/export-type-mismatch/_config.js create mode 100644 test/function/export-type-mismatch/main.js diff --git a/test/function/export-type-mismatch-b/_config.js b/test/function/export-type-mismatch-b/_config.js new file mode 100644 index 0000000..25a7220 --- /dev/null +++ b/test/function/export-type-mismatch-b/_config.js @@ -0,0 +1,11 @@ +var assert = require( 'assert' ); + +module.exports = { + description: 'export type must be auto, default, named or none', + bundleOptions: { + exports: 'blah' + }, + generateError: function ( err ) { + assert.ok( /options\.exports must be 'default', 'named', 'none', 'auto', or left unspecified/.test( err.message ) ); + } +}; diff --git a/test/function/export-type-mismatch-b/main.js b/test/function/export-type-mismatch-b/main.js new file mode 100644 index 0000000..7a4e8a7 --- /dev/null +++ b/test/function/export-type-mismatch-b/main.js @@ -0,0 +1 @@ +export default 42; diff --git a/test/function/export-type-mismatch-c/_config.js b/test/function/export-type-mismatch-c/_config.js new file mode 100644 index 0000000..8df0d58 --- /dev/null +++ b/test/function/export-type-mismatch-c/_config.js @@ -0,0 +1,11 @@ +var assert = require( 'assert' ); + +module.exports = { + description: 'cannot have named exports if explicit export type is default', + bundleOptions: { + exports: 'none' + }, + generateError: function ( err ) { + assert.ok( /'none' was specified for options\.exports/.test( err.message ) ); + } +}; diff --git a/test/function/export-type-mismatch-c/main.js b/test/function/export-type-mismatch-c/main.js new file mode 100644 index 0000000..7a4e8a7 --- /dev/null +++ b/test/function/export-type-mismatch-c/main.js @@ -0,0 +1 @@ +export default 42; diff --git a/test/function/export-type-mismatch/_config.js b/test/function/export-type-mismatch/_config.js new file mode 100644 index 0000000..5b321d6 --- /dev/null +++ b/test/function/export-type-mismatch/_config.js @@ -0,0 +1,11 @@ +var assert = require( 'assert' ); + +module.exports = { + description: 'cannot have named exports if explicit export type is default', + bundleOptions: { + exports: 'default' + }, + generateError: function ( err ) { + assert.ok( /'default' was specified for options\.exports/.test( err.message ) ); + } +}; diff --git a/test/function/export-type-mismatch/main.js b/test/function/export-type-mismatch/main.js new file mode 100644 index 0000000..3b8dc9f --- /dev/null +++ b/test/function/export-type-mismatch/main.js @@ -0,0 +1 @@ +export var foo = 42; diff --git a/test/test.js b/test/test.js index 624c4a0..a69d215 100644 --- a/test/test.js +++ b/test/test.js @@ -110,64 +110,57 @@ describe( 'rollup', function () { format: 'cjs' })); - if ( config.error ) { + if ( config.generateError ) { unintendedError = new Error( 'Expected an error while generating output' ); } } catch ( err ) { - if ( config.error ) { - config.error( err ); + if ( config.generateError ) { + config.generateError( err ); } else { unintendedError = err; } } if ( unintendedError ) throw unintendedError; + if ( config.error || config.generateError ) return; var code; - try { - if ( config.babel ) { - code = babel.transform( result.code, { - blacklist: [ 'es6.modules' ], - loose: [ 'es6.classes' ] - }).code; - } else { - code = result.code; - } + if ( config.babel ) { + code = babel.transform( result.code, { + blacklist: [ 'es6.modules' ], + loose: [ 'es6.classes' ] + }).code; + } else { + code = result.code; + } - var module = { - exports: {} - }; + var module = { + exports: {} + }; - var context = extend({ - require: require, - module: module, - exports: module.exports, - assert: assert - }, config.context || {} ); + var context = extend({ + require: require, + module: module, + exports: module.exports, + assert: assert + }, config.context || {} ); - var contextKeys = Object.keys( context ); - var contextValues = contextKeys.map( function ( key ) { - return context[ key ]; - }); + var contextKeys = Object.keys( context ); + var contextValues = contextKeys.map( function ( key ) { + return context[ key ]; + }); - var fn = new Function( contextKeys, code ); - fn.apply( {}, contextValues ); + var fn = new Function( contextKeys, code ); + fn.apply( {}, contextValues ); - if ( config.error ) { - unintendedError = new Error( 'Expected an error while executing output' ); - } - - if ( config.exports ) config.exports( module.exports ); - if ( config.bundle ) config.bundle( bundle ); - } catch ( err ) { - if ( config.error ) { - config.error( err ); - } else { - unintendedError = err; - } + if ( config.error ) { + unintendedError = new Error( 'Expected an error while executing output' ); } + if ( config.exports ) config.exports( module.exports ); + if ( config.bundle ) config.bundle( bundle ); + if ( config.show || unintendedError ) { console.log( code + '\n\n\n' ); } From ba6f643da9221086984bca70fdaad4cb4598294e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 25 Sep 2015 16:51:26 -0400 Subject: [PATCH 6/6] dont use const in tests... --- test/function/property-keys-not-renamed/one.js | 2 +- test/function/property-keys-not-renamed/three.js | 2 +- test/function/property-keys-not-renamed/two.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/function/property-keys-not-renamed/one.js b/test/function/property-keys-not-renamed/one.js index 124f65a..ab4a0f9 100644 --- a/test/function/property-keys-not-renamed/one.js +++ b/test/function/property-keys-not-renamed/one.js @@ -1,4 +1,4 @@ -const obj = { +var obj = { foo: foo }; diff --git a/test/function/property-keys-not-renamed/three.js b/test/function/property-keys-not-renamed/three.js index f6ef70a..a78e2e5 100644 --- a/test/function/property-keys-not-renamed/three.js +++ b/test/function/property-keys-not-renamed/three.js @@ -1,4 +1,4 @@ -const obj = { +var obj = { foo: foo }; diff --git a/test/function/property-keys-not-renamed/two.js b/test/function/property-keys-not-renamed/two.js index 44874f0..70e142b 100644 --- a/test/function/property-keys-not-renamed/two.js +++ b/test/function/property-keys-not-renamed/two.js @@ -1,4 +1,4 @@ -const obj = { +var obj = { foo: foo };