feat(adapter-node): support all esbuild build options

[sastan]

This PR allows changing the options passed to esbuild build which allows to customize the server generation like target node version, sourcemap, inject, and more.

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 pnpx changeset and following the prompts

[changeset-bot]

🦋 Changeset detected

Latest commit: 3ee4efe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/adapter-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[jthegedus]

I think it would be more clear if these options were nested under a single esbuildOptions param field instead of spread amongst the base param obj.

[sastan]

I think it would be more clear if these options were nested under a single esbuildOptions param field instead of spread amongst the base param obj.

I have adjusted the PR to use a esbuildOptions parameter. I changed the default options to use outdir instead of outfile to support code splitting. This still creates a single build/index.js file because the entrypoints filename is index.js. If the outfile option is set outdir will not be used because esbuild expects either outfile or outdir.

[mohe2015]

Commit message has a typo.

[sastan]

Commit message has a typo.

Sorry, I do not know what you mean.

[mohe2015]

c6075d7
I assume the title is supposed to be "adapter" and not "adpater"

[mohe2015]

c6075d7
I assume the title is supposed to be "adapter" and not "adpater"

Same thing in the commit message. (Don't know if you will fix it later or overlooked it)

[sastan]

c6075d7
I assume the title is supposed to be "adapter" and not "adpater"

Same thing in the commit message. (Don't know if you will fix it later or overlooked it)

I can not fix that now. But I believe that PRs are squashed and use the PR title (which is fixed) as commit message.

[jthegedus]

Just thought I would raise before we adopt esbuild more significantly, but the initial usage of esbuild was a suggested soln

Mostly just playing around for now, but this is intended to solve the perennial problem of dependencies behaving weirdly, by allowing adapters to determine what gets bundled.

-Rich Harris in #1091

Has there been more thought on alternative solutions or is esbuild usage likely to stick around? @Conduitry @benmccann

[benmccann]

@jthegedus it's not an area I understand well, so I will defer to Rich and Conduitry

[benmccann]

There's a related PR for adapter-vercel here: #1732

[sastan]

It looks like this PR needs to be rebased

Done.

There's a related PR for adapter-vercel here: #1732

I do not know if using config.kit.vite().esbuild is the best way here. In my specific use-case I want to use a different esbuild config (mainly target and format).

Maybe a hybrid approach:

{
	...config?.kit?.vite()?.esbuild,
	...esbuildOptions,
}

But that would make reasoning about the resulting config difficult.

What is your take on this?

[sastan]

Is there anything I can do to progress this PR?

[jasonlyu123]

Since the common js output format is also available in this PR. I'm wondering if we can check if __dirname and require already exist before shim it, so the configuring is easier. I tried building it to cjs before and it's throwing an error in the runtime because import.meta.url is undefined.

But regarding how to check if __dirname exists, I can only think of some hack like this:

var __dirname;
__dirname = __dirname ?? dirname(fileURLToPath(import.meta.url))

(Sorry I spamed the notification. accidently deleted the previous comment)

[sastan]

Since the common js output format is also available in this PR. I'm wondering if we can check if __dirname and require already exist before shim it, so the configuring is easier. I tried building it to cjs before and it's throwing an error in the runtime because import.meta.url is undefined.

But regarding how to check if __dirname exists, I can only think of some hack like this:

var __dirname;
__dirname = __dirname ?? dirname(fileURLToPath(import.meta.url))

(Sorry I spamed the notification. accidently deleted the previous comment)

I use define + inject:

shim-node-cjs.js

import { pathToFileURL } from 'url'

export const shim_import_meta_url = /*#__PURE__*/ pathToFileURL(__filename)

And the in the esbuild config:

    // Or something like path.relative(import.meta.url, './shim-node-cjs.js')
    inject: [require.resolve('./shim-node-cjs.js')],
    define: {
      'import.meta.url': 'shim_import_meta_url',
    },

This works quite well for single file outputs.

[jasonlyu123]

I'm also fine with an example in the doc. I just thought making it work out of the box would be a little bit easier for the user.

[benmccann]

Why are you wanting to change target and format?

[sastan]

Why are you wanting to change target and format?

We are bundling the server and the assets using pkg into an executable. pkg needs cjs as format and we can use a higher target (v16).

[jasonlyu123]

And we're hosting with Windows IIS with iisnode, which doesn't seem to support ESM.

People using other hosting platforms that don't already have an official adapter may want to leverage the node adapter. And since some other adapters also ship common js. I think it would a good idea to allow configuring node adapter to output common js.

[benmccann]

@sastan it looks like this PR will need to be rebased

[benmccann]

I do not know if using config.kit.vite().esbuild is the best way here

I agree. It's better to keep them separately configurable