aws-cdk-lib/aws-lambda-nodejs: default packages esbuildArg causes package imports to not be bundled

[ykageyama-mondo]

Describe the bug

With the v0.22 release of esbuild, the default bundling behaviour has been changed to omit imported packages from the bundle by default when the platform is node. The recommendation is to set the packages option to bundle to get the desired bundling behaviour.

Expected Behavior

When executing the lambda, import package from 'somePackage' imports the target package

Current Behavior

When executing the lambda, import package from 'somePackage' throws a Runtime.ImportModuleError with message Cannot find module 'somePackage'

Reproduction Steps

1. Clone https://github.com/ykageyama-mondo/esbuild-issue-recreation
2. Install and deploy
3. Execute deployed lambda

Possible Solution

Add --packages=bundle to the default esbuildCommand created. Tried this on the reproduction lambda but seems to error with Must use "outdir" when there are multiple input files. I'm not familiar enough with esbuild to understand why this is happening and how to fix it.

Additional Information/Context

No response

CDK CLI Version

2.147.2

Framework Version

2.147.2

Node.js Version

18.17.1

OS

Ubuntu 22.04.4 LTS

Language

TypeScript

Language Version

5.5.2

Other information

No response

[ykageyama-mondo]

The temporary solution to this is to pin the version of esbuild to ~0.21.

[ykageyama-mondo]

The README section on modules will also need to be updated to reflect the change in default behaviour

[barendb]

We made the following change and that fixed the issue

Issue reported on esbuild gh as well

https://github.com/evanw/esbuild/issues/3817

nodejs_bundling_options = NodejsBundlingOptions(esbuild_version="0.21.5")

some_lambda = NodejsFunction(
    scope=self,
    ...
    bundling=nodejs_bundling_options,
)

[tobiberger]

Another temporary solution is to add the now required parameter instead of specifying the version. This will keep the old bundling functionality working with the new esbuild version.

In JS/TS this would look somewhat like this:

new NodejsFunction(scope, "my-esbuild-lambda", {
  functionName: "awesome-function",
  ...
  bundling: {
    esbuildArgs: {
      "--packages": "bundle",
    },
  },
})

[mrgrain]

Everyone looking for a temporary fix, I recommend doing this until we have settled on a solution in CDK:

We made the following change and that fixed the issue

nodejs_bundling_options = NodejsBundlingOptions(esbuild_version="0.21.5")

some_lambda = NodejsFunction(
    scope=self,
    ...
    bundling=nodejs_bundling_options,
)

[kristof-kasa]

@tobiberger solution worked for me. <3

[tobiberger]

As there are already some people liking my proposed solution, here's something for your consideration: I personally prefer this approach, because it avoids locking esbuild to an outdated version, which might also be easy to forget and stay like that for longer than intended. However, I expect that a proper fix of the underlying issue by the aws-cdk lib might very well be in conflict with this setting. I guess it will either include packages in the lambda bundle or (hopefully) it will add the --packages=bundle parameter by default, which might lead to errors in builds once we get the cdk update. This is all speculation of course, but if you value a high certainty of perfectly running pipelines over ensuring up to date dependencies, then you should probably go with @mrgrain's approach and hard-code the esbuild version. Just make sure you don't forget to revert when this issue is resolved.

One more thing you should know is that setting the esbuild version in cdk code only works when using docker for bundling. This is the default, so it should work for most people. However, if your cdk project is running in a node environment where esbuild is already installed and you haven't specified the forceDockerBundling bundling option, it will run with the installed version, no matter what you specify. Of course, then it's still your decision whether you lock the esbuild version or add the bundling option.

Edit: I just realized that the fact about using the esbuild version present in a node project can be very useful for large projects, as it provides the opportunity to set the esbuild version for the whole project in one place instead of setting it in every NodejsFunction constructor.

[Ys-Zhou]

Hey, aws guys. This is really a critical issue. I have a huge CDK project and I cannot add --packages=bundle to every NodejsFunction resource. So please set it as the default bundling behavior in CDK or provide us a global option for setting it. I will lock esbuild to 0.21.5 for now but I want to know your decision.

[TheRealAmazonKendra]

We are currently in the process of creating a patch release with the esbuild version pinned to 0.21.5 to get customers out of immediate pain. We will also look at adding the --packages=bundle flag in but we will not include that in the patch release in case there are unintended consequences that also be customer impacting.

[mrgrain]

As there are already some people liking my proposed solution, here's something for your consideration: I personally prefer this approach, because it avoids locking esbuild to an outdated version, which might also be easy to forget and stay like that for longer than intended. However, I expect that a proper fix of the underlying issue by the aws-cdk lib might very well be in conflict with this setting. I guess it will either include packages in the lambda bundle or (hopefully) it will add the --packages=bundle parameter by default, which might lead to errors in builds once we get the cdk update. This is all speculation of course, but if you value a high certainty of perfectly running pipelines over ensuring up to date dependencies, then you should probably go with @mrgrain's approach and hard-code the esbuild version. Just make sure you don't forget to revert when this issue is resolved.

Thank you for the balanced write-up @tobiberger ! My main concern right now is that --packages=bundle is not backwards compatible. So for anyone using a version < 0.22.0, just adding this in will fail the build.

$ touch index.js
$ npx [email protected] index.js --packages=bundle
✘ [ERROR] Invalid value "bundle" in "--packages=bundle"

  The only valid value is "external".

1 error

[elasticrash]

my workaround for Lambdas functions was to pin the version of esbuild in the NodeJsFunction

this.lambda = new NodejsFunction(this,
    StateApi, {
        ....
        bundling: {
            esbuildVersion: "0.21.0",
	}
});

[github-actions[bot]]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

[ykageyama-mondo]

I am not sure if this issue should be marked a closed. The fix to pin the esbuild version in the docker file does not resolve the issue where the local bundling is broken by default for esbuild version 0.22.

[TheRealAmazonKendra]

The closure happened automatically because the linked PR was merged. I'm not sure, however, if we will be able to support a touch-free upgrade to 0.22. We cannot update the default to include --packages=bundle because it will break people with certain configurations. Now that we have the immediate fix released, however, we can dig into this further.

[ykageyama-mondo]

Thanks for re-opening the issue @TheRealAmazonKendra. Would we be able to add the option conditionally when this.esbuildInstallation?.version >= '0.22'? Are there any concerns with this approach?

[github-actions[bot]]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

[dreamorosi]

Esbuild released a new version that reverts the change (0.23), so that it now behaves the same as pre-0.22.

Despite all this, I think CDK should consider pinning to a known working version at least in the docker-based build.

[MightySepp666]

I would also like to share some of my expectations and opinions on this matter.

Despite all this, I think CDK should consider pinning to a known working version at least in the docker-based build.

Absolutely agree. ESBuild doesn't follow semantic versioning and uses minor versions for breaking changes, as stated by the maintainer in #3817:

The major version is 0 so the minor version is used for breaking changes.

In the release notes of 0.23 he also strongly advocates on pinning the version:

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.

However, I would also like to point out that any build dependencies have to be pinned to expected versions, in order to avoid any unexpected system outages like we had this time. There's nothing worse, than having a version deployed to a dev/staging system that works and deploying the same version without changes a day later to production and having an outage 😕.

For example, I saw this line in the Docker build logs, which suggests that TypeScript is also not pinned to any version (not even a major version):

RUN npm install --global typescript

Maybe other dependencies are affected as well, so it would be great, if you could check this.

We cannot update the default to include --packages=bundle because it will break people with certain configurations.

As long as the build/stack synth fails, I'm personally fine with this, as long as there's an expressive error message telling me what to do. For me it's much more preferable to see a build time error and a broken pipeline, than an unexpected runtime error in a production system 😉.

[sholtomaud]

Another comment.

a) NodejsFunction is cool. b) AWS is a $2 trillion stock value company.

Therefore, why not just buy ESBUILD, or create your own, and roll it into CDK CLI?

Also, the more AWS controls the code, the more it can control the DevSecOps pipeline, the (hopefully) more reliable the CDK, and the minimisation of these 3rd party derived issues. Right?

[ajhool]

I bumped to the latest aws-cdk version 2.148.0 and was still seeing the problem. I realized it is because I also had [email protected]^ in my package.json

For those unaware, you can install esbuild (eg. yarn add esbuild) and CDK will use the NPM version instead of docker. I find the performance of the npm esbuild to be faster but I haven't looked closely at where that performance bump comes from.

So, if you've bumped cdk and are still seeing this issue, you might have esbuild in your package.json. You'll need to install the [email protected] and it should be pinned to the version.

#  Or whatever the latest working version is...
yarn add -D [email protected]

[tobiberger]

For those unaware, you can install esbuild (eg. yarn add esbuild) and CDK will use the NPM version instead of docker. I find the performance of the npm esbuild to be faster but I haven't looked closely at where that performance bump comes from.

Docker bundling will run a docker build ever time you run it. If the layers aren't already built on the machine, this means installing yarn, pnpm, typescript, and esbuild. That might be where the performance difference comes from.

[xazhao]

Closing this issue because the error has been mitigated. We will create a separate Github issue to pin the version to prevent this happening in the future.

[github-actions[bot]]

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.