From af5f7dd7a0cb75a1cdb0d6ce2c6d2d4936fa97d0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 6 Jun 2016 17:31:05 -0400 Subject: [PATCH 1/6] switch from babel to buble --- package.json | 8 +++----- rollup.config.js | 4 ++-- test/.babelrc | 3 --- test/test.js | 12 ++++++------ 4 files changed, 11 insertions(+), 16 deletions(-) delete mode 100644 test/.babelrc diff --git a/package.json b/package.json index dd08912..19d362b 100644 --- a/package.json +++ b/package.json @@ -3,20 +3,18 @@ "version": "2.2.1", "description": "Convert CommonJS modules to ES2015", "devDependencies": { - "babel-preset-es2015": "^6.3.13", - "babel-preset-es2015-rollup": "^1.0.0", - "babel-register": "^6.3.13", + "buble": "^0.10.6", "eslint": "^1.7.3", "mocha": "^2.3.3", "rollup": "^0.25.8", - "rollup-plugin-babel": "^2.2.0", + "rollup-plugin-buble": "^0.10.0", "rollup-plugin-node-resolve": "^1.1.0", "source-map": "^0.5.3" }, "main": "dist/rollup-plugin-commonjs.cjs.js", "jsnext:main": "dist/rollup-plugin-commonjs.es6.js", "scripts": { - "test": "mocha --compilers js:babel-register", + "test": "mocha --compilers js:buble/register", "pretest": "npm run build", "build": "rm -rf dist/* && rollup -c -f cjs -o dist/rollup-plugin-commonjs.cjs.js && rollup -c -f es6 -o dist/rollup-plugin-commonjs.es6.js", "prepublish": "npm test" diff --git a/rollup.config.js b/rollup.config.js index b586a36..5c78ba1 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -1,9 +1,9 @@ -import babel from 'rollup-plugin-babel'; +import buble from 'rollup-plugin-buble'; var external = Object.keys( require( './package.json' ).dependencies ).concat([ 'fs', 'path' ]); export default { entry: 'src/index.js', - plugins: [ babel() ], + plugins: [ buble() ], external: external }; diff --git a/test/.babelrc b/test/.babelrc deleted file mode 100644 index de9f4a8..0000000 --- a/test/.babelrc +++ /dev/null @@ -1,3 +0,0 @@ -{ - "presets": [ "es2015" ] -} diff --git a/test/test.js b/test/test.js index 21846d8..40e5e25 100644 --- a/test/test.js +++ b/test/test.js @@ -1,9 +1,9 @@ -import * as path from 'path'; -import * as assert from 'assert'; -import { SourceMapConsumer } from 'source-map'; -import { rollup } from 'rollup'; -import nodeResolve from 'rollup-plugin-node-resolve'; -import commonjs from '..'; +const path = require( 'path' ); +const assert = require( 'assert' ); +const { SourceMapConsumer } = require( 'source-map' ); +const { rollup } = require( 'rollup' ); +const nodeResolve = require( 'rollup-plugin-node-resolve' ); +const commonjs = require( '..' ); process.chdir( __dirname ); From b5d5a3e320a29109e98ad6400870366c6fe59b52 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 6 Jun 2016 17:32:38 -0400 Subject: [PATCH 2/6] update deps --- package.json | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 19d362b..617ca9a 100644 --- a/package.json +++ b/package.json @@ -4,12 +4,12 @@ "description": "Convert CommonJS modules to ES2015", "devDependencies": { "buble": "^0.10.6", - "eslint": "^1.7.3", - "mocha": "^2.3.3", - "rollup": "^0.25.8", + "eslint": "^2.11.1", + "mocha": "^2.5.3", + "rollup": "^0.26.7", "rollup-plugin-buble": "^0.10.0", - "rollup-plugin-node-resolve": "^1.1.0", - "source-map": "^0.5.3" + "rollup-plugin-node-resolve": "^1.5.0", + "source-map": "^0.5.6" }, "main": "dist/rollup-plugin-commonjs.cjs.js", "jsnext:main": "dist/rollup-plugin-commonjs.es6.js", @@ -25,11 +25,11 @@ "README.md" ], "dependencies": { - "acorn": "^2.4.0", - "estree-walker": "^0.2.0", - "magic-string": "^0.10.0", - "resolve": "^1.1.6", - "rollup-pluginutils": "^1.2.0" + "acorn": "^3.1.0", + "estree-walker": "^0.2.1", + "magic-string": "^0.15.0", + "resolve": "^1.1.7", + "rollup-pluginutils": "^1.3.1" }, "repository": { "type": "git", From 115d39d0e6bf1c233a2f587a7eae725c4595c95c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 6 Jun 2016 18:26:58 -0400 Subject: [PATCH 3/6] stateless plugin --- src/index.js | 52 +++++++++++++++++++++++++--------------------------- test/test.js | 27 +++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/src/index.js b/src/index.js index 376577c..d488bf8 100644 --- a/src/index.js +++ b/src/index.js @@ -37,13 +37,20 @@ function getCandidates ( resolved, extensions ) { ); } +const HELPERS_ID = '%cjs%'; +const HELPERS_NAME = '__commonjsHelpers'; +const HELPERS = ` +export var commonjsGlobal = typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {} + +export function createCommonjsModule(fn, module) { + return module = { exports: {} }, fn(module, module.exports), module.exports; +}`; + export default function commonjs ( options = {} ) { const extensions = options.extensions || ['.js']; const filter = createFilter( options.include, options.exclude ); const ignoreGlobal = options.ignoreGlobal; const firstpass = ignoreGlobal ? firstpassNoGlobal : firstpassGlobal; - let bundleUsesGlobal = false; - let bundleRequiresWrappers = false; const sourceMap = options.sourceMap !== false; @@ -64,6 +71,8 @@ export default function commonjs ( options = {} ) { return { resolveId ( importee, importer ) { + if ( importee === HELPERS_ID ) return importee; + if ( importee[0] !== '.' ) return; // not our problem const resolved = resolve( dirname( importer ), importee ); @@ -77,6 +86,10 @@ export default function commonjs ( options = {} ) { } }, + load ( id ) { + if ( id === HELPERS_ID ) return HELPERS; + }, + transform ( code, id ) { if ( !filter( id ) ) return null; if ( extensions.indexOf( extname( id ) ) === -1 ) return null; @@ -145,13 +158,16 @@ export default function commonjs ( options = {} ) { } if ( node.type === 'Identifier' ) { - if ( ( node.name in uses && !uses[ node.name ] ) && isReference( node, parent ) && !scope.contains( node.name ) ) uses[ node.name ] = true; + if ( ( node.name in uses && !uses[ node.name ] ) && isReference( node, parent ) && !scope.contains( node.name ) ) { + uses[ node.name ] = true; + if ( node.name === 'global' ) magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal` ); + } return; } if ( node.type === 'ThisExpression' && scopeDepth === 0 && !ignoreGlobal ) { uses.global = true; - magicString.overwrite( node.start, node.end, `__commonjs_global`, true ); + magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true ); return; } @@ -195,20 +211,18 @@ export default function commonjs ( options = {} ) { return null; // not a CommonJS module } - bundleRequiresWrappers = true; - const name = getName( id ); - const importBlock = sources.length ? + const importBlock = [ `import * as ${HELPERS_NAME} from '${HELPERS_ID}';` ].concat( sources.map( source => { const { name, importsDefault } = required[ source ]; return `import ${importsDefault ? `${name} from ` : ``}'${source}';`; - }).join( '\n' ) : - ''; + }) + ).join( '\n' ); - const args = `module${uses.exports || uses.global ? ', exports' : ''}${uses.global ? ', global' : ''}`; + const args = `module${uses.exports ? ', exports' : ''}`; - const intro = `\n\nvar ${name} = __commonjs(function (${args}) {\n`; + const intro = `\n\nvar ${name} = __commonjsHelpers.createCommonjsModule(function (${args}) {\n`; let outro = `\n});\n\nexport default (${name} && typeof ${name} === 'object' && 'default' in ${name} ? ${name}['default'] : ${name});\n`; outro += Object.keys( namedExports ) @@ -224,23 +238,7 @@ export default function commonjs ( options = {} ) { code = magicString.toString(); const map = sourceMap ? magicString.generateMap() : null; - if ( uses.global ) bundleUsesGlobal = true; - return { code, map }; - }, - - intro () { - var intros = []; - - if ( bundleUsesGlobal ) { - intros.push( `var __commonjs_global = typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {}` ); - } - - if ( bundleRequiresWrappers ) { - intros.push( `function __commonjs(fn, module) { return module = { exports: {} }, fn(module, module.exports${bundleUsesGlobal ? ', __commonjs_global' : ''}), module.exports; }\n` ); - } - - return intros.join( '\n' ); } }; } diff --git a/test/test.js b/test/test.js index 40e5e25..3bb68df 100644 --- a/test/test.js +++ b/test/test.js @@ -20,6 +20,27 @@ function executeBundle ( bundle ) { return module; } +function getLocation ( source, charIndex ) { + var lines = source.split( '\n' ); + var len = lines.length; + + var lineStart = 0; + var i; + + for ( i = 0; i < len; i += 1 ) { + var line = lines[i]; + var lineEnd = lineStart + line.length + 1; // +1 for newline + + if ( lineEnd > charIndex ) { + return { line: i + 1, column: charIndex - lineStart }; + } + + lineStart = lineEnd; + } + + throw new Error( 'Could not determine location of character' ); +} + describe( 'rollup-plugin-commonjs', () => { it( 'converts a basic CommonJS module', () => { return rollup({ @@ -61,12 +82,14 @@ describe( 'rollup-plugin-commonjs', () => { const smc = new SourceMapConsumer( generated.map ); - let loc = smc.originalPositionFor({ line: 5, column: 17 }); // 42 + let generatedLoc = getLocation( generated.code, generated.code.indexOf( '42' ) ); + let loc = smc.originalPositionFor( generatedLoc ); // 42 assert.equal( loc.source, 'samples/sourcemap/foo.js' ); assert.equal( loc.line, 1 ); assert.equal( loc.column, 15 ); - loc = smc.originalPositionFor({ line: 9, column: 8 }); // log + generatedLoc = getLocation( generated.code, generated.code.indexOf( 'log' ) ); + loc = smc.originalPositionFor( generatedLoc ); // log assert.equal( loc.source, 'samples/sourcemap/main.js' ); assert.equal( loc.line, 2 ); assert.equal( loc.column, 8 ); From 103d3ce873c415f762432faf9a785f643d567154 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 7 Jun 2016 12:13:16 -0400 Subject: [PATCH 4/6] make helper module ID invisible to node-resolve etc --- package.json | 4 ++-- src/index.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 617ca9a..104007f 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "mocha": "^2.5.3", "rollup": "^0.26.7", "rollup-plugin-buble": "^0.10.0", - "rollup-plugin-node-resolve": "^1.5.0", + "rollup-plugin-node-resolve": "^1.6.0", "source-map": "^0.5.6" }, "main": "dist/rollup-plugin-commonjs.cjs.js", @@ -29,7 +29,7 @@ "estree-walker": "^0.2.1", "magic-string": "^0.15.0", "resolve": "^1.1.7", - "rollup-pluginutils": "^1.3.1" + "rollup-pluginutils": "^1.5.0" }, "repository": { "type": "git", diff --git a/src/index.js b/src/index.js index d488bf8..3659d76 100644 --- a/src/index.js +++ b/src/index.js @@ -37,7 +37,7 @@ function getCandidates ( resolved, extensions ) { ); } -const HELPERS_ID = '%cjs%'; +const HELPERS_ID = '\0commonjsHelpers'; const HELPERS_NAME = '__commonjsHelpers'; const HELPERS = ` export var commonjsGlobal = typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {} From 98a996adebfb022b095e7ef6d9a93871ed192840 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 7 Jun 2016 12:34:30 -0400 Subject: [PATCH 5/6] update test now that __esModule is generated in Rollup itself --- test/samples/__esModule/main.js | 7 +++++-- test/test.js | 13 +------------ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/test/samples/__esModule/main.js b/test/samples/__esModule/main.js index 135547a..aad08ea 100644 --- a/test/samples/__esModule/main.js +++ b/test/samples/__esModule/main.js @@ -1,2 +1,5 @@ -exports.__esModule = true; -exports.answer = 42; +import * as x from './answer'; + +assert.ok( 'answer' in x ); +assert.ok( 'default' in x ); // TODO is this right? +assert.ok( !( '__esModule' in x ) ); diff --git a/test/test.js b/test/test.js index 3bb68df..9824db0 100644 --- a/test/test.js +++ b/test/test.js @@ -199,18 +199,7 @@ describe( 'rollup-plugin-commonjs', () => { return rollup({ entry: 'samples/__esModule/main.js', plugins: [ commonjs() ] - }).then( bundle => { - const generated = bundle.generate({ - format: 'cjs' - }); - - const fn = new Function ( 'module', 'exports', generated.code ); - let module = { exports: {} }; - - fn( module, module.exports ); - - assert.ok( !module.exports.__esModule ); - }); + }).then( executeBundle ); }); it( 'allows named exports to be added explicitly via config', () => { From a18bbc7c072801d1eafbfa104a7301ea1647d7ce Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 7 Jun 2016 14:41:32 -0400 Subject: [PATCH 6/6] guarantee name is deconflicted --- src/index.js | 13 +++++++++++-- test/samples/deconflict-helpers/main.js | 2 ++ test/test.js | 9 +++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 test/samples/deconflict-helpers/main.js diff --git a/src/index.js b/src/index.js index 3659d76..0c56ccd 100644 --- a/src/index.js +++ b/src/index.js @@ -37,8 +37,15 @@ function getCandidates ( resolved, extensions ) { ); } +function deconflict ( identifier, code ) { + let i = 1; + let deconflicted = identifier; + + while ( ~code.indexOf( deconflicted ) ) deconflicted = `${identifier}_${i++}`; + return deconflicted; +} + const HELPERS_ID = '\0commonjsHelpers'; -const HELPERS_NAME = '__commonjsHelpers'; const HELPERS = ` export var commonjsGlobal = typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {} @@ -122,6 +129,8 @@ export default function commonjs ( options = {} ) { let scopeDepth = 0; + const HELPERS_NAME = deconflict( 'commonjsHelpers', code ); + walk( ast, { enter ( node, parent ) { if ( node.scope ) scope = node.scope; @@ -222,7 +231,7 @@ export default function commonjs ( options = {} ) { const args = `module${uses.exports ? ', exports' : ''}`; - const intro = `\n\nvar ${name} = __commonjsHelpers.createCommonjsModule(function (${args}) {\n`; + const intro = `\n\nvar ${name} = ${HELPERS_NAME}.createCommonjsModule(function (${args}) {\n`; let outro = `\n});\n\nexport default (${name} && typeof ${name} === 'object' && 'default' in ${name} ? ${name}['default'] : ${name});\n`; outro += Object.keys( namedExports ) diff --git a/test/samples/deconflict-helpers/main.js b/test/samples/deconflict-helpers/main.js new file mode 100644 index 0000000..6065c64 --- /dev/null +++ b/test/samples/deconflict-helpers/main.js @@ -0,0 +1,2 @@ +var commonjsHelpers = { commonjsGlobal: 'nope' }; +module.exports = global; diff --git a/test/test.js b/test/test.js index 9824db0..e403a8f 100644 --- a/test/test.js +++ b/test/test.js @@ -266,4 +266,13 @@ describe( 'rollup-plugin-commonjs', () => { assert.equal( global.setImmediate, mod.immediate, generated.code ); }); }); + + it( 'deconflicts helper name', () => { + return rollup({ + entry: 'samples/deconflict-helpers/main.js', + plugins: [ commonjs() ] + }).then( executeBundle ).then( module => { + assert.notEqual( module.exports, 'nope' ); + }); + }) });