diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js index de9eb91ea..d9c78e5ac 100644 --- a/src/core/drive/page_renderer.js +++ b/src/core/drive/page_renderer.js @@ -1,5 +1,6 @@ -import { Renderer } from "../renderer" import { activateScriptElement, waitForLoad } from "../../util" +import { Renderer } from "../renderer" +import { ProgressBarID } from "./progress_bar" export class PageRenderer extends Renderer { static renderElement(currentElement, newElement) { @@ -73,8 +74,13 @@ export class PageRenderer extends Renderer { const mergedHeadElements = this.mergeProvisionalElements() const newStylesheetElements = this.copyNewHeadStylesheetElements() this.copyNewHeadScriptElements() + await mergedHeadElements await newStylesheetElements + + if (this.willRender) { + this.removeUnusedHeadStylesheetElements() + } } async replaceBody() { @@ -106,6 +112,12 @@ export class PageRenderer extends Renderer { } } + removeUnusedHeadStylesheetElements() { + for (const element of this.unusedHeadStylesheetElements) { + document.head.removeChild(element) + } + } + async mergeProvisionalElements() { const newHeadElements = [...this.newHeadProvisionalElements] @@ -171,6 +183,14 @@ export class PageRenderer extends Renderer { await this.renderElement(this.currentElement, this.newElement) } + get unusedHeadStylesheetElements() { + return this.oldHeadStylesheetElements.filter((element) => element.id !== ProgressBarID) + } + + get oldHeadStylesheetElements() { + return this.currentHeadSnapshot.getStylesheetElementsNotInSnapshot(this.newHeadSnapshot) + } + get newHeadStylesheetElements() { return this.newHeadSnapshot.getStylesheetElementsNotInSnapshot(this.currentHeadSnapshot) } diff --git a/src/core/drive/progress_bar.js b/src/core/drive/progress_bar.js index caff5f5da..bd0f6958b 100644 --- a/src/core/drive/progress_bar.js +++ b/src/core/drive/progress_bar.js @@ -1,5 +1,7 @@ import { unindent, getMetaContent } from "../../util" +export const ProgressBarID = "turbo-progress-bar" + export class ProgressBar { static animationDuration = 300 /*ms*/ @@ -104,6 +106,7 @@ export class ProgressBar { createStylesheetElement() { const element = document.createElement("style") + element.id = ProgressBarID element.type = "text/css" element.textContent = ProgressBar.defaultCSS if (this.cspNonce) { diff --git a/src/tests/fixtures/additional_script.html b/src/tests/fixtures/additional_script.html new file mode 100644 index 000000000..0f67d936a --- /dev/null +++ b/src/tests/fixtures/additional_script.html @@ -0,0 +1,14 @@ + + + + + Additional assets + + + + + + +

Additional assets

+ + diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html index ffaaa693a..f03e725a8 100644 --- a/src/tests/fixtures/rendering.html +++ b/src/tests/fixtures/rendering.html @@ -36,6 +36,7 @@

Rendering

Tracked nonce tag

Additional assets

+

Additional script

Head script

Body script

data-turbo-eval=false script

diff --git a/src/tests/fixtures/stylesheets/common.css b/src/tests/fixtures/stylesheets/common.css new file mode 100644 index 000000000..1f4632a21 --- /dev/null +++ b/src/tests/fixtures/stylesheets/common.css @@ -0,0 +1,5 @@ +body { + background-color: rgb(0, 0, 128); + color: rgb(0, 128, 0); + margin: 0; +} diff --git a/src/tests/fixtures/stylesheets/left.css b/src/tests/fixtures/stylesheets/left.css new file mode 100644 index 000000000..30f31ca46 --- /dev/null +++ b/src/tests/fixtures/stylesheets/left.css @@ -0,0 +1,4 @@ +body { + background-color: rgb(128, 0, 0); + color: rgb(128, 0, 0); +} diff --git a/src/tests/fixtures/stylesheets/left.html b/src/tests/fixtures/stylesheets/left.html new file mode 100644 index 000000000..7da4b67f9 --- /dev/null +++ b/src/tests/fixtures/stylesheets/left.html @@ -0,0 +1,20 @@ + + + + + Left + + + + + + + + +

Left

+

go right

+ + diff --git a/src/tests/fixtures/stylesheets/right.css b/src/tests/fixtures/stylesheets/right.css new file mode 100644 index 000000000..38bdfb182 --- /dev/null +++ b/src/tests/fixtures/stylesheets/right.css @@ -0,0 +1,3 @@ +body { + background-color: rgb(0, 128, 0); +} diff --git a/src/tests/fixtures/stylesheets/right.html b/src/tests/fixtures/stylesheets/right.html new file mode 100644 index 000000000..13001bbf5 --- /dev/null +++ b/src/tests/fixtures/stylesheets/right.html @@ -0,0 +1,20 @@ + + + + + Right + + + + + + + + +

Right

+

go left

+ + diff --git a/src/tests/functional/drive_stylesheet_merging_tests.js b/src/tests/functional/drive_stylesheet_merging_tests.js new file mode 100644 index 000000000..c51e7421e --- /dev/null +++ b/src/tests/functional/drive_stylesheet_merging_tests.js @@ -0,0 +1,24 @@ +import { test } from "@playwright/test" +import { assert } from "chai" +import { cssClassIsDefined, getComputedStyle, hasSelector, nextBody } from "../helpers/page" + +test.beforeEach(async ({ page }) => { + await page.goto("/src/tests/fixtures/stylesheets/left.html") +}) + +test("navigating removes unused style elements", async ({ page }) => { + await page.locator("#go-right").click() + await nextBody(page) + + 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 cssClassIsDefined(page, "right")) + assert.notOk(await cssClassIsDefined(page, "left")) + assert.equal(await getComputedStyle(page, "body", "marginLeft"), "0px") + assert.equal(await getComputedStyle(page, "body", "marginRight"), "20px") +}) + diff --git a/src/tests/functional/rendering_tests.js b/src/tests/functional/rendering_tests.js index d56f75e9a..9df7ffd04 100644 --- a/src/tests/functional/rendering_tests.js +++ b/src/tests/functional/rendering_tests.js @@ -222,11 +222,11 @@ test("changes the html[lang] attribute", async ({ page }) => { assert.equal(await page.getAttribute("html", "lang"), "es") }) -test("accumulates asset elements in head", async ({ page }) => { - const assetElements = () => page.$$('script, style, link[rel="stylesheet"]') +test("accumulates script elements in head", async ({ page }) => { + const assetElements = () => page.$$('script') const originalElements = await assetElements() - await page.click("#additional-assets-link") + await page.click("#additional-script-link") await nextBody(page) const newElements = await assetElements() assert.notOk(await deepElementsEqual(page, newElements, originalElements)) diff --git a/src/tests/helpers/page.js b/src/tests/helpers/page.js index 886941e6a..760b6631f 100644 --- a/src/tests/helpers/page.js +++ b/src/tests/helpers/page.js @@ -23,6 +23,28 @@ export function disposeAll(...handles) { return Promise.all(handles.map((handle) => handle.dispose())) } +export function getComputedStyle(page, selector, propertyName) { + return page.evaluate( + ([selector, propertyName]) => { + const element = document.querySelector(selector) + return getComputedStyle(element)[propertyName] + }, + [selector, propertyName] + ) +} + +export function cssClassIsDefined(page, className) { + return page.evaluate((className) => { + for (const stylesheet of document.styleSheets) { + for (const rule of stylesheet.cssRules) { + if (rule instanceof CSSStyleRule && rule.selectorText == `.${className}`) { + return true + } + } + } + }, className) +} + export function getFromLocalStorage(page, key) { return page.evaluate((storageKey) => localStorage.getItem(storageKey), key) }