fix(esbuild): support compose root dependencies with packages external#13742
fix(esbuild): support compose root dependencies with packages external#13742czubocha merged 2 commits intomainserverless/serverless:mainfrom fix/esbuild-compose-packages-externalserverless/serverless:fix/esbuild-compose-packages-externalCopy head branch name to clipboard
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughExternal esbuild packaging now writes service dependencies for ChangesExternal dependency preparation
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/serverless/lib/plugins/esbuild/index.js`:
- Around line 1195-1197: Update the dependency selection in the package JSON
assembly flow so `packageJson.dependencies` is used only when it contains at
least one key; otherwise fall back to `composePackageJson.dependencies`. Add a
regression test covering a Compose service with an empty dependencies object and
verify root dependencies are included in the archive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c209b06-bc22-4388-9085-2e1ce2843c61
📒 Files selected for processing (3)
docs/sf/providers/aws/guide/building.mdpackages/serverless/lib/plugins/esbuild/index.jspackages/serverless/test/unit/lib/plugins/esbuild/prepare-package-json.test.js
Summary
TypeError: Cannot convert undefined or null to objectcrash inpackages/serverless/lib/plugins/esbuild/index.js(_preparePackageJson) when a service inside a Serverless Compose project has nopackage.jsonof its own (or none with adependenciesfield) and setsbuild.esbuild.packages: externalpackages: external, a service that declares its own dependencies keeps exactly those (unchanged behavior); a service that declares none now falls back to thepackage.jsonnext toserverless-compose.yml, so the deployment archive gets a workingnode_modulesinstead of a crashdocs/sf/providers/aws/guide/building.mdRoot cause
When packaging inside a Compose project,
_preparePackageJsonenters its dependency-processing branch if either the service or the compose-rootpackage.jsondeclares dependencies. Thepackages: externalpath kept only the service's dependencies and skipped initializing the rewrittendependenciesobject, so with no service dependencies the subsequent exclude-cleanup loop (the@aws-sdk/*default is always present) dereferencedundefinedand crashed thepackage/deploycommand.Backward compatibility
The change is scoped to configurations that previously crashed. Every configuration that packaged successfully before produces identical output: outside Compose nothing changes, and inside Compose a service with its own dependencies never consults the compose root under
packages: external. This deliberately includes a service declaring an explicitly empty"dependencies": {}— that configuration packaged successfully before (producing emptynode_modules), so it keeps doing exactly that rather than pulling in the compose-root dependencies; a dedicated unit test pins this contract.Test plan
packages/serverless/test/unit/lib/plugins/esbuild/prepare-package-json.test.js: crash case (compose + no servicepackage.json+packages: external), service-declared dependencies remain exactly as before (compose root not merged), non-composepackages: externalunchanged, bundling-path compose fallback unchangedpackages/serverlessunit suite passes (160 suites, 2893 tests)package.jsonandpackages: external— previously crashed, now packages successfully with the root dependencies innode_modules; the same project with a service-levelpackage.jsonproduces a zip containing only the service's dependencies (root packages absent), matching pre-fix outputHow to test
In a Compose project where only the root (next to
serverless-compose.yml) has apackage.json:Before: crashes with
TypeError: Cannot convert undefined or null to object. After: packaging succeeds and the function zip containsnode_moduleswith the compose-root dependencies.Related
packages: externalpath)Summary by CodeRabbit
esbuildwhen using external packages, ensuring dependencies are correctly included in the deployment archive’snode_modules.dependencies) whenpackages: "external"is enabled.packages: "external"in both Compose and non-Compose scenarios.