From 89948351d73410d19b0b4d5638bbe80181269cef Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sun, 30 Jun 2024 15:20:45 -0400 Subject: [PATCH] temp --- CHANGELOG.md | 8 ++++++++ Makefile | 1 + lib/shared/types.ts | 2 +- pkg/api/api.go | 1 + pkg/api/api_impl.go | 15 ++++++++++++++- pkg/cli/cli_impl.go | 9 ++++++--- scripts/end-to-end-tests.js | 34 +++++++++++++++++++++++++++++++++- scripts/test-yarnpnp.js | 5 +++-- 8 files changed, 67 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea0d8abcfb5..c0f625a0860 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +* Omit packages from bundle by default when targeting node ([#1874](https://github.com/evanw/esbuild/issues/1874), [#2830](https://github.com/evanw/esbuild/issues/2830), [#2846](https://github.com/evanw/esbuild/issues/2846), [#2915](https://github.com/evanw/esbuild/issues/2915), [#3145](https://github.com/evanw/esbuild/issues/3145), [#3294](https://github.com/evanw/esbuild/issues/3294), [#3323](https://github.com/evanw/esbuild/issues/3323), [#3582](https://github.com/evanw/esbuild/issues/3582), [#3809](https://github.com/evanw/esbuild/issues/3809), [#3815](https://github.com/evanw/esbuild/issues/3815)) + + This breaking change is an experiment. People are commonly confused when using esbuild to bundle code for node (i.e. for `--platform=node`) because some packages may not be intended for bundlers, and may use node-specific features that don't work with a bundler. Even though esbuild's "getting started" instructions say to use `--packages=external` to work around this problem, many people don't read the documentation and don't do this, and are then confused when it doesn't work. So arguably this is a bad default behavior for esbuild to have if people keep tripping over this. + + With this release, esbuild will now omit packages from the bundle by default when the platform is `node` (i.e. the previous behavior of `--packages=external` is now the default in this case). If you don't want this behavior, you can do `--packages=bundle` to allow packages to be included in the bundle (i.e. the previous default behavior). Note that `--packages=bundle` doesn't mean all packages are bundled, just that packages are allowed to be bundled. You can still exclude individual packages from the bundle using `--external:` even when `--packages=bundle` is present. + + The `--packages=` setting considers all import paths that "look like" package imports in the original source code to be packages. Specifically packages that don't start with a path segment of `/` or `.` or `..` are considered to be package imports. The only two exceptions to this rule are [subpath imports](https://nodejs.org/api/packages.html#subpath-imports) (which start with a `#` character) and TypeScript path remappings via `path` and/or `baseUrl` in `tsconfig.json` (which are applied first). + * Drop support for older platforms ([#3802](https://github.com/evanw/esbuild/issues/3802)) This release drops support for the following operating systems: diff --git a/Makefile b/Makefile index 2a3aad6e862..c9d0b7a0e7e 100644 --- a/Makefile +++ b/Makefile @@ -769,6 +769,7 @@ TEST_ROLLUP_REPLACE += "paths": { "package.json": [".\/package.json"] }, TEST_ROLLUP_FLAGS += --bundle TEST_ROLLUP_FLAGS += --external:fsevents TEST_ROLLUP_FLAGS += --outfile=dist/rollup.js +TEST_ROLLUP_FLAGS += --packages=bundle TEST_ROLLUP_FLAGS += --platform=node TEST_ROLLUP_FLAGS += --target=es6 TEST_ROLLUP_FLAGS += src/node-entry.ts diff --git a/lib/shared/types.ts b/lib/shared/types.ts index d5c6ac9efb8..c7053070fe0 100644 --- a/lib/shared/types.ts +++ b/lib/shared/types.ts @@ -125,7 +125,7 @@ export interface BuildOptions extends CommonOptions { /** Documentation: https://esbuild.github.io/api/#external */ external?: string[] /** Documentation: https://esbuild.github.io/api/#packages */ - packages?: 'external' + packages?: 'bundle' | 'external' /** Documentation: https://esbuild.github.io/api/#alias */ alias?: Record /** Documentation: https://esbuild.github.io/api/#loader */ diff --git a/pkg/api/api.go b/pkg/api/api.go index 69ba4193d24..08a597ec2a0 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -180,6 +180,7 @@ type Packages uint8 const ( PackagesDefault Packages = iota + PackagesBundle PackagesExternal ) diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go index e07e901bae5..76320e4a887 100644 --- a/pkg/api/api_impl.go +++ b/pkg/api/api_impl.go @@ -216,6 +216,19 @@ func validateASCIIOnly(value Charset) bool { } } +func validateExternalPackages(value Packages, platform Platform) bool { + switch value { + case PackagesDefault: + return platform == PlatformNode + case PackagesBundle: + return false + case PackagesExternal: + return true + default: + panic("Invalid packages") + } +} + func validateTreeShaking(value TreeShaking, bundle bool, format Format) bool { switch value { case TreeShakingDefault: @@ -1267,7 +1280,7 @@ func validateBuildOptions( ExtensionToLoader: validateLoaders(log, buildOpts.Loader), ExtensionOrder: validateResolveExtensions(log, buildOpts.ResolveExtensions), ExternalSettings: validateExternals(log, realFS, buildOpts.External), - ExternalPackages: buildOpts.Packages == PackagesExternal, + ExternalPackages: validateExternalPackages(buildOpts.Packages, buildOpts.Platform), PackageAliases: validateAlias(log, realFS, buildOpts.Alias), TSConfigPath: validatePath(log, realFS, buildOpts.Tsconfig, "tsconfig path"), TSConfigRaw: buildOpts.TsconfigRaw, diff --git a/pkg/cli/cli_impl.go b/pkg/cli/cli_impl.go index cc98920cc0b..16b5859737c 100644 --- a/pkg/cli/cli_impl.go +++ b/pkg/cli/cli_impl.go @@ -613,12 +613,15 @@ func parseOptionsImpl( case strings.HasPrefix(arg, "--packages=") && buildOpts != nil: value := arg[len("--packages="):] var packages api.Packages - if value == "external" { + switch value { + case "bundle": + packages = api.PackagesBundle + case "external": packages = api.PackagesExternal - } else { + default: return parseOptionsExtras{}, cli_helpers.MakeErrorWithNote( fmt.Sprintf("Invalid value %q in %q", value, arg), - "The only valid value is \"external\".", + "Valid values are \"bundle\" or \"external\".", ) } buildOpts.Packages = packages diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 8508d3981c9..1b08bd9e36b 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -8069,7 +8069,7 @@ for (const flags of [[], ['--bundle']]) { } }`, }), - test(['in.js', '--outfile=node.js', '--bundle', '--platform=node'].concat(flags), { + test(['in.js', '--outfile=node.js', '--bundle', '--platform=node', '--packages=bundle'].concat(flags), { 'in.js': `import abc from 'pkg'; if (abc !== 'module') throw 'fail'`, 'node_modules/pkg/default.js': `module.exports = 'default'`, 'node_modules/pkg/module.js': `export default 'module'`, @@ -8109,6 +8109,38 @@ for (const flags of [[], ['--bundle']]) { }`, }), + // Check the default behavior of "--platform=node" + test(['in.js', '--outfile=node.js', '--bundle', '--platform=node', '--format=esm'].concat(flags), { + 'in.js': `import abc from 'pkg'; if (abc !== 'import') throw 'fail'`, + 'node_modules/pkg/fail.js': `TEST FAILED`, // This package should not be bundled + 'node_modules/pkg/require.cjs': `module.exports = 'require'`, + 'node_modules/pkg/import.mjs': `export default 'import'`, + 'node_modules/pkg/package.json': `{ + "exports": { + ".": { + "module": "./fail.js", + "import": "./import.mjs", + "require": "./require.cjs" + } + } + }`, + }), + test(['in.js', '--outfile=node.js', '--bundle', '--platform=node', '--format=cjs'].concat(flags), { + 'in.js': `import abc from 'pkg'; if (abc !== 'require') throw 'fail'`, + 'node_modules/pkg/fail.js': `TEST FAILED`, // This package should not be bundled + 'node_modules/pkg/require.cjs': `module.exports = 'require'`, + 'node_modules/pkg/import.mjs': `export default 'import'`, + 'node_modules/pkg/package.json': `{ + "exports": { + ".": { + "module": "./fail.js", + "import": "./import.mjs", + "require": "./require.cjs" + } + } + }`, + }), + // This is an edge case for extensionless files. The file should be treated // as CommonJS even though package.json says "type": "module" because that // only applies to ".js" files in node, not to all JavaScript files. diff --git a/scripts/test-yarnpnp.js b/scripts/test-yarnpnp.js index 30ded93400b..01f4ebd34d5 100644 --- a/scripts/test-yarnpnp.js +++ b/scripts/test-yarnpnp.js @@ -66,6 +66,7 @@ function runTests() { 'in.mjs', '--bundle', '--log-level=debug', + '--packages=bundle', '--platform=node', '--outfile=out-native.js', ], { cwd: rootDir, stdio: 'inherit' }) @@ -73,12 +74,12 @@ function runTests() { // Test the WebAssembly build esbuild.buildWasmLib(ESBUILD_BINARY_PATH) - run('node ../../npm/esbuild-wasm/bin/esbuild in.mjs --bundle --log-level=debug --platform=node --outfile=out-wasm.js') + run('node ../../npm/esbuild-wasm/bin/esbuild in.mjs --bundle --log-level=debug --packages=bundle --platform=node --outfile=out-wasm.js') run('node out-wasm.js') // Test the WebAssembly build when run through Yarn's file system shim esbuild.buildWasmLib(ESBUILD_BINARY_PATH) - run('yarn node ../../npm/esbuild-wasm/bin/esbuild in.mjs --bundle --log-level=debug --platform=node --outfile=out-wasm-yarn.js') + run('yarn node ../../npm/esbuild-wasm/bin/esbuild in.mjs --bundle --log-level=debug --packages=bundle --platform=node --outfile=out-wasm-yarn.js') run('node out-wasm-yarn.js') }