From 29e13abc59e6316db43a24367d21817b2b8c0533 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 15 Jul 2018 18:48:52 -0700 Subject: [PATCH] Support optionally bundling node_modules for --target=node (#1690) --- packages/core/parcel/src/Bundler.js | 4 + packages/core/parcel/src/Resolver.js | 17 +++- packages/core/parcel/src/builtins/index.js | 11 +-- packages/core/parcel/src/cli.js | 12 +++ .../core/parcel/src/visitors/dependencies.js | 8 +- .../resolve-entries/browser-multiple.js | 9 -- .../integration/resolve-entries/browser.js | 4 - .../pkg-browser-multiple/node-entry.js | 3 + .../pkg-browser-multiple/package.json | 4 +- ...ojected-module.js => projected-browser.js} | 0 .../pkg-browser-multiple/projected.js | 3 + .../pkg-browser/node-module.js | 3 + .../resolve-entries/pkg-browser/package.json | 2 +- packages/core/parcel/test/javascript.js | 84 ++++++++++++++++++- packages/core/parcel/test/resolver.js | 61 ++++++++++++++ 15 files changed, 198 insertions(+), 27 deletions(-) create mode 100644 packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/node-entry.js rename packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/{projected-module.js => projected-browser.js} (100%) create mode 100644 packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected.js create mode 100644 packages/core/parcel/test/integration/resolve-entries/pkg-browser/node-module.js diff --git a/packages/core/parcel/src/Bundler.js b/packages/core/parcel/src/Bundler.js index 0623095b941..8850c10aeaf 100644 --- a/packages/core/parcel/src/Bundler.js +++ b/packages/core/parcel/src/Bundler.js @@ -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, diff --git a/packages/core/parcel/src/Resolver.js b/packages/core/parcel/src/Resolver.js index 24834f58d69..ab96d494ee7 100755 --- a/packages/core/parcel/src/Resolver.js +++ b/packages/core/parcel/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'); @@ -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]}; } @@ -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]; } @@ -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 ); } diff --git a/packages/core/parcel/src/builtins/index.js b/packages/core/parcel/src/builtins/index.js index 4ec52fa3e0f..5a582c7c624 100644 --- a/packages/core/parcel/src/builtins/index.js +++ b/packages/core/parcel/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'); diff --git a/packages/core/parcel/src/cli.js b/packages/core/parcel/src/cli.js index 510a13c2449..1f57461b6e3 100755 --- a/packages/core/parcel/src/cli.js +++ b/packages/core/parcel/src/cli.js @@ -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 ', @@ -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 ', 'set the log level, either "0" (no output), "1" (errors), "2" (warnings + errors) or "3" (all).', @@ -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' diff --git a/packages/core/parcel/src/visitors/dependencies.js b/packages/core/parcel/src/visitors/dependencies.js index 4f04a87baa9..fb2db76948d 100644 --- a/packages/core/parcel/src/visitors/dependencies.js +++ b/packages/core/parcel/src/visitors/dependencies.js @@ -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)'); @@ -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; } diff --git a/packages/core/parcel/test/integration/resolve-entries/browser-multiple.js b/packages/core/parcel/test/integration/resolve-entries/browser-multiple.js index c45bb19d085..323f28352be 100644 --- a/packages/core/parcel/test/integration/resolve-entries/browser-multiple.js +++ b/packages/core/parcel/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} diff --git a/packages/core/parcel/test/integration/resolve-entries/browser.js b/packages/core/parcel/test/integration/resolve-entries/browser.js index 2bce4d5ab85..81a260da4b1 100644 --- a/packages/core/parcel/test/integration/resolve-entries/browser.js +++ b/packages/core/parcel/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 diff --git a/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/node-entry.js b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/node-entry.js new file mode 100644 index 00000000000..0ec85aaabec --- /dev/null +++ b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/node-entry.js @@ -0,0 +1,3 @@ +export function test() { + return 'pkg-browser-multiple main-entry' +} diff --git a/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/package.json b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/package.json index 15a3ab99b29..c4d569812f1 100644 --- a/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/package.json +++ b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/package.json @@ -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" } } \ No newline at end of file diff --git a/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected-module.js b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected-browser.js similarity index 100% rename from packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected-module.js rename to packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected-browser.js diff --git a/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected.js b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected.js new file mode 100644 index 00000000000..3a82f56d6ec --- /dev/null +++ b/packages/core/parcel/test/integration/resolve-entries/pkg-browser-multiple/projected.js @@ -0,0 +1,3 @@ +export function test() { + return 'pkg-main-multiple' +} diff --git a/packages/core/parcel/test/integration/resolve-entries/pkg-browser/node-module.js b/packages/core/parcel/test/integration/resolve-entries/pkg-browser/node-module.js new file mode 100644 index 00000000000..4ed818ae364 --- /dev/null +++ b/packages/core/parcel/test/integration/resolve-entries/pkg-browser/node-module.js @@ -0,0 +1,3 @@ +export function test() { + return 'pkg-main' +} diff --git a/packages/core/parcel/test/integration/resolve-entries/pkg-browser/package.json b/packages/core/parcel/test/integration/resolve-entries/pkg-browser/package.json index c8610ec13f6..7a42627e814 100644 --- a/packages/core/parcel/test/integration/resolve-entries/pkg-browser/package.json +++ b/packages/core/parcel/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" } \ No newline at end of file diff --git a/packages/core/parcel/test/javascript.js b/packages/core/parcel/test/javascript.js index befa3ec47d3..5393296ad02 100644 --- a/packages/core/parcel/test/javascript.js +++ b/packages/core/parcel/test/javascript.js @@ -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'); @@ -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' @@ -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: [ @@ -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' diff --git a/packages/core/parcel/test/resolver.js b/packages/core/parcel/test/resolver.js index 467a569900b..ed1fc739569 100644 --- a/packages/core/parcel/test/resolver.js +++ b/packages/core/parcel/test/resolver.js @@ -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() { @@ -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', @@ -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',