Browse Source

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

gh-786
Rich-Harris 8 years ago
parent
commit
c36f965e7d
  1. 2
      src/finalisers/amd.js
  2. 4
      src/finalisers/iife.js
  3. 39
      src/finalisers/shared/warnOnBuiltins.js
  4. 3
      src/finalisers/umd.js
  5. 29
      test/test.js

2
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 );

4
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 ) {

39
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` );
}

3
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 );

29
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'`
]);
});
});
});
});

Loading…
Cancel
Save