workers-sdk icon indicating copy to clipboard operation
workers-sdk copied to clipboard

🚀 Feature Request: Add `live-reload` option to `wrangler dev`

Open yusukebe opened this issue 3 years ago • 5 comments

I really want --live-reload option for wrangler dev. I know wrangler pages dev already have --live-reload option by PR #220 .

Not only using Pages, I often build HTML pages with Workers. I think there are some developers like me. For example, some frameworks that support Cloudflare Workers will be rendering HTML on the server-side. Remix works on Workers not only Pages (Although the reload function in Remix is implemented originally not using Miniflare's). create-remix app asks me to deploy to Workers or others.

SS

Despite Remix, the framework I am building also supports HTML rendering.

I believe it'll be a good feature.

yusukebe avatar Jun 02 '22 09:06 yusukebe

We definitely want to do this, but it's tricky because we don't "own" the html being rendered. An idea I had is to provide a low level primitive, for example a websocket that pushes messages anytime we reload. Additionally, a script tag/snippet that developers can add into their html. This script would listen on this socket and simply reload the page whenever it receives a message. Frameworks like remix etc could add this snippet by default, but it shouldn't be too hard for developers to do themselves.

(To clarify, this isn't about hot reload, which would not reload the page but would swap in modules. That might be a lot more complicated, but we have a couple of ideas there too, the simplest being that metaframeworks like next.js already provide network channels for this, and we could figure out a way to proxy it correctly)

threepointone avatar Jun 02 '22 11:06 threepointone

HI @threepointone !

How about implementing --live-reload on only "local mode"? It may be easy. Miniflare included in Wrangler has the --live-reload option already. Using WebSocket, being enabled only if content-type is text/html, and inserting the script into HTML. The patch below will enable --live-reload only local mode. It works.

diff --git a/packages/wrangler/src/dev/dev.tsx b/packages/wrangler/src/dev/dev.tsx
index e6ca4c1..c04de3f 100644
--- a/packages/wrangler/src/dev/dev.tsx
+++ b/packages/wrangler/src/dev/dev.tsx
@@ -60,6 +60,7 @@ export type DevProps = {
         host: string;
       }
     | undefined;
+  liveReload: boolean;
 };
 
 export function DevImplementation(props: DevProps): JSX.Element {
@@ -176,6 +177,7 @@ function DevSession(props: DevSessionProps) {
       inspectorPort={props.inspectorPort}
       enableLocalPersistence={props.enableLocalPersistence}
       crons={props.crons}
+      liveReload={props.liveReload}
     />
   ) : (
     <Remote
diff --git a/packages/wrangler/src/dev/local.tsx b/packages/wrangler/src/dev/local.tsx
index 7f86792..e8c76d3 100644
--- a/packages/wrangler/src/dev/local.tsx
+++ b/packages/wrangler/src/dev/local.tsx
@@ -29,6 +29,7 @@ interface LocalProps {
   inspectorPort: number;
   enableLocalPersistence: boolean;
   crons: Config["triggers"]["crons"];
+  liveReload: boolean;
 }
 
 export function Local(props: LocalProps) {
@@ -55,6 +56,7 @@ function useLocalWorker({
   enableLocalPersistence,
   ip,
   crons,
+  liveReload,
 }: LocalProps) {
   // TODO: pass vars via command line
   const local = useRef<ReturnType<typeof spawn>>();
@@ -214,6 +216,7 @@ function useLocalWorker({
         sourceMap: true,
         logUnhandledRejections: true,
         crons,
+        liveReload,
       };
 
       // The path to the Miniflare CLI assumes that this file is being run from
diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx
index 2fa97d6..8a8a753 100644
--- a/packages/wrangler/src/index.tsx
+++ b/packages/wrangler/src/index.tsx
@@ -973,6 +973,10 @@ function createCLIParser(argv: string[]) {
           type: "boolean",
           describe: "Use legacy environments",
           hidden: true,
+        })
+        .option("live-reload", {
+          type: "boolean",
+          describe: "Enable live-reload (only for local mode)",
         });
     },
     async (args) => {
@@ -1230,6 +1234,7 @@ function createCLIParser(argv: string[]) {
           usageModel={config.usage_model}
           bindings={bindings}
           crons={config.triggers.crons}
+          liveReload={args["live-reload"] || false}
         />
       );
       await waitUntilExit();

But if you don't like it, feel free to close this issue.

PS. Wrangler2 is fantastic! I love it. Thank you for developing.

yusukebe avatar Jun 02 '22 23:06 yusukebe

When we do this, we'll likely want it for both local and remote modes. Miniflare injecting it into every html response is clever, but could be bad when the entire page is being rendered /hydrated by a framework (like with react's new streaming renderer). We'll see. Happy to keep this issue open while we discuss options and execute, it just may not happen immediately.

Thank you for the kind words on wrangler! Please do share feedback. I love hono too, it's very cool!

threepointone avatar Jun 03 '22 08:06 threepointone

Unassigning myself since I'm going on leave for a bit, open to whoever else wants a crack at this

threepointone avatar Jul 28 '22 08:07 threepointone

see https://github.com/cloudflare/wrangler2/issues/1185#issuecomment-1204210411 for note on why this is blocked

lrapoport-cf avatar Sep 14 '22 14:09 lrapoport-cf

@yusukebe a --live-reload option for local mode development was added in v2.4.1 (https://github.com/cloudflare/wrangler2/pull/2214). does this address the request?

lrapoport-cf avatar Dec 15 '22 20:12 lrapoport-cf

Hi @lrapoport-cf !

Cool! I did not know the option. It works well. There is nothing more to say. Now I can get on with development. Thank you!

yusukebe avatar Dec 15 '22 21:12 yusukebe