Skip to content

Commit

Permalink
Support optionally bundling node_modules for --target=node (#1690)
Browse files Browse the repository at this point in the history
  • Loading branch information
devongovett committed Jul 16, 2018
1 parent 871ffc7 commit 5355685
Show file tree
Hide file tree
Showing 15 changed files with 198 additions and 27 deletions.
4 changes: 4 additions & 0 deletions src/Bundler.js
Expand Up @@ -112,6 +112,10 @@ class Bundler extends EventEmitter {
minify:
typeof options.minify === 'boolean' ? options.minify : isProduction,
target: target,
bundleNodeModules:
typeof options.bundleNodeModules === 'boolean'
? options.bundleNodeModules
: target === 'browser',
hmr: hmr,
https: options.https || false,
logLevel: isNaN(options.logLevel) ? 3 : options.logLevel,
Expand Down
17 changes: 13 additions & 4 deletions src/Resolver.js
@@ -1,4 +1,5 @@
const builtins = require('./builtins');
const nodeBuiltins = require('node-libs-browser');
const path = require('path');
const isGlob = require('is-glob');
const fs = require('./utils/fs');
Expand Down Expand Up @@ -158,6 +159,10 @@ class Resolver {

async findNodeModulePath(filename, dir) {
if (builtins[filename]) {
if (this.options.target === 'node' && filename in nodeBuiltins) {
throw new Error('Cannot resolve builtin module for node target');
}

return {filePath: builtins[filename]};
}

Expand Down Expand Up @@ -265,10 +270,14 @@ class Resolver {
return pkg;
}

getPackageMain(pkg) {
let {browser} = pkg;
getBrowserField(pkg) {
let target = this.options.target || 'browser';
return target === 'browser' ? pkg.browser : null;
}

if (typeof browser === 'object' && browser[pkg.name]) {
getPackageMain(pkg) {
let browser = this.getBrowserField(pkg);
if (browser && typeof browser === 'object' && browser[pkg.name]) {
browser = browser[pkg.name];
}

Expand Down Expand Up @@ -332,7 +341,7 @@ class Resolver {
return (
this.getAlias(filename, pkg.pkgdir, pkg.source) ||
this.getAlias(filename, pkg.pkgdir, pkg.alias) ||
this.getAlias(filename, pkg.pkgdir, pkg.browser) ||
this.getAlias(filename, pkg.pkgdir, this.getBrowserField(pkg)) ||
filename
);
}
Expand Down
11 changes: 6 additions & 5 deletions src/builtins/index.js
@@ -1,9 +1,10 @@
var builtins = require('node-libs-browser');
var nodeBuiltins = require('node-libs-browser');

for (var key in builtins) {
if (builtins[key] == null) {
builtins[key] = require.resolve('./_empty.js');
}
var builtins = Object.create(null);
for (var key in nodeBuiltins) {
builtins[key] = nodeBuiltins[key] == null
? require.resolve('./_empty.js')
: nodeBuiltins[key];
}

builtins['_bundle_loader'] = require.resolve('./bundle-loader.js');
Expand Down
12 changes: 12 additions & 0 deletions src/cli.js
Expand Up @@ -51,6 +51,10 @@ program
'set the runtime environment, either "node", "browser" or "electron". defaults to "browser"',
/^(node|browser|electron)$/
)
.option(
'--bundle-node-modules',
'force bundling node modules, even on node/electron target'
)
.option('-V, --version', 'output the version number')
.option(
'--log-level <level>',
Expand Down Expand Up @@ -97,6 +101,10 @@ program
'set the runtime environment, either "node", "browser" or "electron". defaults to "browser"',
/^(node|browser|electron)$/
)
.option(
'--bundle-node-modules',
'force bundling node modules, even on node/electron target'
)
.option(
'--log-level <level>',
'set the log level, either "0" (no output), "1" (errors), "2" (warnings + errors) or "3" (all).',
Expand Down Expand Up @@ -133,6 +141,10 @@ program
'set the runtime environment, either "node", "browser" or "electron". defaults to "browser"',
/^(node|browser|electron)$/
)
.option(
'--bundle-node-modules',
'force bundling node modules, even on node/electron target'
)
.option(
'--detailed-report',
'print a detailed build report after a completed build'
Expand Down
8 changes: 7 additions & 1 deletion src/visitors/dependencies.js
Expand Up @@ -4,6 +4,7 @@ const traverse = require('babel-traverse').default;
const urlJoin = require('../utils/urlJoin');
const isURL = require('../utils/is-url');
const matchesPattern = require('./matches-pattern');
const nodeBuiltins = require('node-libs-browser');

const requireTemplate = template('require("_bundle_loader")');
const argTemplate = template('require.resolve(MODULE)');
Expand Down Expand Up @@ -150,7 +151,12 @@ function evaluateExpression(node) {
}

function addDependency(asset, node, opts = {}) {
if (asset.options.target !== 'browser') {
// Don't bundle node builtins
if (asset.options.target === 'node' && node.value in nodeBuiltins) {
return;
}

if (!asset.options.bundleNodeModules) {
const isRelativeImport = /^[/~.]/.test(node.value);
if (!isRelativeImport) return;
}
Expand Down
9 changes: 0 additions & 9 deletions test/integration/resolve-entries/browser-multiple.js
@@ -1,13 +1,4 @@
const projected = require('./pkg-browser-multiple/projected')

if(projected.test() !== 'pkg-browser-multiple') {
throw new Error('Invalid module')
}

const entry = require('./pkg-browser-multiple')

if(entry.test() !== 'pkg-browser-multiple browser-entry') {
throw new Error('Invalid module')
}

export const test = {projected, entry}
4 changes: 0 additions & 4 deletions test/integration/resolve-entries/browser.js
@@ -1,7 +1,3 @@
const required = require('./pkg-browser')

if(required.test() !== 'pkg-browser') {
throw new Error('Invalid module')
}

export const test = required.test
@@ -0,0 +1,3 @@
export function test() {
return 'pkg-browser-multiple main-entry'
}
@@ -1,8 +1,8 @@
{
"name": "pkg-browser-multiple",
"main": "./does-not-exist.js",
"main": "./node-entry.js",
"browser": {
"./projected.js": "./projected-module.js",
"./projected.js": "./projected-browser.js",
"pkg-browser-multiple": "browser-entry.js"
}
}
@@ -0,0 +1,3 @@
export function test() {
return 'pkg-main-multiple'
}
3 changes: 3 additions & 0 deletions test/integration/resolve-entries/pkg-browser/node-module.js
@@ -0,0 +1,3 @@
export function test() {
return 'pkg-main'
}
2 changes: 1 addition & 1 deletion test/integration/resolve-entries/pkg-browser/package.json
@@ -1,5 +1,5 @@
{
"name": "pkg-browser",
"main": "./does-not-exist.js",
"main": "./node-module.js",
"browser": "./browser-module.js"
}
84 changes: 83 additions & 1 deletion test/javascript.js
Expand Up @@ -85,6 +85,38 @@ describe('javascript', function() {
assert.equal(output(), 7);
});

it('should bundle node_modules on --target=node and --bundle-node-modules', async function() {
let b = await bundle(__dirname + '/integration/node_require/main.js', {
target: 'node',
bundleNodeModules: true
});

await assertBundleTree(b, {
name: 'main.js',
assets: ['main.js', 'local.js', 'index.js']
});

let output = await run(b);
assert.equal(typeof output, 'function');
assert.equal(output(), 3);
});

it('should bundle node_modules on --target=electron and --bundle-node-modules', async function() {
let b = await bundle(__dirname + '/integration/node_require/main.js', {
target: 'electron',
bundleNodeModules: true
});

await assertBundleTree(b, {
name: 'main.js',
assets: ['main.js', 'local.js', 'index.js']
});

let output = await run(b);
assert.equal(typeof output, 'function');
assert.equal(output(), 3);
});

it('should produce a JS bundle with default exports and no imports', async function() {
let b = await bundle(__dirname + '/integration/es6-default-only/index.js');

Expand Down Expand Up @@ -676,6 +708,30 @@ describe('javascript', function() {
assert.equal(output.test(), 'pkg-browser');
});

it('should not resolve the browser field for --target=node', async function() {
let b = await bundle(
__dirname + '/integration/resolve-entries/browser.js',
{
target: 'node'
}
);

await assertBundleTree(b, {
name: 'browser.js',
assets: ['browser.js', 'node-module.js'],
childBundles: [
{
type: 'map'
}
]
});

let output = await run(b);

assert.equal(typeof output.test, 'function');
assert.equal(output.test(), 'pkg-main');
});

it('should resolve advanced browser resolution', async function() {
let b = await bundle(
__dirname + '/integration/resolve-entries/browser-multiple.js'
Expand All @@ -685,7 +741,7 @@ describe('javascript', function() {
name: 'browser-multiple.js',
assets: [
'browser-multiple.js',
'projected-module.js',
'projected-browser.js',
'browser-entry.js'
],
childBundles: [
Expand All @@ -703,6 +759,32 @@ describe('javascript', function() {
assert.equal(output.entry.test(), 'pkg-browser-multiple browser-entry');
});

it('should not resolve advanced browser resolution with --target=node', async function() {
let b = await bundle(
__dirname + '/integration/resolve-entries/browser-multiple.js',
{
target: 'node'
}
);

await assertBundleTree(b, {
name: 'browser-multiple.js',
assets: ['browser-multiple.js', 'node-entry.js', 'projected.js'],
childBundles: [
{
type: 'map'
}
]
});

let {test: output} = await run(b);

assert.equal(typeof output.projected.test, 'function');
assert.equal(typeof output.entry.test, 'function');
assert.equal(output.projected.test(), 'pkg-main-multiple');
assert.equal(output.entry.test(), 'pkg-browser-multiple main-entry');
});

it('should resolve the module field before main', async function() {
let b = await bundle(
__dirname + '/integration/resolve-entries/module-field.js'
Expand Down
61 changes: 61 additions & 0 deletions test/resolver.js
Expand Up @@ -106,6 +106,25 @@ describe('resolver', function() {
path.join(__dirname, '..', 'src', 'builtins', '_empty.js')
);
});

it('should error when resolving node builtin modules with --target=node', async function() {
let resolver = new Resolver({
rootDir,
extensions: {
'.js': true,
'.json': true
},
target: 'node'
});

let threw = false;
try {
await resolver.resolve('zlib', path.join(rootDir, 'foo.js'));
} catch (err) {
threw = true;
}
assert(threw, 'did not throw');
});
});

describe('node_modules', function() {
Expand Down Expand Up @@ -157,6 +176,27 @@ describe('resolver', function() {
assert.equal(resolved.pkg.name, 'package-browser');
});

it('should not resolve a node_modules package.browser main field with --target=node', async function() {
let resolver = new Resolver({
rootDir,
extensions: {
'.js': true,
'.json': true
},
target: 'node'
});

let resolved = await resolver.resolve(
'package-browser',
path.join(rootDir, 'foo.js')
);
assert.equal(
resolved.path,
path.join(rootDir, 'node_modules', 'package-browser', 'main.js')
);
assert.equal(resolved.pkg.name, 'package-browser');
});

it('should fall back to index.js when it cannot find package.main', async function() {
let resolved = await resolver.resolve(
'package-fallback',
Expand Down Expand Up @@ -271,6 +311,27 @@ describe('resolver', function() {
assert.equal(resolved.pkg.name, 'package-browser-alias');
});

it('should not alias using the package.browser field with --target=node', async function() {
let resolver = new Resolver({
rootDir,
extensions: {
'.js': true,
'.json': true
},
target: 'node'
});

let resolved = await resolver.resolve(
'package-browser-alias/foo',
path.join(rootDir, 'foo.js')
);
assert.equal(
resolved.path,
path.join(rootDir, 'node_modules', 'package-browser-alias', 'foo.js')
);
assert.equal(resolved.pkg.name, 'package-browser-alias');
});

it('should alias a sub-file using the package.alias field', async function() {
let resolved = await resolver.resolve(
'package-alias/foo',
Expand Down

0 comments on commit 5355685

Please sign in to comment.