Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloudflare adapter disables known cloudflare worker "compatible" dependencies #263

Closed
patdx opened this issue Sep 1, 2022 · 24 comments
Closed
Labels
upstream Issue waiting on dependencies

Comments

@patdx
Copy link
Contributor

patdx commented Sep 1, 2022

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

@nksaraf
Copy link
Member

nksaraf commented Sep 2, 2022

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
Copy link
Member

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
Copy link
Contributor Author

patdx commented Sep 6, 2022

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
Copy link
Contributor Author

patdx commented Sep 7, 2022

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
Copy link
Member

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
Copy link
Contributor Author

patdx commented Sep 7, 2022

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
Copy link
Member

nksaraf commented Sep 17, 2022

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
Copy link
Contributor Author

patdx commented Sep 21, 2022

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
Copy link
Contributor Author

patdx commented Sep 28, 2022

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:


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
Copy link
Member

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
Copy link
Contributor Author

patdx commented Sep 28, 2022

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
Copy link
Member

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

@patdx
Copy link
Contributor Author

patdx commented Sep 30, 2022

@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

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
Copy link
Contributor Author

patdx commented Oct 3, 2022

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:

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
Copy link
Contributor Author

patdx commented Oct 3, 2022

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.

@patdx
Copy link
Contributor Author

patdx commented Oct 3, 2022

I saw some new tickets created for rollup and vite. In case it may help I created an alternative branch of my last test here, browser-override-issue with the rollup config patch only. This branch demonstrates the failing build. This can be verified by the existence of browser-only code for Solid.js, like Effects.push inside of dist/server.js).

@guybedford
Copy link

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.

@patdx if you run esbuild with the "browser" condition it will definitely have the same behaviour. The key thing is that the "browser" condition must be set. Note that esbuild actually was the first to implement this and other bundlers (Rollup and Webpack) followed.

@patdx
Copy link
Contributor Author

patdx commented Oct 4, 2022

@patdx if you run esbuild with the "browser" condition it will definitely have the same behaviour. The key thing is that the "browser" condition must be set. Note that esbuild actually was the first to implement this and other bundlers (Rollup and Webpack) followed.

@guybedford I wonder if we are talking about the same thing or maybe I missed something in my esbuild config? I did a test here: https://github.com/patdx/package-exports-test/blob/main/out/esbuild/simple.js Esbuild seems to resolve to browser.js, so it uses the value from pkg.exports directly. This behavior is preferred for solid-js. But some other bundlers like Vite seem to resolve to browser-override.js. Vite is taking the value resolved from pkg.exports then applying the pkg.browser map on top of it. This is not preferred for solid-js.

esbuild resolution

@guybedford
Copy link

@patdx thanks for putting together this clear test repo, that's certainly information for me. I've posted evanw/esbuild#2593 for further discussion.

@patdx
Copy link
Contributor Author

patdx commented Oct 24, 2022

There hasn't been much action on this issue so let me try to summarize the current status:

The resolution behavior of different bundlers is unchanged since the creation of this issue:

https://github.com/patdx/package-exports-test

ESBuild still resolves in the way that is most helpful to solid-js project, and the other bundlers tested do not.

There have been three related issues kindly created by Ryan and Guy:

evanw/esbuild#2593
rollup/plugins#1307
vitejs/vite#10323

Unfortunately does not seem to be much traction on them at the moment. I'm not quite sure what to do next or how to get this issue moved closer to a solution. Is there some good way to get it prioritized for discussion?

@guybedford
Copy link

@patdx to add some context, Webpack I believe was the source of this behaviour, where RollupJS followed Webpack in discussions with @sokra who had good reasons for it. We can revert that behaviour in RollupJS to instead follow ES Build. I'm open to further discussions on the appropriate issuess.

@ryansolid ryansolid added the upstream Issue waiting on dependencies label Dec 19, 2022
@balupton
Copy link

Cross-linking this issue as seems to be related:

evanw/esbuild#2952

@peterhirn
Copy link

peterhirn commented Jun 27, 2023

All patches required to make SolidStart work on Cloudflare Workers right now for anyone interested. Use pnpm patch or yarn patch to apply them to your project. Work in progress, I might update this comment later if I find more infos.

Updated 2023-09-03 for solid-start v0.3.5

solid-js@1.7.6.patch

diff --git a/package.json b/package.json
index f0a0483df966f0e66eff7c9b88e8b2bfc022ddbb..0b0686e37e5354bcf8ec58940a543423bde7f02b 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,
diff --git a/store/package.json b/store/package.json
index 252dd14b8c3c4b9c2bd84cc9c117176912b5b223..ffd2889042a12c4ca352779ddc84c8f571789060 100644
--- a/store/package.json
+++ b/store/package.json
@@ -2,10 +2,6 @@
   "name": "solid-js/store",
   "main": "./dist/server.cjs",
   "module": "./dist/server.js",
-  "browser": {
-    "./dist/server.cjs": "./dist/store.cjs",
-    "./dist/server.js": "./dist/store.js"
-  },
   "unpkg": "./dist/store.cjs",
   "types": "./types/index.d.ts",
   "type": "module",
diff --git a/web/package.json b/web/package.json
index fee7a002f4418f67b2f9f2c6fd202e2c94fc5e21..f4b2a9881a58ecf0f5d3412d6d415e57579e9f74 100644
--- a/web/package.json
+++ b/web/package.json
@@ -2,10 +2,6 @@
   "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",

solid-start-cloudflare-workers@0.3.5.patch

diff --git a/index.js b/index.js
index 8586eeccb3f41f8d84a1995f3bdb1b7bb793fa33..2cc145a25b887baad9cc9dd303a6adf6a5aa8c45 100644
--- a/index.js
+++ b/index.js
@@ -88,7 +88,7 @@ export default function (miniflareOptions = {}) {
               Request,
               Response,
               fetch,
-              crypto,
+              ...(!globalThis.crypto && { crypto }),
               Headers,
               ReadableStream,
               WritableStream,
@@ -197,6 +197,7 @@ export default function (miniflareOptions = {}) {
         plugins: [
           json(),
           nodeResolve({
+            browser: true,
             preferBuiltins: true,
             exportConditions: ["worker", "solid"]
           }),

NOTE: the following patch is only required if you want passing typechecks (tsc --noEmit). Additionally I'm patching the options for renderToStringAsync so I can use Content-Security-Policy, see #924

solid-start@0.3.5.patch

diff --git a/api/internalFetch.ts b/api/internalFetch.ts
index eea745dfaa9e31a12a3e22cf343d1c06ee0d7b2f..c541c0df6e9e0c9bfdc8f8ac22da6a92f1fd49b1 100644
--- a/api/internalFetch.ts
+++ b/api/internalFetch.ts
@@ -8,6 +8,7 @@ export const registerApiRoutes = (routes: MatchRoute[]) => {
   apiRoutes = routes;
 };
 
+// @ts-ignore
 export async function internalFetch(route: string, init: RequestInit, env: Env = {}, locals: Record<string, unknown> = {}) {
   if (route.startsWith("http")) {
     return await fetch(route, init);
diff --git a/server/middleware.ts b/server/middleware.ts
index b605421621b9636507ac3472add36dbd6702f082..6013558c8cac208b78e5e5780120cc18b424f737 100644
--- a/server/middleware.ts
+++ b/server/middleware.ts
@@ -67,6 +67,7 @@ export const inlineServerFunctions: ServerMiddleware = ({ forward }) => {
                   JSON.stringify({
                     url: url.pathname,
                     entries: entries,
+                    // @ts-ignore
                     ...(await serverResponse.json())
                   })
                 )
diff --git a/server/render.ts b/server/render.ts
index 8b2e8dc36cbdd36581e5ff98fe716d5fc000f419..313a294e44ef585ea6759b3c2a6226b7f5984a37 100644
--- a/server/render.ts
+++ b/server/render.ts
@@ -68,7 +68,8 @@ export function renderAsync(
 
         let pageEvent = createPageEvent(event);
 
-        let markup = await renderToStringAsync(() => fn(pageEvent), options);
+        // @ts-ignore
+        let markup = await renderToStringAsync(() => fn(pageEvent), event.locals.options);
 
         if (pageEvent.routerContext && pageEvent.routerContext.url) {
           return redirect(pageEvent.routerContext.url, {

NOTE: another patch only required if you want passing typechecks, see natemoo-re/micromorph#22

micromorph@0.4.5.patch

diff --git a/package.json b/package.json
index 3251a6ca39f860b6aeb377afff63b57339a4f24c..b167449d14c777a4a644d9bd427400c794656cf1 100644
--- a/package.json
+++ b/package.json
@@ -2,9 +2,12 @@
   "name": "micromorph",
   "version": "0.4.5",
   "main": "./index.mjs",
-  "types": "./types.d.ts",
+  "types": "./index.d.ts",
   "exports": {
-    ".": "./index.mjs",
+    ".": {
+      "default": "./index.mjs",
+      "types": "./index.d.ts"
+    },
     "./spa": "./dist/spa.js",
     "./nav": "./dist/nav.js"
   },

package.json

  "pnpm": {
    "overrides": {
      "micromorph@<0.4.5": "^0.4.5"
    },
    "patchedDependencies": {
      "solid-js@1.7.11": "patches/solid-js@1.7.11.patch",
      "solid-start@0.3.5": "patches/solid-start@0.3.5.patch",
      "solid-start-cloudflare-workers@0.3.5": "patches/solid-start-cloudflare-workers@0.3.5.patch",
      "micromorph@0.4.5": "patches/micromorph@0.4.5.patch"
    }
  }

pnpm dev requires changes to vite.config.ts to automatically load configuration from .env, package.json and wrangler.toml

import { defineConfig } from "vite"
import solid from "solid-start/vite"
import cloudflare from "solid-start-cloudflare-workers"

export default defineConfig({
  plugins: [
    solid({
      adapter: cloudflare({
        envPath: true,
        packagePath: true,
        wranglerConfigPath: true,
        buildCommand: ""
      })
    })
  ]
})

pnpm start automatically loads configuration out of the box.

KV values are persisted in two different folders: dev -> .mf/kv/.., start -> .wrangler/state/v3/..

I'm currently looking into https://github.com/cloudflare/workers-sdk and try to make pnpm dev use workerd as well.

Miniflare v3 was just released https://github.com/cloudflare/miniflare/releases and comes with a long list of breaking API changes https://miniflare.dev/get-started/migrating#api-changes.

I did a quick test upgrading solid-start-cloudflare-workers, unfortunately v3 doesn't support globals which is where all the websocket/vite setup is implemented: https://github.com/solidjs/solid-start/blob/main/packages/start-cloudflare-workers/index.js#L28

Update 2023-09-17 Ryan recently announced #1052 that all adapters will be removed and replaced by https://github.com/unjs/nitro in SolidStart Beta 2. Unfortunately Nitro is also using miniflare v2 at the moment.

@ryansolid
Copy link
Member

In setting up for SolidStarts next Beta Phase built on Nitro and Vinxi we are closing all PRs/Issues that will not be merged due to the system changing. If you feel your issue was closed in mistake. Feel free to re-open it after updating/testing against 0.4.x release. Thank you for your patience.

See #1139 for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Issue waiting on dependencies
Projects
None yet
Development

No branches or pull requests

6 participants