Skip to content

Commit

Permalink
temp
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jun 30, 2024
1 parent 3f57db8 commit 8994835
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 8 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>
/** Documentation: https://esbuild.github.io/api/#loader */
Expand Down
1 change: 1 addition & 0 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ type Packages uint8

const (
PackagesDefault Packages = iota
PackagesBundle
PackagesExternal
)

Expand Down
15 changes: 14 additions & 1 deletion pkg/api/api_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 6 additions & 3 deletions pkg/cli/cli_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 33 additions & 1 deletion scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'`,
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions scripts/test-yarnpnp.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,20 @@ function runTests() {
'in.mjs',
'--bundle',
'--log-level=debug',
'--packages=bundle',
'--platform=node',
'--outfile=out-native.js',
], { cwd: rootDir, stdio: 'inherit' })
run('node out-native.js')

// 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')
}

Expand Down

0 comments on commit 8994835

Please sign in to comment.