From c36f965e7de5605e8931c1d113628e12890f3825 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Thu, 29 Dec 2016 08:28:39 -0500 Subject: [PATCH] warn about using node built-ins in browser bundle (#1051) --- src/finalisers/amd.js | 2 ++ src/finalisers/iife.js | 4 ++- src/finalisers/shared/warnOnBuiltins.js | 39 +++++++++++++++++++++++++ src/finalisers/umd.js | 3 ++ test/test.js | 29 ++++++++++++++++-- 5 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 src/finalisers/shared/warnOnBuiltins.js diff --git a/src/finalisers/amd.js b/src/finalisers/amd.js index 70b404d..efe1a8e 100644 --- a/src/finalisers/amd.js +++ b/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 ); diff --git a/src/finalisers/iife.js b/src/finalisers/iife.js index 60105c1..f6d702e 100644 --- a/src/finalisers/iife.js +++ b/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 ) { diff --git a/src/finalisers/shared/warnOnBuiltins.js b/src/finalisers/shared/warnOnBuiltins.js new file mode 100644 index 0000000..e5ac1ba --- /dev/null +++ b/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` ); +} diff --git a/src/finalisers/umd.js b/src/finalisers/umd.js index 279226d..c176242 100644 --- a/src/finalisers/umd.js +++ b/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 ); diff --git a/test/test.js b/test/test.js index 8b2b66f..50f63e8 100644 --- a/test/test.js +++ b/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'` + ]); + }); + }); + }); });