Merge pull request #1196 from rollup/gh-1051

warn about using node built-ins in browser bundle (#1051)

src/finalisers/amd.js

@@ -2,8 +2,10 @@ import { getName, quotePath } from '../utils/map-helpers.js';
 import getInteropBlock from './shared/getInteropBlock.js';
 import getExportBlock from './shared/getExportBlock.js';
 import esModuleExport from './shared/esModuleExport.js';
+import warnOnBuiltins from './shared/warnOnBuiltins.js';
 
 export default function amd ( bundle, magicString, { exportMode, indentString, intro, outro }, options ) {
+	warnOnBuiltins( bundle );
 	const deps = bundle.externalModules.map( quotePath );
 	const args = bundle.externalModules.map( getName );
 

src/finalisers/iife.js

@@ -4,6 +4,7 @@ import getInteropBlock from './shared/getInteropBlock.js';
 import getExportBlock from './shared/getExportBlock.js';
 import getGlobalNameMaker from './shared/getGlobalNameMaker.js';
 import propertyStringFor from './shared/propertyStringFor';
+import warnOnBuiltins from './shared/warnOnBuiltins.js';
 
 // thisProp('foo.bar-baz.qux') === "this.foo['bar-baz'].qux"
 const thisProp = propertyStringFor('this');
@@ -29,8 +30,9 @@ export default function iife ( bundle, magicString, { exportMode, indentString,
 	const name = options.moduleName;
 	const isNamespaced = name && ~name.indexOf( '.' );
 
-	const dependencies = bundle.externalModules.map( globalNameMaker );
+	warnOnBuiltins( bundle );
 
+	const dependencies = bundle.externalModules.map( globalNameMaker );
 	const args = bundle.externalModules.map( getName );
 
 	if ( exportMode !== 'none' && !name ) {

src/finalisers/shared/warnOnBuiltins.js

@@ -0,0 +1,39 @@
+const builtins = {
+	process: true,
+	events: true,
+	stream: true,
+	util: true,
+	path: true,
+	buffer: true,
+	querystring: true,
+	url: true,
+	string_decoder: true,
+	punycode: true,
+	http: true,
+	https: true,
+	os: true,
+	assert: true,
+	constants: true,
+	timers: true,
+	console: true,
+	vm: true,
+	zlib: true,
+	tty: true,
+	domain: true
+};
+
+// Creating a browser bundle that depends on Node.js built-in modules ('util'). You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins
+
+export default function warnOnBuiltins ( bundle ) {
+	const externalBuiltins = bundle.externalModules
+		.filter( mod => mod.id in builtins )
+		.map( mod => mod.id );
+
+	if ( !externalBuiltins.length ) return;
+
+	const detail = externalBuiltins.length === 1 ?
+		`module ('${externalBuiltins[0]}')` :
+		`modules (${externalBuiltins.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${externalBuiltins.pop()}')`;
+
+	bundle.onwarn( `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` );
+}

src/finalisers/umd.js

@@ -5,6 +5,7 @@ import getExportBlock from './shared/getExportBlock.js';
 import getGlobalNameMaker from './shared/getGlobalNameMaker.js';
 import esModuleExport from './shared/esModuleExport.js';
 import propertyStringFor from './shared/propertyStringFor.js';
+import warnOnBuiltins from './shared/warnOnBuiltins.js';
 
 // globalProp('foo.bar-baz') === "global.foo['bar-baz']"
 const globalProp = propertyStringFor('global');
@@ -30,6 +31,8 @@ export default function umd ( bundle, magicString, { exportMode, indentString, i
 		throw new Error( 'You must supply options.moduleName for UMD bundles' );
 	}
 
+	warnOnBuiltins( bundle );
+
 	const globalNameMaker = getGlobalNameMaker( options.globals || blank(), bundle.onwarn );
 
 	const amdDeps = bundle.externalModules.map( quotePath );

test/test.js

@@ -49,7 +49,7 @@ function loadConfig ( path ) {
 function loader ( modules ) {
 	return {
 		resolveId ( id ) {
-			return id;
+			return id in modules ? id : null;
 		},
 
 		load ( id ) {
@@ -710,8 +710,6 @@ describe( 'rollup', function () {
 					dest,
 					format: 'cjs'
 				});
-
-
 			}).then( () => {
 				assert.deepEqual( result, [
 					{ a: dest, format: 'cjs' },
@@ -722,4 +720,29 @@ describe( 'rollup', function () {
 			});
 		});
 	});
+
+	describe( 'misc', () => {
+		it( 'warns if node builtins are unresolved in a non-CJS, non-ES bundle (#1051)', () => {
+			const warnings = [];
+
+			return rollup.rollup({
+				entry: 'entry',
+				plugins: [
+					loader({ entry: `import { format } from 'util';\nexport default format( 'this is a %s', 'formatted string' );` })
+				],
+				onwarn: warning => warnings.push( warning )
+			}).then( bundle => {
+				bundle.generate({
+					format: 'iife',
+					moduleName: 'myBundle'
+				});
+
+				assert.deepEqual( warnings, [
+					`'util' is imported by entry, but could not be resolved – treating it as an external dependency. For help see https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency`,
+					`Creating a browser bundle that depends on Node.js built-in module ('util'). You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins`,
+					`No name was provided for external module 'util' in options.globals – guessing 'util'`
+				]);
+			});
+		});
+	});
 });