Merge pull request #749 from rollup/gh-746

indicate source of transform error where possible, and downgrade non-sourcemap-generating transforms to a warning

src/Bundle.js

@@ -1,5 +1,6 @@
 import { Bundle as MagicStringBundle } from 'magic-string';
 import first from './utils/first.js';
+import { find } from './utils/array.js';
 import { blank, forOwn, keys } from './utils/object.js';
 import Module from './Module.js';
 import ExternalModule from './ExternalModule.js';
@@ -52,14 +53,6 @@ export default class Bundle {
 		this.hasLoaders = loaders.length !== 0;
 		this.load = first( loaders.concat( load ) );
 
-		this.transformers = this.plugins
-			.map( plugin => plugin.transform )
-			.filter( Boolean );
-
-		this.bundleTransformers = this.plugins
-			.map( plugin => plugin.transformBundle )
-			.filter( Boolean );
-
 		this.moduleById = new Map();
 		this.modules = [];
 
@@ -200,7 +193,7 @@ export default class Bundle {
 					return this.cachedModules.get( id );
 				}
 
-				return transform( source, id, this.transformers );
+				return transform( source, id, this.plugins );
 			})
 			.then( source => {
 				const { code, originalCode, originalSourceMap, ast, sourceMapChain } = source;
@@ -350,16 +343,16 @@ export default class Bundle {
 		let map = null;
 		let bundleSourcemapChain = [];
 
-		code = transformBundle( code, this.bundleTransformers, bundleSourcemapChain )
+		code = transformBundle( code, this.plugins, bundleSourcemapChain )
 			.replace( new RegExp( `\\/\\/#\\s+${SOURCEMAPPING_URL}=.+\\n?`, 'g' ), '' );
 
 		if ( options.sourceMap ) {
 			let file = options.sourceMapFile || options.dest;
 			if ( file ) file = resolve( typeof process !== 'undefined' ? process.cwd() : '', file );
 
-			if ( this.hasLoaders || this.transformers.length || this.bundleTransformers.length ) {
+			if ( this.hasLoaders || find( this.plugins, plugin => plugin.transform || plugin.transformBundle ) ) {
 				map = magicString.generateMap( {} );
-				map = collapseSourcemaps( file, map, usedModules, bundleSourcemapChain );
+				map = collapseSourcemaps( file, map, usedModules, bundleSourcemapChain, this.onwarn );
 			} else {
 				map = magicString.generateMap({ file, includeContent: true });
 			}

src/utils/array.js

@@ -0,0 +1,7 @@
+export function find ( array, fn ) {
+	for ( let i = 0; i < array.length; i += 1 ) {
+		if ( fn( array[i], i ) ) return array[i];
+	}
+
+	return null;
+}

src/utils/collapseSourcemaps.js

@@ -15,8 +15,6 @@ class Source {
 
 class Link {
 	constructor ( map, sources ) {
-		if ( !map ) throw new Error( 'Cannot generate a sourcemap if non-sourcemap-generating transformers are used' );
-
 		this.sources = sources;
 		this.names = map.names;
 		this.mappings = decode( map.mappings );
@@ -97,7 +95,7 @@ class Link {
 	}
 }
 
-export default function collapseSourcemaps ( file, map, modules, bundleSourcemapChain ) {
+export default function collapseSourcemaps ( file, map, modules, bundleSourcemapChain, onwarn ) {
 	const moduleSources = modules.filter( module => !module.excludeFromSourcemap ).map( module => {
 		let sourceMapChain = module.sourceMapChain;
 
@@ -125,6 +123,15 @@ export default function collapseSourcemaps ( file, map, modules, bundleSourcemap
 		}
 
 		sourceMapChain.forEach( map => {
+			if ( map.missing ) {
+				onwarn( `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 https://github.com/rollup/rollup/wiki/Troubleshooting and the plugin documentation for more information` );
+
+				map = {
+					names: [],
+					mappings: ''
+				};
+			}
+
 			source = new Link( map, [ source ]);
 		});
 

src/utils/transform.js

@@ -1,4 +1,4 @@
-export default function transform ( source, id, transformers ) {
+export default function transform ( source, id, plugins ) {
 	let sourceMapChain = [];
 
 	const originalSourceMap = typeof source.map === 'string' ? JSON.parse( source.map ) : source.map;
@@ -6,9 +6,11 @@ export default function transform ( source, id, transformers ) {
 	let originalCode = source.code;
 	let ast = source.ast;
 
-	return transformers.reduce( ( promise, transformer ) => {
+	return plugins.reduce( ( promise, plugin ) => {
 		return promise.then( previous => {
-			return Promise.resolve( transformer( previous, id ) ).then( result => {
+			if ( !plugin.transform ) return previous;
+
+			return Promise.resolve( plugin.transform( previous, id ) ).then( result => {
 				if ( result == null ) return previous;
 
 				if ( typeof result === 'string' ) {
@@ -23,19 +25,18 @@ export default function transform ( source, id, transformers ) {
 					result.map = JSON.parse( result.map );
 				}
 
-				sourceMapChain.push( result.map );
+				sourceMapChain.push( result.map || { missing: true, plugin: plugin.name }); // lil' bit hacky but it works
 				ast = result.ast;
 
 				return result.code;
 			});
+		}).catch( err => {
+			err.id = id;
+			err.plugin = plugin.name;
+			err.message = `Error transforming ${id}${plugin.name ? ` with '${plugin.name}' plugin` : ''}: ${err.message}`;
+			throw err;
 		});
-
 	}, Promise.resolve( source.code ) )
 
-	.then( code => ({ code, originalCode, originalSourceMap, ast, sourceMapChain }) )
-	.catch( err => {
-		err.id = id;
-		err.message = `Error loading ${id}: ${err.message}`;
-		throw err;
-	});
+	.then( code => ({ code, originalCode, originalSourceMap, ast, sourceMapChain }) );
 }

src/utils/transformBundle.js

@@ -1,6 +1,16 @@
-export default function transformBundle ( code, transformers, sourceMapChain ) {
-	return transformers.reduce( ( code, transformer ) => {
-		let result = transformer( code );
+export default function transformBundle ( code, plugins, sourceMapChain ) {
+	return plugins.reduce( ( code, plugin ) => {
+		if ( !plugin.transformBundle ) return code;
+
+		let result;
+
+		try {
+			result = plugin.transformBundle( code );
+		} catch ( err ) {
+			err.plugin = plugin.name;
+			err.message = `Error transforming bundle${plugin.name ? ` with '${plugin.name}' plugin` : ''}: ${err.message}`;
+			throw err;
+		}
 
 		if ( result == null ) return code;
 

test/sourcemaps/transform-without-sourcemap/_config.js

@@ -0,0 +1,26 @@
+const assert = require( 'assert' );
+
+let warnings = [];
+
+module.exports = {
+	description: 'preserves sourcemap chains when transforming',
+	before: () => warnings = [], // reset
+	options: {
+		plugins: [
+			{
+				name: 'fake plugin',
+				transform: function ( code ) {
+					return code;
+				}
+			}
+		],
+		onwarn ( msg ) {
+			warnings.push( msg );
+		}
+	},
+	test: () => {
+		assert.deepEqual( warnings, [
+			`Sourcemap is likely to be incorrect: a plugin ('fake plugin') was used to transform files, but didn't generate a sourcemap for the transformation. Consult https://github.com/rollup/rollup/wiki/Troubleshooting and the plugin documentation for more information`
+		]);
+	}
+};

test/sourcemaps/transform-without-sourcemap/main.js

@@ -0,0 +1 @@
+console.log( 42 );

test/test.js

@@ -358,6 +358,7 @@ describe( 'rollup', function () {
 
 							bundle.write( options );
 
+							if ( config.before ) config.before();
 							var result = bundle.generate( options );
 							config.test( result.code, result.map );
 						});