feat(adapters): Add back adapter.esbuild to allow configuration of bundling step

[tomaspietravallo]

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

---

Hey there! I am opening this PR to solve my own issue.

This is my first PR to svelte/ any big oss project, all feedback or comments welcomed

The problem:

Essentially the lack of configuration prevents us from modifying key configuration of the bundling step which cannot be accessed in any other way. Related issues/ PRs: #3733 #1914

In my particular case: platform: neutral prevents certain modules that require node modules from bundling as edge functions (in particular @google-cloud/storage but I heavily suspect many other packages may have the same issue #3733). I cannot find any workarounds to get the packages to bundle other than modifying the adapter (be it hard coding or creating a configurable one)

It appears from #1914 this was a feature at one point but has silently been dropped. If there is a reason for this to be the case, I would be glad to hear.

This PR’s proposed solution

This PR adds an argument esbuild of type function which gets called inside the adapter with the default options as only parameter. I adhered to what’s described in the adapters’ (older) read mes as much as possible.

Changes:

This PR does not change the default platform nor should have any effect on projects that do not pass the esbuild argument to the adapter in their svelte.config.js files

This PR does affect sites that pass an esbuild argument to the adapter in their configs. It takes the output of esbuild and uses it in place of the default options

I am not fully aware platforms other than Netlify would support platform: node but I still believe being able to access this and other key configuration options for esbuild should be permitted as a power-feature.

I have tested the Netlify adapter on my website overwriting the npm package with a local hot-fixed copy and it does work.

Packages affected:

• adapter-netlify
• adapter-vercel
• adapter-cloudflare

Thank you for considering this PR, and thank you to all maintainers for the hard work you put in, Svelte/Kit is awesome!

[ghostdevv]

@benmccann has mentioned previously that esbuild is considered an implementation detail. However, I personally think it's unlikely that we would move from esbuild here, and if we do it should probably be a breaking change regardless of if we added esbuild specific options because of how different the two are. A perspective against this would be the unnecessary complexity in a lot of cases, and an increase in the ability to accidentally break your build.

At the end of the day I am generally for this change as the adapters default settings can't cover every weird situation people seem to get themself into. We should see what other maintainers think about merging this before reviewing in case it's decided that we want to be more generic with the options

[benmccann]

Thanks @ghostdevv. I might agree with you. I wanted to represent Rich in the discussion though. His point of view is that the underlying issue should be addressed rather than working around it. E.g. in #10521 the user wants to stub out APIs, but a nicer solution would be to update the ical-generator package as I mentioned here: #10521 (comment).

In my particular case: platform: neutral prevents certain modules that require node modules from bundling as edge functions (in particular @google-cloud/storage but I heavily suspect many other packages may have the same issue #3733).

@tomaspietravallo Are you using an old version of the adapter? We temporarily set it to neutral at one point in the past, but it's currently set to browser, so I'm surprised to hear you mentioning that platform: 'neutral' would be causing you difficulty

[ghostdevv]

Thanks @ghostdevv. I might agree with you. I wanted to represent Rich in the discussion though. His point of view is that the underlying issue should be addressed rather than working around it. E.g. in #10521 the user wants to stub out APIs, but a nicer solution would be to update the ical-generator package as I mentioned here: #10521 (comment).

I think we can fix quite a few of the issues (good example #10544). However, there will still be edge cases - for example for the first time ever I had to use an esbuild plugin with the cloudflare adapter, the only way I could do that was by installing this pr's code changes

[tomaspietravallo]

In my particular case: platform: neutral prevents certain modules that require node modules from bundling as edge functions (in particular @google-cloud/storage but I heavily suspect many other packages may have the same issue #3733).

@tomaspietravallo Are you using an old version of the adapter? We temporarily set it to neutral at one point in the past, but it's currently set to browser, so I'm surprised to hear you mentioning that platform: 'neutral' would be causing you difficulty

@benmccann well this PR is somewhat old (Mar 10th), but I was using the latest version of the adapter at that point in time.

It does seem to be set to platform: browser instead of platform: neutral now.

This was just a test on my personal website so I ended up giving up on this, but ideally I would've been able to set platform: node for things to work (even if this isn't advisable for most projects). The underlying issue was that Netlify was able to serve Node APIs to the edge function if platform was set to node but wouldn't if platform was browser/neutral, so the specific API I was trying to use which relied on node would fail the build.

I am still open to working on this issue and pushing a way to modify esbuild settings as an advanced feature. I fully agree with you #10521 (comment) in that this would be a nice option to have for unmaintained projects and the like, but should strongly be advised against (I'm glad ghostdevv found my code useful but I wouldn't recommend anyone try it out on production)

[gtm-nayan]

@tomaspietravallo We decided to go ahead with exposing esbuild options. Are you able to continue this PR? The merge conflicts need to be resolved, and the new options would need some documentation updates.

[tomaspietravallo]

@tomaspietravallo We decided to go ahead with exposing esbuild options. Are you able to continue this PR? The merge conflicts need to be resolved, and the new options would need some documentation updates.

I am. Things are ramping up at work and I'm pretty busy lately but I'll do my best to make progress on this PR

[benmccann]

Evan just announced that they will be replacing esbuild in Vite and are aiming to do so by the end of the year: https://viteconf.org/23/replay/vite_keynote. I think we would also end up replacing esbuild with rolldown in order to minimize the number of dependencies in projects. Given that we may soon be removing esbuild, I'm not sure we should be exposing the esbuild options right now

[gtm-nayan]

esbuild is great for the "chuck everything in a single file with lazy closures" case that we use in the adapters, which Rollup currently doesn't do, and I don't think Rolldown will either if it maintains parity with Rollup. I think we'll be on esbuild for a while.

[benmccann]

If you jump to the end of Evan's keynote you can see that his plan is to make rolldown a replacement for esbuild in the first of four stages. It's not until later on in stage two that they plan to start replacing rollup with it. So at the beginning it will actually be more of a replacement for esbuild than rollup as far as I understand

[Rich-Harris]

Compatible Node built-ins are now supported in Cloudflare deployments, thanks to #10544. I've just opened #11675 which does the same thing for edge functions on Netlify and Vercel.

As @benmccann says upthread, my view is that this is far preferable to exposing the esbuild configuration — there's some finicky stuff around aliasing and error handling and so on that is unreasonable to expect users of these adapters to do — so I'll close this. Thanks!