create commonjs bundle with adapter-node, since esm output has issues

[Rich-Harris]

This is an alternative approach to #6797. By generating CommonJS instead of ESM, esbuild doesn't replace require calls for built-in modules with __require (which throws a 'Dynamic require of ... is not supported' error — evanw/esbuild#1921).

As such, it's possible to bundle libraries that have such calls, instead of having to move them to dependencies so that they're not bundled, which is onerous since it's very hard to figure out which are the offending packages.

The downside of this approach is that it's no longer possible to have ESM packages as external dependencies — they must always be bundled. I don't have a clear sense of which is the lesser of two evils.

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. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 1239904

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

[Conduitry]

I have the feeling you're not going to like it, but: Would it make sense for this to be configurable?

Also, what file extension are these being written out as? It looks like still .js which would be a problem if the project's pkg.type was module.

Edit: Oh, never mind about the last bit, I see you're writing build/package.json with just {"type":"commonjs"}.

[Rich-Harris]

Would definitely prefer to avoid configuration if we can, just because knowing when to use which is such an in-the-weeds problem. I guess the question is whether or not there are cases where an ESM package couldn't be bundled. I suppose you could have an ESM package that depended on a native module like sharp. Blurgh.

I guess an alternative would be some version of evanw/esbuild#1921 (comment) — e.g. rewrite all the __require calls back to require and add this to the top of each module:

import { createRequire } from 'module';
const require = createRequire(import.meta.url);

[Conduitry]

I'm less worried about adding potentially regrettable APIs to adapters than I am to SK core. If we can't/don't want to do something like the __require->require hack mentioned above, then I don't see another great option now besides allowing people to bundle to either CJS or to ESM.

Having a nasty API wart isn't as bad as telling some people they are just SOL unless they want to fork the adapter. If we find a satisfactory workaround for the esbuild problem (or if esbuild addresses this itself), then we can remove the option and release a new major version of the adapter.

[benmccann]

My solution, which isn't an immediate one, is to fix Vite so that we no longer need esbuild involved. I filed vitejs/vite#9919 about this and if Vite worked the way patak described then we could remove esbuild. I'm not sure if it should be considered a bug that might be able to be fixed in Vite 3 that it doesn't behave as expected, but there's a chance we can't fix it until Vite 4 a couple of months from now. Either way, I'll be pushing to get that fixed and drop esbuild so that we can have ESM and avoid all the esbuild bugs

[Rich-Harris]

Alternative approach here: #6896

[Rich-Harris]

closing as #6896 was merged