From 389b10439ba4626d9bec43c6ad88941f41a764ce Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 5 Jan 2016 18:58:28 -0500 Subject: [PATCH 1/8] =?UTF-8?q?prevent=20stack=20overflow=20with=20mutual?= =?UTF-8?q?=20strong=20dependencies=20=E2=80=93=20fixes=20#425?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Bundle.js | 8 +++++++- test/function/cycles-stack-overflow/_config.js | 3 +++ test/function/cycles-stack-overflow/b.js | 11 +++++++++++ test/function/cycles-stack-overflow/c.js | 13 +++++++++++++ test/function/cycles-stack-overflow/d.js | 12 ++++++++++++ test/function/cycles-stack-overflow/main.js | 3 +++ 6 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 test/function/cycles-stack-overflow/_config.js create mode 100644 test/function/cycles-stack-overflow/b.js create mode 100644 test/function/cycles-stack-overflow/c.js create mode 100644 test/function/cycles-stack-overflow/d.js create mode 100644 test/function/cycles-stack-overflow/main.js diff --git a/src/Bundle.js b/src/Bundle.js index fa53ae6..6bc9acc 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -335,19 +335,25 @@ export default class Bundle { let unordered = ordered; ordered = []; + let placed = blank(); + // unordered is actually semi-ordered, as [ fewer dependencies ... more dependencies ] unordered.forEach( module => { // ensure strong dependencies of `module` that don't strongly depend on `module` go first strongDeps[ module.id ].forEach( place ); function place ( dep ) { - if ( !stronglyDependsOn[ dep.id ][ module.id ] && !~ordered.indexOf( dep ) ) { + if ( placed[ dep.id ] ) return; + placed[ dep.id ] = true; + + if ( !stronglyDependsOn[ dep.id ][ module.id ] ) { strongDeps[ dep.id ].forEach( place ); ordered.push( dep ); } } if ( !~ordered.indexOf( module ) ) { + placed[ module.id ] = true; ordered.push( module ); } }); diff --git a/test/function/cycles-stack-overflow/_config.js b/test/function/cycles-stack-overflow/_config.js new file mode 100644 index 0000000..ea40267 --- /dev/null +++ b/test/function/cycles-stack-overflow/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'does not stack overflow on crazy cyclical dependencies' +}; diff --git a/test/function/cycles-stack-overflow/b.js b/test/function/cycles-stack-overflow/b.js new file mode 100644 index 0000000..df7267a --- /dev/null +++ b/test/function/cycles-stack-overflow/b.js @@ -0,0 +1,11 @@ +import { C } from './c.js'; + +export function B () {}; + +B.prototype = { + c: function () { + return function () { + new C(); + }; + }() +}; diff --git a/test/function/cycles-stack-overflow/c.js b/test/function/cycles-stack-overflow/c.js new file mode 100644 index 0000000..5ee8efd --- /dev/null +++ b/test/function/cycles-stack-overflow/c.js @@ -0,0 +1,13 @@ +import { D } from './d.js'; + +export function C () { + this.x = 'x'; +} + +C.prototype = { + d: function () { + return function () { + new D(); + }; + }() +}; diff --git a/test/function/cycles-stack-overflow/d.js b/test/function/cycles-stack-overflow/d.js new file mode 100644 index 0000000..4abd410 --- /dev/null +++ b/test/function/cycles-stack-overflow/d.js @@ -0,0 +1,12 @@ +import { B } from './b.js'; +import { C } from './c.js'; + +export function D () {}; + +D.prototype = { + c: function () { + return function () { + new C(); + }; + }() +}; diff --git a/test/function/cycles-stack-overflow/main.js b/test/function/cycles-stack-overflow/main.js new file mode 100644 index 0000000..1f1d737 --- /dev/null +++ b/test/function/cycles-stack-overflow/main.js @@ -0,0 +1,3 @@ +import { C } from './c.js'; + +assert.equal( new C().x, 'x' ); From 9d91ecaa71bd0c36a51268579b13f10d9c0786b7 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Mon, 18 Jan 2016 13:08:25 -0500 Subject: [PATCH 2/8] error if namespace is called (#446) --- src/Declaration.js | 2 ++ src/utils/error.js | 9 +++++++++ src/utils/run.js | 16 +++++++++++++++- .../cannot-call-external-namespace/_config.js | 12 ++++++++++++ .../cannot-call-external-namespace/main.js | 2 ++ .../cannot-call-internal-namespace/_config.js | 12 ++++++++++++ .../cannot-call-internal-namespace/foo.js | 1 + .../cannot-call-internal-namespace/main.js | 2 ++ 8 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 src/utils/error.js create mode 100644 test/function/cannot-call-external-namespace/_config.js create mode 100644 test/function/cannot-call-external-namespace/main.js create mode 100644 test/function/cannot-call-internal-namespace/_config.js create mode 100644 test/function/cannot-call-internal-namespace/foo.js create mode 100644 test/function/cannot-call-internal-namespace/main.js diff --git a/src/Declaration.js b/src/Declaration.js index 831a966..2aca2a8 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -134,6 +134,7 @@ export class SyntheticDefaultDeclaration { export class SyntheticNamespaceDeclaration { constructor ( module ) { + this.isNamespace = true; this.module = module; this.name = null; @@ -221,6 +222,7 @@ export class ExternalDeclaration { this.module = module; this.name = name; this.isExternal = true; + this.isNamespace = name === '*'; } addAlias () { diff --git a/src/utils/error.js b/src/utils/error.js new file mode 100644 index 0000000..5c0a703 --- /dev/null +++ b/src/utils/error.js @@ -0,0 +1,9 @@ +export default function error ( props ) { + const err = new Error( props.message ); + + Object.keys( props ).forEach( key => { + err[ key ] = props[ key ]; + }); + + throw err; +} diff --git a/src/utils/run.js b/src/utils/run.js index bf35a73..4470305 100644 --- a/src/utils/run.js +++ b/src/utils/run.js @@ -3,6 +3,8 @@ import modifierNodes, { isModifierNode } from '../ast/modifierNodes.js'; import isReference from '../ast/isReference.js'; import flatten from '../ast/flatten'; import pureFunctions from './pureFunctions.js'; +import getLocation from './getLocation.js'; +import error from './error.js'; function call ( callee, scope, statement, strongDependencies ) { while ( callee.type === 'ParenthesizedExpression' ) callee = callee.expression; @@ -11,7 +13,19 @@ function call ( callee, scope, statement, strongDependencies ) { const declaration = scope.findDeclaration( callee.name ) || statement.module.trace( callee.name ); - if ( declaration ) return declaration.run( strongDependencies ); + if ( declaration ) { + if ( declaration.isNamespace ) { + error({ + message: `Cannot call a namespace ('${callee.name}')`, + file: statement.module.id, + pos: callee.start, + loc: getLocation( statement.module.code, callee.start ) + }); + } + + return declaration.run( strongDependencies ); + } + return !pureFunctions[ callee.name ]; } diff --git a/test/function/cannot-call-external-namespace/_config.js b/test/function/cannot-call-external-namespace/_config.js new file mode 100644 index 0000000..fa1e422 --- /dev/null +++ b/test/function/cannot-call-external-namespace/_config.js @@ -0,0 +1,12 @@ +var path = require( 'path' ); +var assert = require( 'assert' ); + +module.exports = { + description: 'errors if code calls an external namespace', + error: function ( err ) { + assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); + assert.equal( err.file, path.resolve( __dirname, 'main.js' ) ); + assert.equal( err.pos, 28 ); + assert.deepEqual( err.loc, { line: 2, column: 0 }); + } +}; diff --git a/test/function/cannot-call-external-namespace/main.js b/test/function/cannot-call-external-namespace/main.js new file mode 100644 index 0000000..c1ad7c6 --- /dev/null +++ b/test/function/cannot-call-external-namespace/main.js @@ -0,0 +1,2 @@ +import * as foo from 'foo'; +foo(); diff --git a/test/function/cannot-call-internal-namespace/_config.js b/test/function/cannot-call-internal-namespace/_config.js new file mode 100644 index 0000000..cf58000 --- /dev/null +++ b/test/function/cannot-call-internal-namespace/_config.js @@ -0,0 +1,12 @@ +var path = require( 'path' ); +var assert = require( 'assert' ); + +module.exports = { + description: 'errors if code calls an internal namespace', + error: function ( err ) { + assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); + assert.equal( err.file, path.resolve( __dirname, 'main.js' ) ); + assert.equal( err.pos, 33 ); + assert.deepEqual( err.loc, { line: 2, column: 0 }); + } +}; diff --git a/test/function/cannot-call-internal-namespace/foo.js b/test/function/cannot-call-internal-namespace/foo.js new file mode 100644 index 0000000..cc798ff --- /dev/null +++ b/test/function/cannot-call-internal-namespace/foo.js @@ -0,0 +1 @@ +export const a = 1; diff --git a/test/function/cannot-call-internal-namespace/main.js b/test/function/cannot-call-internal-namespace/main.js new file mode 100644 index 0000000..74f0acb --- /dev/null +++ b/test/function/cannot-call-internal-namespace/main.js @@ -0,0 +1,2 @@ +import * as foo from './foo.js'; +foo(); From 29f7732f5f8ee64c75eb6400c5911945eb9a5228 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Mon, 18 Jan 2016 13:18:11 -0500 Subject: [PATCH 3/8] ugh windows --- test/function/cannot-call-external-namespace/_config.js | 2 +- test/function/cannot-call-internal-namespace/_config.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/function/cannot-call-external-namespace/_config.js b/test/function/cannot-call-external-namespace/_config.js index fa1e422..2357d6a 100644 --- a/test/function/cannot-call-external-namespace/_config.js +++ b/test/function/cannot-call-external-namespace/_config.js @@ -5,7 +5,7 @@ module.exports = { description: 'errors if code calls an external namespace', error: function ( err ) { assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); - assert.equal( err.file, path.resolve( __dirname, 'main.js' ) ); + assert.equal( err.file, path.resolve( __dirname, 'main.js' ).replace( /\//g, path.sep ) ); assert.equal( err.pos, 28 ); assert.deepEqual( err.loc, { line: 2, column: 0 }); } diff --git a/test/function/cannot-call-internal-namespace/_config.js b/test/function/cannot-call-internal-namespace/_config.js index cf58000..08fbf97 100644 --- a/test/function/cannot-call-internal-namespace/_config.js +++ b/test/function/cannot-call-internal-namespace/_config.js @@ -5,7 +5,7 @@ module.exports = { description: 'errors if code calls an internal namespace', error: function ( err ) { assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); - assert.equal( err.file, path.resolve( __dirname, 'main.js' ) ); + assert.equal( err.file, path.resolve( __dirname, 'main.js' ).replace( /\//g, path.sep ) ); assert.equal( err.pos, 33 ); assert.deepEqual( err.loc, { line: 2, column: 0 }); } From a20fc4122feb5d22eb8a0daf8271c1adf0fef36e Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Mon, 18 Jan 2016 14:44:28 -0500 Subject: [PATCH 4/8] gah --- test/function/cannot-call-external-namespace/_config.js | 2 +- test/function/cannot-call-internal-namespace/_config.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/function/cannot-call-external-namespace/_config.js b/test/function/cannot-call-external-namespace/_config.js index 2357d6a..263c14a 100644 --- a/test/function/cannot-call-external-namespace/_config.js +++ b/test/function/cannot-call-external-namespace/_config.js @@ -5,7 +5,7 @@ module.exports = { description: 'errors if code calls an external namespace', error: function ( err ) { assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); - assert.equal( err.file, path.resolve( __dirname, 'main.js' ).replace( /\//g, path.sep ) ); + assert.equal( err.file.replace( /\//g, path.sep ), path.resolve( __dirname, 'main.js' ) ); assert.equal( err.pos, 28 ); assert.deepEqual( err.loc, { line: 2, column: 0 }); } diff --git a/test/function/cannot-call-internal-namespace/_config.js b/test/function/cannot-call-internal-namespace/_config.js index 08fbf97..ceb42c3 100644 --- a/test/function/cannot-call-internal-namespace/_config.js +++ b/test/function/cannot-call-internal-namespace/_config.js @@ -5,7 +5,7 @@ module.exports = { description: 'errors if code calls an internal namespace', error: function ( err ) { assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); - assert.equal( err.file, path.resolve( __dirname, 'main.js' ).replace( /\//g, path.sep ) ); + assert.equal( err.file.replace( /\//g, path.sep ), path.resolve( __dirname, 'main.js' ) ); assert.equal( err.pos, 33 ); assert.deepEqual( err.loc, { line: 2, column: 0 }); } From d99a89de578dc51178b07ba602424690fee1a4b8 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Mon, 18 Jan 2016 22:18:22 -0500 Subject: [PATCH 5/8] provide informative error if rollup is used in browser without custom resolveId/load functions --- src/utils/defaults.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils/defaults.js b/src/utils/defaults.js index 4753d1b..70c36fe 100644 --- a/src/utils/defaults.js +++ b/src/utils/defaults.js @@ -16,6 +16,8 @@ function addJsExtensionIfNecessary ( file ) { } export function resolveId ( importee, importer ) { + if ( typeof process === 'undefined' ) throw new Error( `It looks like you're using Rollup in a non-Node.js environment. This means you must supply a plugin with custom resolveId and load functions. See https://github.com/rollup/rollup/wiki/Plugins for more information` ); + // absolute paths are left untouched if ( isAbsolute( importee ) ) return addJsExtensionIfNecessary( importee ); From 98e4adb3226bdd70a629050d0e4bd674578c462b Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Tue, 19 Jan 2016 23:37:18 -0800 Subject: [PATCH 6/8] use an interopRequire function for external CJS deps with a default export --- src/finalisers/cjs.js | 19 +++++++++++++------ .../_expected/cjs.js | 5 +++-- test/form/external-imports/_expected/cjs.js | 11 ++++++----- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/finalisers/cjs.js b/src/finalisers/cjs.js index 3a56ac9..4651409 100644 --- a/src/finalisers/cjs.js +++ b/src/finalisers/cjs.js @@ -3,17 +3,24 @@ import getExportBlock from './shared/getExportBlock.js'; export default function cjs ( bundle, magicString, { exportMode }, options ) { let intro = options.useStrict === false ? `` : `'use strict';\n\n`; + const hasDefaultImport = bundle.externalModules.some( mod => mod.declarations.default); + + if (hasDefaultImport) { + intro += `function _interopRequire (id) { var ex = require(id); return 'default' in ex ? ex['default'] : ex; }\n\n`; + } + // TODO handle empty imports, once they're supported const importBlock = bundle.externalModules .map( module => { - let requireStatement = `var ${module.name} = require('${module.id}');`; - if ( module.declarations.default ) { - requireStatement += '\n' + ( module.exportsNames ? `var ${module.name}__default = ` : `${module.name} = ` ) + - `'default' in ${module.name} ? ${module.name}['default'] : ${module.name};`; + let importStatement = `var ${module.name} = _interopRequire('${module.id}');`; + if (module.exportsNames) { + importStatement += `\nvar ${module.name}__default = ${module.name};`; + } + return importStatement; + } else { + return `var ${module.name} = require('${module.id}');`; } - - return requireStatement; }) .join( '\n' ); diff --git a/test/form/external-imports-custom-names/_expected/cjs.js b/test/form/external-imports-custom-names/_expected/cjs.js index e83d17d..7c31c60 100644 --- a/test/form/external-imports-custom-names/_expected/cjs.js +++ b/test/form/external-imports-custom-names/_expected/cjs.js @@ -1,7 +1,8 @@ 'use strict'; -var $ = require('jquery'); -$ = 'default' in $ ? $['default'] : $; +function _interopRequire (id) { var ex = require(id); return 'default' in ex ? ex['default'] : ex; } + +var $ = _interopRequire('jquery'); $( function () { $( 'body' ).html( '

hello world!

' ); diff --git a/test/form/external-imports/_expected/cjs.js b/test/form/external-imports/_expected/cjs.js index 6c20bcc..6cfae7f 100644 --- a/test/form/external-imports/_expected/cjs.js +++ b/test/form/external-imports/_expected/cjs.js @@ -1,14 +1,15 @@ 'use strict'; -var factory = require('factory'); -factory = 'default' in factory ? factory['default'] : factory; +function _interopRequire (id) { var ex = require(id); return 'default' in ex ? ex['default'] : ex; } + +var factory = _interopRequire('factory'); var baz = require('baz'); var containers = require('shipping-port'); -var alphabet = require('alphabet'); -var alphabet__default = 'default' in alphabet ? alphabet['default'] : alphabet; +var alphabet = _interopRequire('alphabet'); +var alphabet__default = alphabet; factory( null ); baz.foo( baz.bar, containers.port ); containers.forEach( console.log, console ); console.log( alphabet.a ); -console.log( alphabet__default.length ); +console.log( alphabet__default.length ); \ No newline at end of file From 1f3488dcfdb059a53fa24aaef827bbdcae19ab6e Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Wed, 20 Jan 2016 14:59:48 -0800 Subject: [PATCH 7/8] add _interopDefault to globals, fix behavior or module__default, make cjs work with browserify --- src/Bundle.js | 2 +- src/finalisers/cjs.js | 9 +++++---- test/form/external-imports-custom-names/_expected/cjs.js | 6 +++--- test/form/external-imports/_expected/cjs.js | 8 ++++---- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index accd9ae..c6a2386 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -63,7 +63,7 @@ export default class Bundle { this.onwarn = options.onwarn || makeOnwarn(); // TODO strictly speaking, this only applies with non-ES6, non-default-only bundles - [ 'module', 'exports' ].forEach( global => this.assumedGlobals[ global ] = true ); + [ 'module', 'exports', '_interopDefault' ].forEach( global => this.assumedGlobals[ global ] = true ); } build () { diff --git a/src/finalisers/cjs.js b/src/finalisers/cjs.js index 4651409..c71f59d 100644 --- a/src/finalisers/cjs.js +++ b/src/finalisers/cjs.js @@ -6,18 +6,19 @@ export default function cjs ( bundle, magicString, { exportMode }, options ) { const hasDefaultImport = bundle.externalModules.some( mod => mod.declarations.default); if (hasDefaultImport) { - intro += `function _interopRequire (id) { var ex = require(id); return 'default' in ex ? ex['default'] : ex; }\n\n`; + intro += `function _interopDefault (ex) { return 'default' in ex ? ex['default'] : ex; }\n\n`; } // TODO handle empty imports, once they're supported const importBlock = bundle.externalModules .map( module => { if ( module.declarations.default ) { - let importStatement = `var ${module.name} = _interopRequire('${module.id}');`; if (module.exportsNames) { - importStatement += `\nvar ${module.name}__default = ${module.name};`; + return `var ${module.name} = require('${module.id}');` + + `\nvar ${module.name}__default = _interopDefault(${module.name});`; + } else { + return `var ${module.name} = _interopDefault(require('${module.id}'));`; } - return importStatement; } else { return `var ${module.name} = require('${module.id}');`; } diff --git a/test/form/external-imports-custom-names/_expected/cjs.js b/test/form/external-imports-custom-names/_expected/cjs.js index 7c31c60..8f6506e 100644 --- a/test/form/external-imports-custom-names/_expected/cjs.js +++ b/test/form/external-imports-custom-names/_expected/cjs.js @@ -1,9 +1,9 @@ 'use strict'; -function _interopRequire (id) { var ex = require(id); return 'default' in ex ? ex['default'] : ex; } +function _interopDefault (ex) { return 'default' in ex ? ex['default'] : ex; } -var $ = _interopRequire('jquery'); +var $ = _interopDefault(require('jquery')); $( function () { $( 'body' ).html( '

hello world!

' ); -}); +}); \ No newline at end of file diff --git a/test/form/external-imports/_expected/cjs.js b/test/form/external-imports/_expected/cjs.js index 6cfae7f..a3ee9b0 100644 --- a/test/form/external-imports/_expected/cjs.js +++ b/test/form/external-imports/_expected/cjs.js @@ -1,12 +1,12 @@ 'use strict'; -function _interopRequire (id) { var ex = require(id); return 'default' in ex ? ex['default'] : ex; } +function _interopDefault (ex) { return 'default' in ex ? ex['default'] : ex; } -var factory = _interopRequire('factory'); +var factory = _interopDefault(require('factory')); var baz = require('baz'); var containers = require('shipping-port'); -var alphabet = _interopRequire('alphabet'); -var alphabet__default = alphabet; +var alphabet = require('alphabet'); +var alphabet__default = _interopDefault(alphabet); factory( null ); baz.foo( baz.bar, containers.port ); From c750730d9ba2ba3395e28c1a47dc8c8c27019c67 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Thu, 21 Jan 2016 12:35:30 -0500 Subject: [PATCH 8/8] -> 0.25.1 --- CHANGELOG.md | 9 +++++++++ package.json | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0e512e..009ccd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # rollup changelog +## 0.25.1 + +* Throw error if namespace is called ([#446](https://github.com/rollup/rollup/issues/446)) +* Prevent shadowing bug in ES6 output ([#441](https://github.com/rollup/rollup/pull/441)) +* Prevent `var exports.foo` ([#426](https://github.com/rollup/rollup/issues/426)) +* Prevent double export of aliased symbols ([#438](https://github.com/rollup/rollup/issues/438)) +* Provide more informative error if Rollup is used in-browser without appropriate `resolveId`/`load` hooks ([#275](https://github.com/rollup/rollup/issues/275)) +* Use `_interopDefault` function to DRY out code with many external dependencies, in CommonJS output ([#458](https://github.com/rollup/rollup/pull/458)) + ## 0.25.0 * **breaking** Module order is determined according to spec, rather than in a way designed to prevent runtime errors. Rollup will warn if it detects runtime errors are likely ([#435](https://github.com/rollup/rollup/issues/435)) diff --git a/package.json b/package.json index 34e7ed8..1d8bdc0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rollup", - "version": "0.25.0", + "version": "0.25.1", "description": "Next-generation ES6 module bundler", "main": "dist/rollup.js", "jsnext:main": "src/rollup.js",