evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
37.61k stars 1.11k forks source link

aws lambda build issue with esbuild v0.22.0 #3819

Closed volodyad closed 1 day ago

volodyad commented 2 days ago

Aws cdk generates next command on lambda build via esbuild

npx esbuild --bundle path/index.ts --target=node20 --platform=node --outfile=test-index.js '--external:@aws-sdk/*'

with version 0.22.0 it makes build with unbundled dependencies, as aws expect to bundle by default - builds are failing

with version v0.21.0 all works as expected. If you build lambda via default docker is uses next command "npm install -g esbuild@0" to install latest esbuild, so literally all lambda builds via aws docker are failing

VanTanev commented 2 days ago

Hey @evanw,

Just wanted to share that this affects us as well, and it will be affecting any aws-cdk lambda user. Unfortunately, while this one is on aws-cdk for installing esbuild@0 here the fastest fix may be to revert the breaking change that makes --packages=external the default behavior of esbuild.

Of course, that decision is entirely up to you.

The related aws-cdk issue: https://github.com/aws/aws-cdk/issues/30717 (edited to a string link because people coming from the aws-cdk issue apparently think it's fine to shout here)

martzcodes commented 2 days ago

Why was an EXPERIMENTAL deliberate breaking change introduced as a minor version bump?!?

VanTanev commented 2 days ago

Why was an EXPERIMENTAL deliberate breaking change introduced as a minor version bump?!?

This is the contract of a 0.x.y semver. The issue is with aws-cdk installing esbuild@0 and not pinning to a known compatible version.

martzcodes commented 2 days ago

True, and I forgot about the 0.x quirks of npm, but for a project that has 32 million weekly installs a bit more care should be taken. CDK doesn't install esbuild by default (if not installed it uses docker which uses it, but that's super slow because of docker so many avoid it)... on init'ing a project many CDK users will then do npm install esbuild --save-dev as one of their first steps, which would automatically pull in this experimental breaking change

khastation commented 1 day ago

Can report the same, 0.22 kills bundling of aws lambda with cdk, leading to module not found errors. I've pinned esbuild to 0.21.5 to fix for now.

NickIannelli commented 1 day ago

The treatment of esbuild being on a 0.xx release and sitting behind that as an excuse to making breaking changes without any form of input from the community, while getting almost 26 million downloads a week is abysmal behaviour from the maintainer.

Whether you agree with your software being stable or beta doesn't really matter when it's deeply relied on by millions of users.

The decision to release a breaking change without even a second thought intentionally (as indicated by the release notes) for a reason essentially being "people are confusing the initial intent, so I'm breaking their work." is genuinely baffling.

"many people don't read the documentation and don't do this, and are then confused when it doesn't work"

What about the millions of people that this already was working for?

This is something that could've been done progressively with ease.

  1. Start warning when no --packages option is enabled that the default behaviour will be changing with the next release, and a planned date
  2. Release the breaking change after the warning has existed for at least a week, preferably a month. At least people will have been warned and had an opportunity to update their default behaviours

But no, you took the easy way out and sit behind the "it's beta software, we can break things" mask as an excuse for poor practices.

kennu commented 1 day ago

Having the same ImportModuleError problem with CDK Lambda functions. Really bad, because there's no warning at build/deployment time. Lambda functions just stop working after redeploying with esbuild 0.22 and it's not so easy to track down the reason.

evanw commented 1 day ago

It sounds like the primary problem here is caused by a very large company locking their widely-used product to essentially esbuild@latest, which has been very explicitly not the recommended way to use esbuild since the start of the esbuild project. This is the reason the very first paragraph in esbuild's getting started documentation tells you to install esbuild with the --save-exact flag:

First, download and install the esbuild command locally. A prebuilt native executable can be installed using npm (which is automatically installed when you install the node JavaScript runtime):

npm install --save-exact --save-dev esbuild

This is also the reason why every single breaking change release for the past four years (from 0.8.0 to 0.22.0) has had a notice like this as the very first line of the release notes:

This release deliberately contains backwards-incompatible changes. To avoid automatically picking up releases like this, you should either be pinning the exact version of esbuild in your package.json file (recommended) or be using a version range syntax that only accepts patch upgrades such as ^0.21.0 or ~0.21.0. See npm's documentation about semver for more information.

The way I've been getting feedback on things since the start of the project has been to publish a breaking change release to avoid disrupting people, and then get feedback on that new release line. This also isn't some random breaking change for fun. It's a change that I've been thinking about for a while, and it solves a long-running class of issues that people encounter when they use esbuild as you can see from the many issues that I keep getting about it. I linked to some of them in the recent release notes:

I was not previously aware that Amazon was using an unpinned version of esbuild for a widely-used product of theirs. For what it's worth, I don't believe Amazon has ever communicated with me about the use of esbuild in their product. I'm sorry about the disruption that this change has caused. I sincerely hope that Amazon learns to pin their dependencies in the future to avoid breaking their customers.

Perhaps the best path forward here could be for esbuild to use a beta channel model for further development, to make sure that npm install esbuild@latest is a stable esbuild release that has already been widely tested instead of being the latest version of esbuild like it is now. I'll have to learn more about how npm tags work first though as I haven't set up that kind of workflow before.

HanabishiRecca commented 1 day ago

I'm sorry about the disruption that this change has caused.

You shouldn't be sorry really. Amazon makes money on your project instead of you. If I were you, I would have said "f**k your AWS, if you want it your way, then pay me".

And don't listen to the whiners above. Who also probably earn money on free project and still dare to criticize.

prisis commented 1 day ago

They did fix it in https://github.com/aws/aws-cdk/commit/7f5ce4bfe94b19efe6c0c8aa0ec850cdfc4b8ebb.

This (esbuild) project follows semver, in the manifest it described how semver works, aws has a prob and a docker arg to change the version too, so there was no issue with the release of esbuild.

In the end someone would still complain if you would release a v2 (in this case v0.22.0) with a breaking change...

kennu commented 1 day ago

In my view, the whole 0.x numbering scheme is not such a good idea in general. It would be better to release major versions on breaking changes. I don't really see the benefit in having 0.x versions, but I see the problems.

lazarljubenovic commented 1 day ago

In my view, the whole 0.x numbering scheme is not such a good idea in general. It would be better to release major versions on breaking changes. I don't really see the benefit in having 0.x versions, but I see the problems.

The QWERTY keyboard isn't a good idea in general either as it's designed to slow you down, and doesn't translate to the digital world at all, and yet here we are.

The 0.x.y exception to the break-at-major-only makes perfect sense though; otherwise every project would be at version 2103.0.0 by the time you hear of it. That's the benefit. The 0 at the start of the version is a signal enough that breaking changes are coming since it means, by definition, that the public API hasn't been finalized yet. esbuild is simply following a very well-known semver specification.

Don't install @0s, simple as that.

HanabishiRecca commented 1 day ago

Semver spec ignores leading zeroes and default npm save-prefix='^' would not allow minor bump in this case. I.e. default ^0.21.x would never install 0.22 on its own. https://semver.otterlord.dev/?package=esbuild&range=^0.21.0

And If you use latest, *, 0 etc., as AWS did, the version number doesn't do anything anyway.

kennu commented 1 day ago

The 0.x.y exception to the break-at-major-only makes perfect sense though; otherwise every project would be at version 2103.0.0 by the time you hear of it. That's the benefit.

For me, version 2103.0.0 is completely fine, if it would prevent problems like the present one. Versions exist to help people, not other way round.

lazarljubenovic commented 1 day ago

The 0.x.y exception to the break-at-major-only makes perfect sense though; otherwise every project would be at version 2103.0.0 by the time you hear of it. That's the benefit.

For me, version 2103.0.0 is completely fine, if it would prevent problems like the present one. Versions exist to help people, not other way round.

Another thing that prevents problems is following the spec that says that 0.22.0 is a breaking change from 0.21.0. People are pretending like it's rocket science. Heck, leap years are more complicated than this.

kennu commented 1 day ago

Another thing that prevents problems is following the spec that says that 0.22.0 is a breaking change from 0.21.0. People are pretending like it's rocket science. Heck, leap years are more complicated than this.

I agree, both are good ways to prevent problems. I think 0.x versions tend to cause this kind of problems in many places, and you can try to educate people, and you can also try to avoid using 0.x versions unless really required.