Introduce data-turbo-track="dynamic" (hotwired#1140)

To track stylesheets and styles that we can dynamically remove when navigating. We introduced this behaviour in hotwired#1128 and thought it would be a good default to avoid full page reloads when styles change. However, it seems it's quite common for libraries to add stylesheets and styles to the head that they want to keep around. For example, see - hotwired#1133 - hotwired#1139 So let's make this behaviour opt-in by adding a `data-turbo-track="dynamic"` attribute to stylesheets and styles that we want to dynamically remove when they are no longer in a new page navigation. This avoids any breaking changes for existing apps.
domchristie · Jul 15, 2024 · bd79f35 · bd79f35
1 parent 6600b45
commit bd79f35
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 20 deletions.
diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js
@@ -78,7 +78,7 @@ export class PageRenderer extends Renderer {
     await newStylesheetElements
 
     if (this.willRender) {
-      this.removeUnusedHeadStylesheetElements()
+      this.removeUnusedDynamicStylesheetElements()
     }
   }
 
@@ -111,8 +111,8 @@ export class PageRenderer extends Renderer {
     }
   }
 
-  removeUnusedHeadStylesheetElements() {
-    for (const element of this.unusedHeadStylesheetElements) {
+  removeUnusedDynamicStylesheetElements() {
+    for (const element of this.unusedDynamicStylesheetElements) {
       document.head.removeChild(element)
     }
   }
@@ -182,13 +182,9 @@ export class PageRenderer extends Renderer {
     await this.renderElement(this.currentElement, this.newElement)
   }
 
-  get unusedHeadStylesheetElements() {
+  get unusedDynamicStylesheetElements() {
     return this.oldHeadStylesheetElements.filter((element) => {
-      return !(element.hasAttribute("data-turbo-permanent") ||
-        // Trix dynamically adds styles to the head that we want to keep around which have a
-        // `data-tag-name` attribute. Long term we should moves those styles to Trix's CSS file
-        // but for now we'll just skip removing them
-        element.hasAttribute("data-tag-name"))
+      return element.getAttribute("data-turbo-track") === "dynamic"
     })
   }
 

diff --git a/src/core/drive/progress_bar.js b/src/core/drive/progress_bar.js
@@ -106,8 +106,6 @@ export class ProgressBar {
 
   createStylesheetElement() {
     const element = document.createElement("style")
-    element.id = ProgressBarID
-    element.setAttribute("data-turbo-permanent", "")
     element.type = "text/css"
     element.textContent = ProgressBar.defaultCSS
     if (this.cspNonce) {

diff --git a/src/tests/fixtures/stylesheets/left.html b/src/tests/fixtures/stylesheets/left.html
@@ -4,13 +4,18 @@
     <meta charset="utf-8" />
     <title>Left</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
-    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/common.css">
-    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/left.css">
+    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/common.css" data-turbo-track="dynamic">
+    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/left.css" data-turbo-track="dynamic">
 
-    <style>
+    <style data-turbo-track="dynamic">
       body { margin-left: 20px; }
       .left { margin-left: 20px; }
     </style>
+
+    <script type="text/javascript">
+      document.head.insertAdjacentHTML("beforeend", `<style id="added-style">body{background:red}</style>`)
+      document.head.insertAdjacentHTML("beforeend", `<link id="added-link" rel="stylesheet" type="text/css" href="/src/tests/fixtures/test.css">`)
+    </script>
   </head>
 
   <body></body>

diff --git a/src/tests/fixtures/stylesheets/right.html b/src/tests/fixtures/stylesheets/right.html
@@ -4,10 +4,10 @@
     <meta charset="utf-8" />
     <title>Right</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
-    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/common.css">
-    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/right.css">
+    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/common.css" data-turbo-track="dynamic">
+    <link rel="stylesheet" type="text/css" href="/src/tests/fixtures/stylesheets/right.css" data-turbo-track="dynamic">
 
-    <style>
+    <style data-turbo-track="dynamic">
       body { margin-right: 20px; }
       .right { margin-right: 20px; }
     </style>

diff --git a/src/tests/functional/drive_stylesheet_merging_tests.js b/src/tests/functional/drive_stylesheet_merging_tests.js
@@ -6,19 +6,22 @@ test.beforeEach(async ({ page }) => {
   await page.goto("/src/tests/fixtures/stylesheets/left.html")
 })
 
-test("navigating removes unused style elements", async ({ page }) => {
-  assert.ok(await hasSelector(page, 'style[id="turbo-progress-bar"]'))
+test("navigating removes unused dynamically tracked style elements", async ({ page }) => {
+  assert.ok(await hasSelector(page, 'style[id="added-style"]'))
+  assert.ok(await hasSelector(page, 'link[id="added-link"]'))
 
   await page.locator("#go-right").click()
   await nextBody(page)
 
-  assert.ok(await hasSelector(page, 'style[id="turbo-progress-bar"]'))
   assert.ok(await hasSelector(page, 'link[rel=stylesheet][href="/src/tests/fixtures/stylesheets/common.css"]'))
   assert.ok(await hasSelector(page, 'link[rel=stylesheet][href="/src/tests/fixtures/stylesheets/right.css"]'))
   assert.notOk(await hasSelector(page, 'link[rel=stylesheet][href="/src/tests/fixtures/stylesheets/left.css"]'))
   assert.equal(await getComputedStyle(page, "body", "backgroundColor"), "rgb(0, 128, 0)")
   assert.equal(await getComputedStyle(page, "body", "color"), "rgb(0, 128, 0)")
 
+  assert.ok(await hasSelector(page, 'style[id="added-style"]'))
+  assert.ok(await hasSelector(page, 'link[id="added-link"]'))
+
   assert.ok(await cssClassIsDefined(page, "right"))
   assert.notOk(await cssClassIsDefined(page, "left"))
   assert.equal(await getComputedStyle(page, "body", "marginLeft"), "0px")