Use 'will render' false for action 'replace' after GET redirect (#516)

* fix(visit): will render false for action 'replace' after redirect * feat(eager frame): add test for check fetch eager frame count * ref(tests): fix eslint errors * ref(test): fix eslint Insert errors * ref(eslint): fix all errors from eslint in branch Co-authored-by: David Heinemeier Hansson <david@hey.com>
hotwired · Jun 19, 2022 · 88bf29a · 88bf29a
1 parent fa6fd3b
commit 88bf29a
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 0 deletions.
diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts
@@ -281,6 +281,7 @@ export class Visit implements FetchRequestDelegate {
     if (this.redirectedToLocation && !this.followedRedirect && this.response?.redirected) {
       this.adapter.visitProposedToLocation(this.redirectedToLocation, {
         action: "replace",
+        willRender: false,
         response: this.response,
       })
       this.followedRedirect = true

diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html
@@ -121,5 +121,7 @@ <h2>Frames: #nested-child</h2>
     <hr class="push-off-screen">
     <input id="below-the-fold-input">
     <a id="below-the-fold-link-frame-action" data-turbo-action="advance" data-turbo-frame="frame" href="/src/tests/fixtures/frames/frame.html">Navigate #frame</a>
+
+    <a id="link-to-eager-loaded-frame" href="/__turbo/redirect?path=/src/tests/fixtures/page_with_eager_frame.html">Eager-loaded frame after GET redirect</a>
   </body>
 </html>
diff --git a/src/tests/fixtures/frames/frame_for_eager.html b/src/tests/fixtures/frames/frame_for_eager.html
@@ -0,0 +1,3 @@
+<turbo-frame id="eager-loaded-frame" >
+  <h2>Eager-loaded frame: Loaded</h2>
+</turbo-frame>
diff --git a/src/tests/fixtures/page_with_eager_frame.html b/src/tests/fixtures/page_with_eager_frame.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html id="html" data-skip-event-details="turbo:before-render">
+  <head>
+    <meta charset="utf-8">
+    <title>Frame</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <h1>Eager-loaded frame</h1>
+
+    <turbo-frame id="eager-loaded-frame" src="/src/tests/fixtures/frames/frame_for_eager.html" loading="eager">
+      <h2>Eager-loaded frame: NOT Loaded...</h2>
+    </turbo-frame>
+
+
+    <a href="/src/tests/fixtures/frames.html">Page /src/tests/fixtures/frames.html</a>
+  </body>
+</html>
diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts
@@ -579,6 +579,29 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-response"))
   }
 
+  async "test navigating a eager frame with a link[method=get] that does not fetch eager frame twice"() {
+    await this.clickSelector("#link-to-eager-loaded-frame")
+
+    await this.nextBeat
+
+    const eventLogs = await this.eventLogChannel.read()
+    const fetchLogs = eventLogs.filter(
+      ([name, options]) =>
+        name == "turbo:before-fetch-request" &&
+        options?.url?.includes("/src/tests/fixtures/frames/frame_for_eager.html")
+    )
+    this.assert.equal(fetchLogs.length, 1)
+
+    const src = (await this.attributeForSelector("#eager-loaded-frame", "src")) ?? ""
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame_for_eager.html"), "updates src attribute")
+    this.assert.equal(await (await this.querySelector("h1")).getVisibleText(), "Eager-loaded frame")
+    this.assert.equal(
+      await (await this.querySelector("#eager-loaded-frame h2")).getVisibleText(),
+      "Eager-loaded frame: Loaded"
+    )
+    this.assert.equal(await this.pathname, "/src/tests/fixtures/page_with_eager_frame.html")
+  }
+
   async withoutChangingEventListenersCount(callback: () => void) {
     const name = "eventListenersAttachedToDocument"
     const setup = () => {