Cloudflare adapter disables known cloudflare worker "compatible" dependencies

[patdx]

Summary

The adapter solid-start-cloudflare-workers pulls in "node" exports of dependencies by default. This makes sense for solid-js framework and possibly precompiled Solid Components in node_modules. However, for many other third-party packages, this will pull in incompatible "node" versions of those libraries. Using browser/web exports when bundling for cloudflare workers (and similar edge runtimes) will probably lead to more success.

Background

I want to set up a graphql server as an API route.

What happened:

1. Plain cloudflare worker test

I tried to set up a graphql server using a package that claims to work with cloudflare.

Here is my implementation: https://github.com/patdx/solid-start-bundle-experiment-old/tree/main/plain-cloudflare-worker

It works great :)

2. Attempted integration into solid-start

I added it to an API route and In solid-start dev/node mode it worked fine.

https://github.com/patdx/solid-start-bundle-experiment-old/tree/main/solid-with-standard-adapter

I tried to build it with the solid-start-cloudflare-workers adapter.

But, I got a really big 2.5mb dist/server.js with a bunch of node imports

// dist/server.js
import manifestJSON from '__STATIC_CONTENT_MANIFEST';
import Url from 'url';
import require$$1$1 from 'fs';
import require$$6$1 from 'buffer';
import require$$0 from 'util';
import * as crypto__default from 'crypto';
import crypto__default__default from 'crypto';
import * as process$1 from 'process';
import Stream from 'stream';
import require$$0$1 from 'assert';
import require$$4 from 'net';
import http from 'http';
import require$$10 from 'stream/web';
import require$$1$2 from 'perf_hooks';
import require$$4$1 from 'util/types';
import require$$0$2 from 'events';
import require$$4$2 from 'tls';
import require$$3 from 'async_hooks';
import require$$1$3 from 'console';
import zlib from 'zlib';
import require$$0$3 from 'punycode';
import https from 'https';

Even if I turn on node_compat=true in my wrangler.toml it still does not work, wrangler fails with an error like:

✘ [ERROR] Build failed with 4 errors:

  dist/server.js:13:24: ERROR: Could not resolve "stream/web"
  dist/server.js:14:25: ERROR: Could not resolve "perf_hooks"
  dist/server.js:15:25: ERROR: Could not resolve "util/types"
  dist/server.js:18:23: ERROR: Could not resolve "async_hooks"

Not to mention there seems to be at least one or two fetch polyfills inside the bundle

3. My attempted fix

I noticed that cloudflare wrangler CLI is automatically bundling with conditions: ["worker", "browser"]

However, the solid-start-cloudflare-workers prebundles with ["node", "solid"]. This step is pulling in the (unwanted) Node.js version @graphql-yoga/common and all sorts of extra dependencies and Node polyfills.

I tried to give wrangler the .solid/sever.js directly and let wrangler do the only bundling instead.

This looked like it would work, until I went to try running the code and it errored out because that renderToStringAsync is not available in the browser. Oh, oops 😅

After some trial and error, I found I could set resolveOnly: "solid-js" inside of my fork of solid-start-cloudflare-workers. Then, this step bundled the node/server version of solid-js so renderToStringAsync can work. And then wrangler bundled everything else in the browser friendly version. It worked! :)

Conclusion

Edge runtime need the server version of solid-js.

However, for many other packages, this will pull in polyfills and node-native code that are very heavy or do not work in an edge runtime. Not to mention, I think it will be a bit confusing for developers to find that solid-start-cloudflare-workers only supports a subset of the packages supported by plain cloudflare workers.

I've tried to survey some other adapters and it seems most of the are leaning towards targeting web/webworker environments.

Additionally, some Vite-based frameworks do this inside Vite itself, saving a bundle step. For example, Shopify Hydrogen.

In a perfect world, I think we would try to find a way to get solid-js to expose its server exports to worker environments naturally. Perhaps using the worker export condition like emotion css. Then, solid-start-cloudflare-workers could target a worker/browser environment to be consistent with other cloudflare deploy pipelines while still getting the server exports from solid-js. This would also benefit non-solid-start based SSR environments that target cloudflare workers. However, the worker export is not really a standard, I don't know if this will be enough to support all edge runtimes.

I found this old discussion on the main repo: solidjs/solid#308 (reply in thread)

The only thing I'm wondering is do we need something other than node to indicate non-node server environments like Deno or Cloudflare. I am tempted to include server everywhere I include node. To be fair this is only a bundling concern but maybe node === all server isn't right.

Which is kind of related too. Exposing a parallel import "server" seems like it could be nice too (but would not be enough for compatibility with plain cloudflare wrangler API).

If it is preferred to solve by only changing the adapter, maybe we can add some configuration settings or consider using my hack of resolveOnly: "solid-js", so the adapter is only bundling solid-js and wrangler is handling the majority of the dependencies. Note: I guess any solution may need special logic to handle solid components imported from node_modules too. (Not sure how to do this...)

Related links

• Feat: generate bundles for "worker" style environement emotion-js/emotion#2819
• https://github.com/cloudflare/wrangler2/blob/main/packages/wrangler/src/bundle.ts#L200
• https://github.com/cloudflare/wrangler2/blob/main/packages/wranglerjs-compat-webpack-plugin/src/index.ts#L156
• https://github.com/netlify/vite-plugin-netlify-edge/blob/main/src/index.ts
• https://github.com/Aslemammad/vite-plugin-cloudflare/tree/main/packages/vite-plugin-cloudflare
• https://github.com/vercel/next.js/blob/6da621df635304615783b2442b002f0618199796/packages/next/build/webpack/config/blocks/base.ts#L21
• https://github.com/Shopify/hydrogen/blob/40b708a7d2a7563136de307eb462ae2ac68e44b0/packages/hydrogen/src/framework/plugins/vite-plugin-hydrogen-config.ts#L72
• https://github.com/vitejs/vite/blob/f8c8cf272345364e8a6181725335e95b957ba8c4/packages/vite/src/node/ssr/index.ts#L13
• adapters shouldn't pass arbitrary options to esbuild sveltejs/kit#4639
• https://github.com/sveltejs/kit/blob/aa6a5d4d5fb5b5fde2353ce57af45c2dcf7b9e25/packages/adapter-cloudflare-workers/index.js#L65
• Requiring "solid-js/web" on electron (with nodeIntegration) makes it think I'm on the server solid#1102

[nksaraf]

Wow! thanks for this super thorough analysis and is definitely a real problem, looks like. First step would be to fix the adapter to make things work! Then we should explore how solid-js core can cooperate here.

[ryansolid]

I think addressing export conditions in solid is the easiest. Libs use solid export and generally don't follow this so real only core is a concern. worker is legitimate, also used for building for service workers. Webpack respects it.

I was looking and Deno suggested using deno so there is no standard here. It costs us little to add more conditions though.

Part of me wants just to have one server but built in tooling expects their conditions. Let's add worker to the Solid core.

The only contention here are people insisting on adding reactivity to the server and not wanting the default server conditions to result to the reactive-less bundles. But that's a problem for later.

[patdx]

I have struggled to find good examples but today I found that react-dom/server has the "worker" condition (and also "deno") which seems like another point in support of doing the same in solid core. (Pull request which added it)

[patdx]

Hi, I tried another experiment today, I tried to see if Vite could build the worker with the correct dependencies in one build step. I messed around with some of the alias settings but couldn't get any of to work correctly. then I moved on to patching the solid-js package.json file. After patching I was able to build an almost Cloudflare-ready worker with the below Vite config.

(Side note: If I could figure out how to make the cloudflare entry,js the Vite entry point, I think it could be deployed in Wrangler --nobundle mode)

import path from "path";
import solid from "solid-start/vite";
import { defineConfig } from "vite";

export default defineConfig(async (env) => ({
  esbuild: false,
  plugins: [
    solid({
      adapter: {
        start() {},
        async build(config, builder) {
          await builder.server(path.join(config.root, ".solid", "server"));
          await builder.client(path.join(config.root, "dist", "public"));
        },
      },
    }),
  ],

  ...(env.ssrBuild && {
    resolve: {
      conditions: ["worker"],
    },
  }),

  ssr: {
    noExternal: true,
    target: "webworker",
  },
}));

---

What I want to note is, I got stuck about this Vite config not working for a while. After I patched the "worker" condition into the solid-js package JSON files, I could see the vite resolver logic trying to use it as expected. But it still compiled the browser file into the final SSR bundle.

It turns out the secret ingredient is that Vite is also checking the browser map field of package.json.

For Vite SSR mode with target=webworker, after it finds the export from the exports in package.json field, it runs it through the browser map, which maps it back to the client version.

https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/resolve.ts

const targetWeb = !ssr || ssrTarget === 'webworker';
// ...
if (targetWeb && isObject(browserField)) {
  entry = mapWithBrowserField(entry, browserField) || entry
}

So uh, it may be good to consider remove the browser field in package json at the same time as adding the "worker" export condition. From Vite's perspective, workers are in the browser family so it will honor .browser even after conditional exports are resolved. Hope that makes sense.

Here was my patch to solid-js (deployed using yarn berry):

diff --git a/package.json b/package.json
index 79c1d70367baea440cf8092b7da05feb3c6f243f..5a18fd09e135eae82e2d2c7ffd36ffc1a8d68a6e 100644
--- a/package.json
+++ b/package.json
@@ -11,10 +11,6 @@
   },
   "main": "./dist/server.cjs",
   "module": "./dist/server.js",
-  "browser": {
-    "./dist/server.cjs": "./dist/solid.cjs",
-    "./dist/server.js": "./dist/solid.js"
-  },
   "unpkg": "./dist/solid.cjs",
   "types": "types/index.d.ts",
   "sideEffects": false,
@@ -44,6 +40,13 @@
   ],
   "exports": {
     ".": {
+      "worker": {
+        "import": {
+          "types": "./types/index.d.ts",
+          "default": "./dist/server.js"
+        },
+        "require": "./dist/server.cjs"
+      },
       "browser": {
         "development": {
           "import": {
@@ -120,6 +123,13 @@
     },
     "./store/dist/*": "./store/dist/*",
     "./web": {
+      "worker": {
+        "import": {
+          "types": "./web/types/index.d.ts",
+          "default": "./web/dist/server.js"
+        },
+        "require": "./web/dist/server.cjs"
+      },
       "browser": {
         "development": {
           "import": {
diff --git a/web/package.json b/web/package.json
index 4c6dc0a37661bd34435f0ad4666010c5bcb460e9..802ee6512bfb55d4616e2959a2ec84cd23ebef4b 100644
--- a/web/package.json
+++ b/web/package.json
@@ -2,16 +2,19 @@
   "name": "solid-js/web",
   "main": "./dist/server.cjs",
   "module": "./dist/server.js",
-  "browser": {
-    "./dist/server.cjs": "./dist/web.cjs",
-    "./dist/server.js": "./dist/web.js"
-  },
   "unpkg": "./dist/web.cjs",
   "types": "./types/index.d.ts",
   "type": "module",
   "sideEffects": false,
   "exports": {
     ".": {
+      "worker": {
+        "import": {
+          "types": "./types/index.d.ts",
+          "default": "./dist/server.js"
+        },
+        "require": "./dist/server.cjs"
+      },
       "browser": {
         "development": {
           "import": {

[ryansolid]

That's annoying and possibly something to be taken up with Vite I suppose. Browser field is important for non-ESM envs. Things like Jest. Legacy systems that don't support export maps. I don't believe it can just be removed.

[patdx]

Sure that makes sense. Especially considering that exports is the newest standard and it is possible to specify browser condition inside of exports, my gut feeling is the exports field should supersede browser field so library authors can define new behaviors without breaking old ones.

I just checked and esbuild does not seem to have that quirk. So I guess unless/until that behavior can be changed in Vite it is better to NOT set noExternal: true in the vite config. Then either manually do an esbuild bundle after or let Wrangler do its internal esbuild bundle.

[nksaraf]

okay this seems like such a weird and complicated but one that we need a really nice solution for.. I wonder if @patak-dev can help us here with how we should go about this?

[patdx]

Yeah I am not 100% sure how to go about this too. I think it would be worthwhile to do something because IMO, solid-start has a amazing value proposition for the worker deployment target and it has the potential to capture a huge amount of this space if it is easy to deploy, especially if it is space-efficient.

Personally I would suggest first adding some new worker friendly export condition to solid-js core library. To the best of my knowledge this would be "good enough" to let the adapter use esbuild as a second step and create a very nice build for cloudflare workers.

I don't know how to decide about condition names like "workers", "deno", "server", To make it less scary, maybe the Solid team could add a really long and clearly experimental condition name like "experimental-solid-start-server" with no guarantee that it will be there forever? Then the solid-start cloudflare adapter could be updated to match this condition.

If a new solid-js core package is released with this change, it would also be be easier to create a reproduction of the way Vite is resolving exports/browser fields see if there is any wiggle room for Vite to change the behavior here. (Also I guess it would be good to do a comparison between vite, rollup, esbuild, webpack at this point too)

[patdx]

I tried the solid@1.5.6 which had some new exports added for deno and worker, unfortunately I don't think it really makes a difference yet because the "browser" condition comes first and has priority over the "worker" condition.

Would it be possible to move the "worker" condition before the "browser" condition?

I've been doing some more research and made a tool to help visualize how the exports are resolved, here is solid-js/web:

https://package-exports-analyzer-7jzl0pti8-patdx.vercel.app/solid-js/web?preset=cloudflare

(Sorry if there are bugs)

Cloudflare Wrangler uses the following options for esbuild::

{
  conditions: ['browser', 'worker'],
  platform: 'browser' // (default of esbuild)
}

Since both conditions can be matched, the worker condition needs to come first in the export map of solid-js/web for the best results.

I don't think it is really feasible to drop the "browser" condition/platform from the bundler config too, because these settings encourage modules like cross-fetch to prefer their browser-facing imports instead of node-facing imports.

I can't find a perfect example, but the best I could find, react-dom and @emotion/react put "worker" above "browser" too:

• https://package-exports-analyzer-7jzl0pti8-patdx.vercel.app/react-dom?preset=cloudflare
• https://package-exports-analyzer-7jzl0pti8-patdx.vercel.app/@emotion/react?preset=cloudflare

---

Interestingly Deno seemed to go for "deno", "node", "import" so it does not check "browser" and is probably fine where it is, though I did not check the CDN bundlers like Skypack, etc.

[ryansolid]

browser first is weird. Browser as a fallback makes sense. Only reason I put browser first is in case you actually wanted to override with a browser version in the worker. The way they are configured you can't actually do that which really makes no sense to me. But given this is the primary use case for worker for us that's probably fine to change.

[patdx]

Yeah I still have not fully wrapped my head around that part of conditional exports. I like the way aliases work(ed) because as a package consumer I could easily set aliases for some packages I cared about. But with export conditions, basically the bundler only takes one set of conditions that it will use to resolve all the dependencies so it seems like there may always be some whackamole when using global/universal condition names. Recently I noticed this section in webpack's guide and maybe it is the solution:

https://webpack.js.org/guides/package-exports/#conditions-custom

For custom conditions the following naming schema is recommended:
<company-name>:<condition-name>
Examples: example-corp:beta, google:internal.

You could have a for example solid:server and/or solid:client condition and put them at the very top of the exports list for anyone who really wants to override the default behavior.

[ryansolid]

Well changed the order in Solid 1.5.7. Maybe that helps?

[patdx]

@ryansolid Thank you for the update, it definitely helped!

I created a new project from scratch and added some of the problematic dependencies like @graphql-yoga/common and graphql-request

• Final server side bundle: https://github.com/patdx/solid-start-bundle-experiment/tree/main/dist-cloudflare
• Solid SSR App: https://solid-start-2022-09-30.workers.pmil.me/
• Solid SSR App GraphQL API route: https://solid-start-2022-09-30.workers.pmil.me/graphql

That said, I still did one major tweak. I changed the wrangler.toml like so:

- main = "./dist/server.js"
+ main = "./.solid/server/server.js"

This is because the bundled file at /dist/server.js still had some Node junk bundled by this this rollup config in solid-start-cloudflare-workers.

I think the solid-start-cloudflare-workers (and page?) adapters may need an update now. How do you prefer to approach this? I can see a few possibilities:

1. Skip the Rollup completely and let Wrangler do the final bundle
   • Could change the adapter to save the generated server.js to dist/server.js directly which should avoid any breaking changes with existing wrangler.toml files.
2. Keep Rollup and skip bundling node_modules (let Wrangler resolve them in the final step)
3. Keep Rollup and change the module resolution logic to match Wrangler (eg, resolve conditions "worker" and "browser" instead of "worker" and "node")

Personally I would suggest 1 because I think it is the simplest. I think there is value in letting Wrangler do the bundle with the actual module resolution because it will improve solid-start compatibility with any existing Cloudflare Workers tutorial. Also it would be faster to build by reducing the number of build steps from 3 to 2. One concern is what happens when referencing solid-js component libraries through node_modules. I will try to test this out.

[patdx]

I saw in the updated solid-start v0.1.2 is trying "Strategy 3". I updated my test repo and the build output by solid-start-cloudflare-worker at dist/server.js is still depending on a lot of node imports:

patdx/solid-start-bundle-experiment@52f51a7#diff-e12f76b77bea7a8627b75edc85084835099436485f9a53debce4476cfc4bddda

So unfortunately wrangler still throws an error when trying to process this file:

✘ [ERROR] Build failed with 25 errors:

  dist/server.js:2:21: ERROR: [plugin: checkForNodeBuiltins] Detected a Node builtin module import
  while Node compatibility is disabled.
  Add node_compat = true to your wrangler.toml file to enable Node compatibility.
  dist/server.js:3:19: ERROR: [plugin: checkForNodeBuiltins] Detected a Node builtin module import
  while Node compatibility is disabled.
  Add node_compat = true to your wrangler.toml file to enable Node compatibility.
  dist/server.js:4:16: ERROR: [plugin: checkForNodeBuiltins] Detected a Node builtin module import
  while Node compatibility is disabled.
  Add node_compat = true to your wrangler.toml file to enable Node compatibility.
  dist/server.js:5:25: ERROR: [plugin: checkForNodeBuiltins] Detected a Node builtin module import
  while Node compatibility is disabled.
  Add node_compat = true to your wrangler.toml file to enable Node compatibility.
  dist/server.js:6:20: ERROR: [plugin: checkForNodeBuiltins] Detected a Node builtin module import
  while Node compatibility is disabled.
  Add node_compat = true to your wrangler.toml file to enable Node compatibility.

Note, even using the fallback Wrangler mode node_compat = true did not work:

  dist/server.js:17:24: ERROR: Could not resolve "stream/web"
  dist/server.js:18:25: ERROR: Could not resolve "perf_hooks"
  dist/server.js:19:25: ERROR: Could not resolve "util/types"
  dist/server.js:22:23: ERROR: Could not resolve "async_hooks"

I think most of these Node imports are generated by various fetch polyfills for Node (undici/cross-fetch/etc). I think we should support the "browser" (pkg.browser, and pkg.exports[condition=browser])), like Wrangler's esbuild config, to make sure these polyfills do not get included.

I tried to patch the builder in separate branch here: https://github.com/patdx/solid-start-bundle-experiment/pull/2/files

           nodeResolve({
+            browser: true,
             preferBuiltins: true,
             exportConditions: ["worker", "solid"]
           }),

This fixed all the node imports ✔️ , and got the right renderToStream function ✔️ however it brought back the "pkg.browser overriding pkg.exports" conflict ❌ , so I had to patch solid-js too. :( So as of the moment, it seems that both Vite and
and @rollup/plugin-node-resolve allow pkg.browser to override pkg.exports. (Esbuild does not, which is why the module resolution works when piping the build to Wrangler directly.)

Here are some relevant PRs and Issues from Rollup:

• feat(node-resolve): support package entry points rollup/plugins#540

  • ...if a package has package entrypoints defined we ignore any main fields configuration, but we do allow the browser field to override the export map result. This isn't really a conscious decision, it's just a result of how the code is structured right now.

• fix(node-resolve): fix "browser" field mappings. rollup/plugins#786

  • I think we should modify this so that the "browser" exports should take precedence over the base-level "browser" field, as otherwise it will be too confusing when each will apply.

• fix(node-resolve): fix "browser" and "exports.browser" field mappings rollup/plugins#843
• Follow up to #843, refining exports and browser field interaction rollup/plugins#866
  • see the test case "exports.browser can be mapped via pkg.browser"

As far as I can tell this was kind of arbitrarily decided, maybe Rollup (and Vite) could be convinced to change their default behavior to always have pkg.exports take priority over pkg.browser based on solid-js's case (preserving backwards compatibility and test compatibility).

[patdx]

Hmm, I did a test of vite/esbuild/rollup/webpack and all of them except esbuild seem to have pkg.browser override pkg.exports https://github.com/patdx/package-exports-test. So that is the more popular rule at the moment. Of course I like esbuild's handling the best and I think it is the most reasonable logic, but does seem to have been a little bit of a lucky coincidence that Wrangler decided to use esbuild, thus it can resolve to all the most ideal imports for Cloudflare Workers environment.