Add node:* to external whitelist

[mgibbs189]

Allows for running supported Node packages on Cloudflare.

This is a rather serious oversight, given that no official solution has been rolled out after ~8 months of user attempts.

This PR is based on @chientrm's efforts (among others) at getting this pushed through.

Prior attempts:

#10028 -- support node compat in CF adapter
#10110 -- expose external esbuild option
#10214 -- allow override esbuild option
#10424 -- pass external to esbuild
#10521 -- pass alias and external to esbuild

The proposals to expose data to esbuild sound great... but for most use-cases, it's overkill for the simple need to access Node packages on Cloudflare.

@benmccann Can we please just add node:* to the external array?

---

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:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0bc155e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[benmccann]

adapter-cloudflare-workers would need to be updated as well

[mgibbs189]

@benmccann could we go ahead and merge this one in? I could open a PR for workers shortly thereafter.

[benmccann]

I'd rather just have one PR

It looks like #10544 does the same thing, but behind a flag. My first impression is that it seems reasonable to merge one of these, but I'm not sure which approach is better. What happens if you don't have node:* on the list of externals? I'd be interested to see what the exact error is and whether it happens when building the app or running it

[mgibbs189]

I'd rather just have one PR

It looks like #10544 does the same thing, but behind a flag. My first impression is that it seems reasonable to merge one of these, but I'm not sure which approach is better. What happens if you don't have node:* on the list of externals? I'd be interested to see what the exact error is and whether it happens when building the app or running it

Without the node:* external, Cloudflare won't process any node imports, e.g. node:buffer, node:crypto, etc.

The nodeCompat flag appears unnecessary. Cloudflare uses its own nodejs_compat flag, which is configured via CF's dashboard UI or wrangler.

[benmccann]

Cloudflare won't process any node imports, e.g. node:buffer, node:crypto, etc.

I want to know what the user experience is though. What's the error message a user will get and will it occur when building or running? CC @chientrm

[chientrm]

Cloudflare won't process any node imports, e.g. node:buffer, node:crypto, etc.

I want to know what the user experience is though. What's the error message a user will get and will it occur when building or running? CC @chientrm

The error will occur when building without node:* external flag 😗

[mgibbs189]

Error: Cannot find module 'node:crypto' imported from '<the_path_to>/+page.server.js';

i.e. same as what happens when trying to import any other non-existing module...

[benmccann]

Whitelisting node:* may be too aggressive. I think if we do that and someone uses an API like node:fs then they won't find out their app is broken until they go to deploy it and it'd be better for that to occur during build. How about we switch it to just the list of APIs that Cloudflare supports with their node compatibility?

'node:assert', 'node:async_hooks', 'node:buffer', 'node:crypto', node:diagnostics_channel', node:events', node:path', node:process', node:stream', node:string_decoder', node:util'

[mgibbs189]

According to Cloudflare's docs, node:fs isn't available. Are you seeing otherwise?

Also keep in mind that node_compat is disabled by default. Users need to explicitly enable it.

While I'm not against an explicit list of whitelisted APIs, it seems unnecessary (and increases maintance burden as more APIs are gradually added).

[benmccann]

According to Cloudflare's docs, node:fs isn't available. A

Yes, that's my point. If you whitelist node:* then an app using node:fs will pass the build and then fail during deployment. By whitelisting just the supported APIs we could save people by discovering this sooner

[mgibbs189]

@benmccann good point. I've updated the PR to include the curated list of modules.

[Rich-Harris]

Thanks — closing this in favour of #10544 which does the same thing but also updates adapter-cloudflare-workers