Restore attribute "refresh=morph" to flag turbo frames to reload duri…

…ng a page refresh. This adds a new turbo-frame attribute: `refresh`. When its value is `morph`: * It will reload the turbo frame with morphing during a page refresh. * It won't update the turbo frame with the server response. * It won't remove it if it's missing in the server response. This attribute was part of the original proposal we presented in Rails World, then we removed it [1], because we thought it wasn't needed. But after testing the library in different scenarios, we've found assuming certain behavior for all the remote frames was problematic since it implied being too clever about what you wanted to do with the frame. The new attribute makes for a simple behavior: * The default behavior will be the expected one: turbo frames will be morphed as any other element. If they get a new URL they will be reloadded, if the get deleted in the response, they will disappear, etc. * You can use the new attribute to flag the frames for which you want the special behavior. [1]: 0c6a95d
hotwired · Nov 17, 2023 · 9ff6e16 · 9ff6e16
1 parent a247b35
commit 9ff6e16
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 25 deletions.
diff --git a/src/core/drive/morph_renderer.js b/src/core/drive/morph_renderer.js
@@ -1,6 +1,5 @@
 import Idiomorph from "idiomorph"
 import { dispatch } from "../../util"
-import { urlsAreEqual } from "../url"
 import { Renderer } from "../renderer"
 
 export class MorphRenderer extends Renderer {
@@ -27,7 +26,7 @@ export class MorphRenderer extends Renderer {
   }
 
   #morphElements(currentElement, newElement, morphStyle = "outerHTML") {
-    this.isMorphingTurboFrame = this.#remoteFrameReplacement(currentElement, newElement)
+    this.isMorphingTurboFrame = this.#isFrameReloadedWithMorph(currentElement)
 
     Idiomorph.morph(currentElement, newElement, {
       morphStyle: morphStyle,
@@ -44,27 +43,20 @@ export class MorphRenderer extends Renderer {
   }
 
   #shouldMorphElement = (oldNode, newNode) => {
-    if (!(oldNode instanceof HTMLElement) || this.isMorphingTurboFrame) {
-      return true
-    }
-    else if (oldNode.hasAttribute("data-turbo-permanent")) {
-      return false
+    if (oldNode instanceof HTMLElement) {
+      return !oldNode.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isFrameReloadedWithMorph(oldNode))
     } else {
-      return !this.#remoteFrameReplacement(oldNode, newNode)
+      return true
     }
   }
 
-  #remoteFrameReplacement = (oldNode, newNode) => {
-    return this.#isRemoteFrame(oldNode) && this.#isRemoteFrame(newNode) && urlsAreEqual(oldNode.getAttribute("src"), newNode.getAttribute("src"))
-  }
-
   #shouldRemoveElement = (node) => {
     return this.#shouldMorphElement(node)
   }
 
   #reloadRemoteFrames() {
     this.#remoteFrames().forEach((frame) => {
-      if (this.#isRemoteFrame(frame)) {
+      if (this.#isFrameReloadedWithMorph(frame)) {
         this.#renderFrameWithMorph(frame)
         frame.reload()
       }
@@ -85,8 +77,8 @@ export class MorphRenderer extends Renderer {
     this.#morphElements(currentElement, newElement.children, "innerHTML")
   }
 
-  #isRemoteFrame(node) {
-    return node instanceof HTMLElement && node.nodeName.toLowerCase() === "turbo-frame" && node.getAttribute("src")
+  #isFrameReloadedWithMorph(element) {
+    return element.src && element.refresh === "morph"
   }
 
   #remoteFrames() {

diff --git a/src/tests/fixtures/frame_refresh_morph.html b/src/tests/fixtures/frame_refresh_morph.html
@@ -1,3 +1,3 @@
-<turbo-frame id="remote-frame">
+<turbo-frame id="refresh-morph">
   <h2>Loaded morphed frame</h2>
 </turbo-frame>
diff --git a/src/tests/fixtures/frame_refresh_reload.html b/src/tests/fixtures/frame_refresh_reload.html
@@ -0,0 +1,3 @@
+<turbo-frame id="refresh-reload">
+  <h2>Loaded reloadable frame</h2>
+</turbo-frame>
diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html
@@ -24,10 +24,14 @@
   <body>
     <h1>Page to be refreshed</h1>
 
-    <turbo-frame id="remote-frame" src="/src/tests/fixtures/frame_refresh_morph.html">
+    <turbo-frame id="refresh-morph" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
       <h2>Frame to be morphed</h2>
     </turbo-frame>
 
+    <turbo-frame id="refresh-reload" src="/src/tests/fixtures/frame_refresh_reload.html" refresh="reload">
+      <h2>Frame to be reloaded</h2>
+    </turbo-frame>
+
     <div id="preserve-me" data-turbo-permanent>
       Preserve me!
 

diff --git a/src/tests/fixtures/page_refresh_replace.html b/src/tests/fixtures/page_refresh_replace.html
@@ -23,10 +23,14 @@
   <body>
     <h1>Page to be refreshed</h1>
 
-    <turbo-frame id="remote-frame" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
+    <turbo-frame id="refresh-morph" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
       <h2>Frame to be morphed</h2>
     </turbo-frame>
 
+    <turbo-frame id="refresh-reload" src="/src/tests/fixtures/frame_refresh_reload.html" refresh="reload">
+      <h2>Frame to be reloaded</h2>
+    </turbo-frame>
+
     <div id="preserve-me" data-turbo-permanent>
       Preserve me!
     </div>

diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js
@@ -33,16 +33,21 @@ test("doesn't morph when the navigation doesn't go to the same URL", async ({ pa
   expect(await noNextEventNamed(page, "turbo:render", { renderMethod: "morph" })).toBeTruthy()
 })
 
-test("uses morphing to update remote frames", async ({ page }) => {
+test("uses morphing to update remote frames marked with refresh='morph'", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")
 
   await page.click("#form-submit")
   await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
   await nextBeat()
 
   // Only the frame marked with refresh="morph" uses morphing
-  expect(await nextEventOnTarget(page, "remote-frame", "turbo:before-frame-morph")).toBeTruthy()
-  await expect(page.locator("#remote-frame")).toHaveText("Loaded morphed frame")
+  expect(await nextEventOnTarget(page, "refresh-morph", "turbo:before-frame-morph")).toBeTruthy()
+  expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy()
+
+  await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
+
+  // Regular turbo-frames also gets reloaded since their complete attribute is removed
+  await expect(page.locator("#refresh-reload")).toHaveText("Loaded reloadable frame")
 })
 
 test("don't refresh frames contained in [data-turbo-permanent] elements", async ({ page }) => {
@@ -56,17 +61,17 @@ test("don't refresh frames contained in [data-turbo-permanent] elements", async
   expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy()
 })
 
-test("remote frames are excluded from full page morphing", async ({ page }) => {
+test("frames marked with refresh='morph' are excluded from full page morphing", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")
 
-  await page.evaluate(() => document.getElementById("remote-frame").setAttribute("data-modified", "true"))
+  await page.evaluate(() => document.getElementById("refresh-morph").setAttribute("data-modified", "true"))
 
   await page.click("#form-submit")
   await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
   await nextBeat()
 
-  await expect(page.locator("#remote-frame")).toHaveAttribute("data-modified", "true")
-  await expect(page.locator("#remote-frame")).toHaveText("Loaded morphed frame")
+  await expect(page.locator("#refresh-morph")).toHaveAttribute("data-modified", "true")
+  await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
 })
 
 test("it preserves the scroll position when the turbo-refresh-scroll meta tag is 'preserve'", async ({ page }) => {