fix: hotpatching fullstack (#4730)

* fix: hotpatching fullstack

* add hot-patching fullstack test

* add caching to playwright apt-get

* only run playwright on beta

* install binutils?

* dont commit lockfile

* ignore lockfiles

* make sure ld is in path

packages/cli/src/build/builder.rs

@@ -715,7 +715,7 @@ impl AppBuilder {
 
         let changed_file = changed_files.first().unwrap();
         tracing::info!(
-            "Hot-patching: {NOTE_STYLE}{}{NOTE_STYLE:#} took {GLOW_STYLE}{:?}ms{GLOW_STYLE:#} ({})",
+            "Hot-patching: {NOTE_STYLE}{}{NOTE_STYLE:#} took {GLOW_STYLE}{:?}ms{GLOW_STYLE:#}",
             changed_file
                 .display()
                 .to_string()
@@ -723,8 +723,7 @@ impl AppBuilder {
             SystemTime::now()
                 .duration_since(res.time_start)
                 .unwrap()
-                .as_millis(),
-            triple
+                .as_millis()
         );
 
         self.patches.push(jump_table.clone());

packages/cli/src/serve/mod.rs

@@ -175,27 +175,18 @@ pub(crate) async fn serve_all(args: ServeArgs, tracer: &TraceController) -> Resu
                     }
                     BuilderUpdate::BuildReady { bundle } => match bundle.mode {
                         BuildMode::Thin { ref cache, .. } => {
-                            let elapsed =
-                                bundle.time_end.duration_since(bundle.time_start).unwrap();
-                            match builder.hotpatch(&bundle, id, cache).await {
-                                Ok(jumptable) => {
-                                    let pid = match id {
-                                        BuildId::CLIENT => builder.client.pid,
-                                        _ => builder.server.as_ref().and_then(|s| s.pid),
-                                    };
-                                    devserver.send_patch(jumptable, elapsed, id, pid).await
-                                }
-                                Err(err) => {
-                                    tracing::error!("Failed to hot-patch app: {err}");
+                            if let Err(err) =
+                                builder.hotpatch(&bundle, id, cache, &mut devserver).await
+                            {
+                                tracing::error!("Failed to hot-patch app: {err}");
 
-                                    if let Some(_patching) =
-                                        err.downcast_ref::<crate::build::PatchError>()
-                                    {
-                                        tracing::info!("Starting full rebuild: {err}");
-                                        builder.full_rebuild().await;
-                                        devserver.send_reload_start().await;
-                                        devserver.start_build().await;
-                                    }
+                                if let Some(_patching) =
+                                    err.downcast_ref::<crate::build::PatchError>()
+                                {
+                                    tracing::info!("Starting full rebuild: {err}");
+                                    builder.full_rebuild().await;
+                                    devserver.send_reload_start().await;
+                                    devserver.start_build().await;
                                 }
                             }
                         }

packages/cli/src/serve/runner.rs

@@ -28,7 +28,6 @@ use std::{
     sync::Arc,
     time::Duration,
 };
-use subsecond_types::JumpTable;
 use syn::spanned::Spanned;
 use tokio::process::Command;
 
@@ -53,7 +52,7 @@ pub(crate) struct AppServer {
     pub(crate) watcher_rx: UnboundedReceiver<notify::Event>,
 
     // Tracked state related to open builds and hot reloading
-    pub(crate) applied_hot_reload_message: HotReloadMsg,
+    pub(crate) applied_client_hot_reload_message: HotReloadMsg,
     pub(crate) file_map: HashMap<PathBuf, CachedFile>,
 
     // Resolved args related to how we go about processing the rebuilds and logging
@@ -185,7 +184,7 @@ impl AppServer {
         // Create the runner
         let mut runner = Self {
             file_map: Default::default(),
-            applied_hot_reload_message: Default::default(),
+            applied_client_hot_reload_message: Default::default(),
             automatic_rebuilds: true,
             watch_fs,
             use_hotpatch_engine,
@@ -709,27 +708,66 @@ impl AppServer {
 
     pub(crate) async fn hotpatch(
         &mut self,
-        res: &BuildArtifacts,
+        bundle: &BuildArtifacts,
         id: BuildId,
         cache: &HotpatchModuleCache,
-    ) -> Result<JumpTable> {
+        devserver: &mut WebServer,
+    ) -> Result<()> {
+        let elapsed = bundle
+            .time_end
+            .duration_since(bundle.time_start)
+            .unwrap_or_default();
+
         let jump_table = match id {
-            BuildId::CLIENT => self.client.hotpatch(res, cache).await,
+            BuildId::CLIENT => self.client.hotpatch(bundle, cache).await,
             BuildId::SERVER => {
                 self.server
                     .as_mut()
                     .context("Server not found")?
-                    .hotpatch(res, cache)
+                    .hotpatch(bundle, cache)
                     .await
             }
             _ => bail!("Invalid build id"),
         }?;
 
         if id == BuildId::CLIENT {
-            self.applied_hot_reload_message.jump_table = self.client.patches.last().cloned();
+            self.applied_client_hot_reload_message.jump_table = self.client.patches.last().cloned();
         }
 
-        Ok(jump_table)
+        // If no server, just send the patch immediately
+        let Some(server) = self.server.as_mut() else {
+            devserver
+                .send_patch(jump_table, elapsed, id, self.client.pid)
+                .await;
+            return Ok(());
+        };
+
+        // If we have a server, we need to wait until both the client and server are ready
+        // Otherwise we end up with an annoying race condition where the client can't actually load the patch
+        if self.client.stage == BuildStage::Success && server.stage == BuildStage::Success {
+            let client_jump_table = self
+                .client
+                .patches
+                .last()
+                .cloned()
+                .context("Missing client jump table")?;
+
+            let server_jump_table = server
+                .patches
+                .last()
+                .cloned()
+                .context("Missing server jump table")?;
+
+            devserver
+                .send_patch(server_jump_table, elapsed, BuildId::SERVER, server.pid)
+                .await;
+
+            devserver
+                .send_patch(client_jump_table, elapsed, BuildId::CLIENT, self.client.pid)
+                .await;
+        }
+
+        Ok(())
     }
 
     pub(crate) fn get_build(&self, id: BuildId) -> Option<&AppBuilder> {
@@ -751,7 +789,7 @@ impl AppServer {
 
     /// Get any hot reload changes that have been applied since the last full rebuild
     pub(crate) fn applied_hot_reload_changes(&mut self, build: BuildId) -> HotReloadMsg {
-        let mut msg = self.applied_hot_reload_message.clone();
+        let mut msg = self.applied_client_hot_reload_message.clone();
 
         if build == BuildId::CLIENT {
             msg.jump_table = self.client.patches.last().cloned();
@@ -773,7 +811,7 @@ impl AppServer {
 
     /// Clear the hot reload changes. This should be called any time a new build is starting
     pub(crate) fn clear_hot_reload_changes(&mut self) {
-        self.applied_hot_reload_message = Default::default();
+        self.applied_client_hot_reload_message = Default::default();
     }
 
     pub(crate) fn clear_patches(&mut self) {
@@ -836,7 +874,7 @@ impl AppServer {
 
     /// Store the hot reload changes for any future clients that connect
     fn add_hot_reload_message(&mut self, msg: &HotReloadMsg) {
-        let applied = &mut self.applied_hot_reload_message;
+        let applied = &mut self.applied_client_hot_reload_message;
 
         // Merge the assets, unknown files, and templates
         // We keep the newer change if there is both a old and new change

packages/playwright-tests/playwright.config.js

@@ -207,5 +207,13 @@ module.exports = defineConfig({
       reuseExistingServer: !process.env.CI,
       stdout: "pipe",
     },
+    {
+      command:
+        'rm -rf ./web-hot-patch-fullstack-temp && cp -r ./web-hot-patch-fullstack ./web-hot-patch-fullstack-temp && cd web-hot-patch-fullstack-temp && cargo run --manifest-path ../../cli/Cargo.toml --release -- serve --verbose --force-sequential --web --addr "127.0.0.1" --port 9981 --hot-patch --exit-on-error',
+      port: 9981,
+      timeout: 50 * 60 * 1000,
+      reuseExistingServer: !process.env.CI,
+      stdout: "pipe",
+    },
   ],
 });

packages/playwright-tests/web-hot-patch-fullstack/.gitignore

@@ -0,0 +1,2 @@
+dist
+target

packages/playwright-tests/web-hot-patch-fullstack/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "dioxus-playwright-fullstack-patch-test"
+version = "0.0.1"
+edition = "2021"
+description = "Playwright test for Dioxus Web"
+license = "MIT OR Apache-2.0"
+publish = false
+
+[dependencies]
+dioxus = { path = "../../dioxus", features = ["fullstack"] }
+
+[features]
+web = ["dioxus/web"]
+server = ["dioxus/server"]
+
+[workspace]

packages/playwright-tests/web-hot-patch-fullstack/assets/alternative-style.css

@@ -0,0 +1,3 @@
+body {
+  background-color: rgb(100, 100, 100);
+}

packages/playwright-tests/web-hot-patch-fullstack/assets/style.css

@@ -0,0 +1,3 @@
+#main {
+  background-color: red;
+}

packages/playwright-tests/web-hot-patch-fullstack/src/main.rs

@@ -0,0 +1,36 @@
+use dioxus::prelude::*;
+
+const CSS: Asset = asset!("/assets/style.css");
+const IMAGE: Asset = asset!("/assets/toasts.png");
+
+fn app() -> Element {
+    let mut num = use_signal(|| 0);
+
+    rsx! {
+        document::Link {
+            href: CSS,
+            rel: "stylesheet",
+        }
+        img {
+            id: "toasts",
+            src: IMAGE,
+        }
+        button {
+            id: "increment-button",
+            onclick: move |_| async move {
+                let increment_amount = get_count().await.unwrap();
+                *num.write() += increment_amount;
+            },
+            "Click me! Count: {num}"
+        }
+    }
+}
+
+#[post("/api/get_count")]
+async fn get_count() -> Result<i32> {
+    Ok(1)
+}
+
+fn main() {
+    dioxus::launch(app);
+}

packages/playwright-tests/web-hot-patch/Cargo.toml

@@ -7,6 +7,6 @@ license = "MIT OR Apache-2.0"
 publish = false
 
 [dependencies]
-dioxus = { path = "../../dioxus", features = ["web"]}
+dioxus = { path = "../../dioxus", features = ["web"] }
 
-[workspace]
+[workspace]

packages/playwright-tests/web-patch-fullstack.spec.js

@@ -0,0 +1,113 @@
+// @ts-check
+const { test, expect } = require("@playwright/test");
+
+const hotPatchTimeout = {
+  timeout: 1000 * 60 * 2, // 2 minute
+};
+
+test("button click", async ({ page }) => {
+  const fs = require("fs");
+  const mainPath = "web-hot-patch-fullstack-temp/src/main.rs";
+  var mainContent = fs.readFileSync(mainPath, "utf8");
+  const stylePath = "web-hot-patch-fullstack-temp/assets/style.css";
+  var styleContent = fs.readFileSync(stylePath, "utf8");
+
+  // Reset any changes made to the main.rs and style.css files.
+  mainContent = mainContent.replace(/Ok\(\s*2\s*\)/g, "Ok(1)");
+  mainContent = mainContent.replace("Click button! Count:", "Click me! Count:");
+  mainContent = mainContent.replace("asset!('/assets/alternative-style.css')", "asset!('/assets/style.css')");
+  fs.writeFileSync(mainPath, mainContent);
+  styleContent = styleContent.replace(
+    "background-color: blue;",
+    "background-color: red;"
+  );
+  fs.writeFileSync(stylePath, styleContent);
+
+  await page.goto("http://localhost:9981");
+
+
+  // wait a sec for the serverfn to process
+  // give the page time to finish loading resources
+  await page.waitForLoadState("networkidle");
+  await page.waitForTimeout(2000);
+
+  // ** First test make sure the initial fat build is working **
+  // Expect the page to contain the counter text.
+  const main = page.locator("#main");
+  await expect(main).toContainText("Click me! Count: 0");
+
+  // Click the increment button.
+  let button = page.locator("button#increment-button");
+  await button.click();
+
+  await page.waitForTimeout(1000);
+
+  // Expect the page to contain the updated counter text.
+  await expect(main).toContainText("Click me! Count: 1");
+
+  // Make sure the css is applied correctly.
+  await expect(main).toHaveCSS("background-color", "rgb(255, 0, 0)");
+
+  // Make sure the image is loaded.
+  const headerImage = page.locator("#toasts");
+  // expect the attribute src to start with /assets/toasts-
+  await expect(headerImage).toHaveAttribute("src", /\/assets\/toasts-/);
+
+  // ** Then make sure the hot patch is working **
+  // Then change the file to increment by 2.
+  const updatedContent = mainContent.replace(/Ok\(\s*1\s*\)/g, "Ok(2)");
+  // Change the click me text to reflect the new increment.
+  const updatedContentWithText = updatedContent.replace(
+    "Click me! Count:",
+    "Click button! Count:"
+  );
+  fs.writeFileSync(mainPath, updatedContentWithText);
+
+  // Wait for the page to update and show the new text.
+  await expect(main).toContainText("Click button! Count: 1", hotPatchTimeout);
+
+  // Now click the button again.
+  await button.click();
+
+  // wait a sec for the serverfn to process
+  await page.waitForTimeout(1000);
+
+  // Expect the count to update by 2.
+  await expect(main).toContainText("Click button! Count: 3");
+
+  // Next change just the css file to change the background color to blue.
+  const updatedStyleContent = styleContent.replace(
+    "background-color: red;",
+    "background-color: blue;"
+  );
+  fs.writeFileSync(stylePath, updatedStyleContent);
+
+  // Wait for the page to update the background color.
+  await expect(main).toHaveCSS(
+    "background-color",
+    "rgb(0, 0, 255)",
+    hotPatchTimeout
+  );
+
+  // Make sure the image is still loaded.
+  // expect the attribute src to start with /assets/toasts-
+  await expect(headerImage).toHaveAttribute("src", /\/assets\/toasts-/);
+
+  // ** Then add a new asset to the page **
+  // Switch from style to alternative-style
+  const updatedContentWithAlternativeStyle = updatedContentWithText.replace(
+    'asset!("/assets/style.css")',
+    'asset!("/assets/alternative-style.css")'
+  );
+  fs.writeFileSync(mainPath, updatedContentWithAlternativeStyle);
+
+  // Assert the page has the new alternative style applied.
+  const body = page.locator("body");
+  // Log the page content to debug if needed.
+  console.log(await page.content());
+  await expect(body).toHaveCSS(
+    "background-color",
+    "rgb(100, 100, 100)",
+    hotPatchTimeout
+  );
+});

packages/subsecond/subsecond/src/lib.rs

@@ -573,6 +573,16 @@ pub unsafe fn apply_patch(mut table: JumpTable) -> Result<(), PatchError> {
 
         // Wait for the fetch to complete - we need the wasm module size in bytes to reserve in the memory
         let response: web_sys::Response = JsFuture::from(response).await.unwrap().unchecked_into();
+
+        // If the status is not success, we bail
+        if !response.ok() {
+            panic!(
+                "Failed to patch wasm module at {} - response failed with: {}",
+                path,
+                response.status_text()
+            );
+        }
+
         let dl_bytes: ArrayBuffer = JsFuture::from(response.array_buffer().unwrap())
             .await
             .unwrap()