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.

Sign up for GitHub

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

Jump to bottom

Revert "fix #1874: node defaults to --packages=external" #3820

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Revert "fix #1874: node defaults to --packages=external" #3820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

## Unreleased

* Revert the recent change to avoid bundling dependencies for node ([#3819](https://github.com/evanw/esbuild/issues/3819))

This release reverts the recent change in version 0.22.0 that made `--packages=external` the default behavior with `--platform=node`. The default is now back to `--packages=bundle`.

I've just been made aware that Amazon doesn't pin their dependencies in their "AWS CDK" product, which means that whenever esbuild publishes a new release, many people (potentially everyone?) using their SDK around the world instantly starts using it without Amazon checking that it works first. This change in version 0.22.0 happened to break their SDK. I'm amazed that things haven't broken before this point. This revert attempts to avoid these problems for Amazon's customers. Hopefully Amazon will pin their dependencies in the future.

In addition, this is probably a sign that esbuild is used widely enough that it now needs to switch to a more complicated release model. I may have esbuild use a beta channel model for further development.

* Fix preserving collapsed JSX whitespace ([#3818](https://github.com/evanw/esbuild/issues/3818))

When transformed, certain whitespace inside JSX elements is ignored completely if it collapses to an empty string. However, the whitespace should only be ignored if the JSX is being transformed, not if it's being preserved. This release fixes a bug where esbuild was previously incorrectly ignoring collapsed whitespace with `--jsx=preserve`. Here is an example:
Expand Down
3 changes: 1 addition & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ test-tsc: esbuild | github/tsc
cp -r github/tsc/src github/tsc/scripts demo/tsc
cp github/tsc/lib/*.d.ts demo/tsc/built/local
cd demo/tsc && node scripts/processDiagnosticMessages.mjs src/compiler/diagnosticMessages.json
./esbuild --bundle demo/tsc/src/tsc/tsc.ts --outfile=demo/tsc/built/local/tsc.js --platform=node --target=es2018
./esbuild --bundle demo/tsc/src/tsc/tsc.ts --outfile=demo/tsc/built/local/tsc.js --platform=node --target=es2018 --packages=external
echo '{"dependencies":{"@types/node":"20.2.5","@types/microsoft__typescript-etw":"0.1.1","@types/source-map-support":"0.5.6"}}' > demo/tsc/package.json
cd demo/tsc && npm i --silent && echo 'Type checking tsc using tsc...' && time -p node ./built/local/tsc.js -p src/compiler

Expand All @@ -769,7 +769,6 @@ 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
8 changes: 3 additions & 5 deletions pkg/api/api_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,9 @@ func validateASCIIOnly(value Charset) bool {
}
}

func validateExternalPackages(value Packages, platform Platform) bool {
func validateExternalPackages(value Packages) bool {
switch value {
case PackagesDefault:
return platform == PlatformNode
case PackagesBundle:
case PackagesDefault, PackagesBundle:
return false
case PackagesExternal:
return true
Expand Down Expand Up @@ -1280,7 +1278,7 @@ func validateBuildOptions(
ExtensionToLoader: validateLoaders(log, buildOpts.Loader),
ExtensionOrder: validateResolveExtensions(log, buildOpts.ResolveExtensions),
ExternalSettings: validateExternals(log, realFS, buildOpts.External),
ExternalPackages: validateExternalPackages(buildOpts.Packages, buildOpts.Platform),
ExternalPackages: validateExternalPackages(buildOpts.Packages),
PackageAliases: validateAlias(log, realFS, buildOpts.Alias),
TSConfigPath: validatePath(log, realFS, buildOpts.Tsconfig, "tsconfig path"),
TSConfigRaw: buildOpts.TsconfigRaw,
Expand Down
23 changes: 19 additions & 4 deletions scripts/end-to-end-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8108,9 +8108,7 @@ 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), {
test(['in.js', '--outfile=node.js', '--bundle', '--platform=node', '--packages=external', '--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'`,
Expand All @@ -8125,7 +8123,7 @@ for (const flags of [[], ['--bundle']]) {
}
}`,
}),
test(['in.js', '--outfile=node.js', '--bundle', '--platform=node', '--format=cjs'].concat(flags), {
test(['in.js', '--outfile=node.js', '--bundle', '--platform=node', '--packages=external', '--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'`,
Expand All @@ -8141,6 +8139,23 @@ 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 !== 'module') throw 'fail'`,
'node_modules/pkg/module.js': `export default 'module'`,
'node_modules/pkg/require.cjs': `module.exports = 'require'`,
'node_modules/pkg/import.mjs': `export default 'import'`,
'node_modules/pkg/package.json': `{
"exports": {
".": {
"module": "./module.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
7 changes: 3 additions & 4 deletions scripts/test-yarnpnp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const ESBUILD_BINARY_PATH = esbuild.buildBinary()
const rootDir = path.join(__dirname, '..', 'require', 'yarnpnp')

function run(command) {
console.log('\n\033[37m' + '$ ' + command + '\033[0m')
console.log('\n\x1B[37m' + '$ ' + command + '\x1B[0m')
child_process.execSync(command, { cwd: rootDir, stdio: 'inherit' })
}

Expand Down Expand Up @@ -66,20 +66,19 @@ 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 --packages=bundle --platform=node --outfile=out-wasm.js')
run('node ../../npm/esbuild-wasm/bin/esbuild in.mjs --bundle --log-level=debug --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 --packages=bundle --platform=node --outfile=out-wasm-yarn.js')
run('yarn node ../../npm/esbuild-wasm/bin/esbuild in.mjs --bundle --log-level=debug --platform=node --outfile=out-wasm-yarn.js')
run('node out-wasm-yarn.js')
}

Expand Down
Loading