Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support optionally bundling node_modules for --target=node #1690

Merged
merged 1 commit into from Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -638,6 +670,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 @@ -647,7 +703,7 @@ describe('javascript', function() {
name: 'browser-multiple.js',
assets: [
'browser-multiple.js',
'projected-module.js',
'projected-browser.js',
'browser-entry.js'
],
childBundles: [
Expand All @@ -665,6 +721,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