Mark node: as external for Node.js compatibility

[chientrm]

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

Explain

Last time I asked to add polyfill for Buffer API. That's no longer neccessary because Cloudflare added a bunch of nodejs APIs to Workers. They are all prefixed by node:.

Example:

import { Buffer } from 'node:buffer';

These APIs are enabled via compatibility_flags nodejs_compat and work for Workers, Pages and wrangler.

The APIs is here: Node.js compatibility

[changeset-bot]

🦋 Changeset detected

Latest commit: 62dd0f5

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

This PR includes changesets to release 2 packages

Name	Type
@sveltejs/adapter-cloudflare-workers	Minor
@sveltejs/adapter-cloudflare	Minor

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

[ghostdevv]

I'm for this change but I think it may make sense having it behind some sort of config option, maybe nodeCompat? That way we can document what this means and not suddenly start allowing node packages for people who don't know what that means on Cloudflare.

[chientrm]

I'm for this change but I think it may make sense having it behind some sort of config option, maybe nodeCompat? That way we can document what this means and not suddenly start allowing node packages for people who don't know what that means on Cloudflare.

I committed the suggested solution yesterday, not sure GitHub will notify the mods so that I quote your reply here.

[ghostdevv]

We should also add notes to the docs about this 🙏 We'll also will need some opinions from other maintainers before we can merge

[benmccann]

We generally try to avoid adding options, so unless there's a scenario where we don't want to do it, I would suggest we should always do it.

However, I'm not sure whether it's enough? @cimnine reported in #10521 that they had to use alias to actually address an issue using a Node.js module. Is there some kind of guidance we could provide users as to when external is sufficient and when alias would be required?

Update: answering my own question. In #10521 the user was trying to use fs. Cloudflare only provides some Node modules and that isn't one of them: https://developers.cloudflare.com/workers/runtime-apis/nodejs/

[ghostdevv]

However, I'm not sure whether it's enough? @cimnine reported in #10521 that they had to use alias to actually address an issue using a Node.js module. Is there some kind of guidance we could provide users as to when external is sufficient and when alias would be required?

I'm still not 100% sure what the decision is re: various esbuild options being passable to the adapters - regardless this might be a nice change to reduce friction even if we do end up merging one of the esbuild option related prs - WDYT? @benmccann

[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'

[chientrm]

Yep that sounds legit. Cloudflare node_compat doesn't have modules like fs, ...

[chientrm]

Is fixed in my latest commit

[chientrm]

May as well I remove AdapterOptions.nodeCompat and always include those externals? 🤔

[benmccann]

Hmm, I don't know how to merge this commit lol.

I merged it via the GitHub UI for you

[Rich-Harris]

Thank you!

We should remove the nodeCompat option — there's no real value in it since it doesn't reflect or determine the project's compatibility flag. Better to only have to set it in one place.

Though it's probably worth generating a useful error when using unsupported modules like node:fs, or supported-but-unprefixed modules like path — will do that in a follow-up PR

[mgibbs189]

@chientrm @benmccann @Rich-Harris Thanks! 🎉

[Hebilicious]

@Rich-Harris While things are better, the adapter is still un-usable in some cases :/ For example trying to build a sveltekit project that uses pg.

When bundling a project with wrangler and the node_compat flag (cli or wrangler.toml), several things happens :

• https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts this plugin flags all node:x modules as externals.

• These polyfills are injected : https://github.com/cloudflare/workers-sdk/blob/ef0642796dbe17b30fd7b83ccfd3efc651ce0a1a/packages/wrangler/src/deployment-bundle/bundle.ts#L334

This allows cloudflare workers and cloudflare pages projects to correctly build if there's a supported library such as "pg" that doesn't use "node:x" specifiers and use incompatible APIs.

Note that there's also an issue with wrangler deploy where this is not possible to specify node_compat: true, therefore the project must be built manually, then deployed.

As a workaround, I'm using a forked version of the cloudflare adapter to inject the polyfills.

Another gotcha is that the nodejs compatibility flag is not compatible with the polyfills while building with wrangler ... But it's still possible to build manually and enable the flag.

I'm not sure what would be a possible solution, but it would be nice to enable esbuild custom settings for the cloudflare adapters.

[g-wozniak]

I am getting exceptions for every package I am trying to use that uses node: in hooks.server.ts. Tried jsonwebtoken and now paseto.

{
  "outcome": "exception",
  "scriptName": "pages-worker--1989843-preview",
  "diagnosticsChannelEvents": [],
  "exceptions": [
    {
      "name": "Error",
      "message": "Dynamic require of \"node:crypto\" is not supported",
      "timestamp": 1708693796319
    }
  ],
}

Using:

• "@sveltejs/kit": "^2.5.0"
• "@sveltejs/adapter-cloudflare": "^4.1.0"

I think, inside 'hooks.server.ts', got a custom handle which performs:

import {V4} from 'paseto'
...
await V4.verify(sessionToken, secretKey)

and I suspect that's the issue.

Got "nodejs_compat" flag enabled, manually set via Cloudflare dashboard but it does not help.

[chientrm]

I am getting exceptions for every package I am trying to use that uses node: in hooks.server.ts. Tried jsonwebtoken and now paseto.

{
  "outcome": "exception",
  "scriptName": "pages-worker--1989843-preview",
  "diagnosticsChannelEvents": [],
  "exceptions": [
    {
      "name": "Error",
      "message": "Dynamic require of \"node:crypto\" is not supported",
      "timestamp": 1708693796319
    }
  ],
}

Using:

• "@sveltejs/kit": "^2.5.0"
• "@sveltejs/adapter-cloudflare": "^4.1.0"

I think, inside 'hooks.server.ts', got a custom handle which performs:

import {V4} from 'paseto'
...
await V4.verify(sessionToken, secretKey)

and I suspect that's the issue.

Got "nodejs_compat" flag enabled, manually set via Cloudflare dashboard but it does not help.

You're sure you have the flag for the preview environment, too? 😉

[g-wozniak]

Yep, I even changed it to something else to see if the build fails and it does.
It's worth pointing out that wrangler property is called "node_compat" but the flag itself is "nodejs_compat" which may be confusing for some. So made sure I've got the right one.

My svelte.config.js is as following:

      adapter: adapter({
         routes: {
            include: ['/*'],
            exclude: ['<all>']
         },
      }),

Perhaps something is missing here...

[chientrm]

Yep, I even changed it to something else to see if the build fails and it does.
It's worth pointing out that wrangler property is called "node_compat" but the flag itself is "nodejs_compat" which may be confusing for some. So made sure I've got the right one.

Please notice that node_compat and nodejs_compat are two completely different things.

You can ask for support on discord Cloudflare.

[g-wozniak]

According to the documentation the flag name is: "nodejs_compat". Got you though. That means it requires manual deployment via wrangler.

[absktoday]

Is this supposed to work for Pages Adapter or do I have to use Worker adapter, because I am still getting the Could not resolve "crypto" or Could not resolve "node:crypto".

[devinellis]

Same question as @absktoday but for a dependency that is attempting to import url. Cloudflare states this is supported in Pages, and yet the adapter throws an error. I am using Cloudflare Pages which does not use wrangler.toml (as far as I know?). I created one anyways to see if it would "trick" the adapter but it did not work. My workaround is to fork the dependency and remove those imports :(

[ghostdevv]

Is the error at runtime on Cloudflare pages, or at buildtime either locally or on the CF pages builder @devinellis? What imports is the package using url or node:url? Could you share a minimal reproduction?

[devinellis]

Sorry for the lack of detail:

• The package is doing require('url'). Commenting these lines out in a fork made the problem go away.
• It fails both locally and in the CF Pages builder.
• Setting nodejs_compat in wrangler.toml or in the Pages Dashboard didn't seem to make a difference.

Note if I am reading the CF docs right, nodejs_compat no longer needs the node: prefix on package names, as long as your compatibility_date > 2024-09-23