Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for CSS to be loaded before replacing the Body #614

Merged
merged 9 commits into from
Jul 29, 2022
18 changes: 13 additions & 5 deletions src/core/drive/page_renderer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Renderer } from "../renderer"
import { PageSnapshot } from "./page_snapshot"
import { ReloadReason } from "../native/browser_adapter"
import { waitForLoad } from "../../util"

export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
static renderElement(currentElement: HTMLBodyElement, newElement: HTMLBodyElement) {
Expand Down Expand Up @@ -29,8 +30,8 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
}
}

prepareToRender() {
this.mergeHead()
async prepareToRender() {
await this.mergeHead()
}

async render() {
Expand Down Expand Up @@ -58,11 +59,12 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
return this.newSnapshot.element
}

mergeHead() {
this.copyNewHeadStylesheetElements()
async mergeHead() {
const newStylesheetElements = this.copyNewHeadStylesheetElements()
this.copyNewHeadScriptElements()
this.removeCurrentHeadProvisionalElements()
this.copyNewHeadProvisionalElements()
await newStylesheetElements
}

replaceBody() {
Expand All @@ -76,10 +78,16 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
return this.currentHeadSnapshot.trackedElementSignature == this.newHeadSnapshot.trackedElementSignature
}

copyNewHeadStylesheetElements() {
async copyNewHeadStylesheetElements() {
const loadingElements = []

for (const element of this.newHeadStylesheetElements) {
loadingElements.push(waitForLoad(element as HTMLLinkElement))

document.head.appendChild(element)
}

await Promise.all(loadingElements)
}

copyNewHeadScriptElements() {
Expand Down
6 changes: 3 additions & 3 deletions src/core/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export abstract class View<
try {
this.renderPromise = new Promise((resolve) => (this.resolveRenderPromise = resolve))
this.renderer = renderer
this.prepareToRenderSnapshot(renderer)
await this.prepareToRenderSnapshot(renderer)

const renderInterception = new Promise((resolve) => (this.resolveInterceptionPromise = resolve))
const options = { resume: this.resolveInterceptionPromise, render: this.renderer.renderElement }
Expand All @@ -112,9 +112,9 @@ export abstract class View<
this.delegate.viewInvalidated(reason)
}

prepareToRenderSnapshot(renderer: R) {
async prepareToRenderSnapshot(renderer: R) {
this.markAsPreview(renderer.isPreview)
renderer.prepareToRender()
await renderer.prepareToRender()
}

markAsPreview(isPreview: boolean) {
Expand Down
68 changes: 68 additions & 0 deletions src/tests/functional/rendering_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,74 @@ test("test does not evaluate head stylesheet elements inside noscript elements",
assert.equal(await isNoscriptStylesheetEvaluated(page), false)
})

test("test waits for CSS to be loaded before rendering", async ({ page }) => {
let finishLoadingCSS = (_value?: unknown) => {}
const promise = new Promise((resolve) => {
finishLoadingCSS = resolve
})
page.route("**/*.css", async (route) => {
await promise
route.continue()
})

await page.click("#additional-assets-link")

assert.equal(await isStylesheetEvaluated(page), false)
assert.notEqual(await page.textContent("h1"), "Additional assets")

finishLoadingCSS()

await nextEventNamed(page, "turbo:render")

assert.equal(await page.textContent("h1"), "Additional assets")
assert.equal(await isStylesheetEvaluated(page), true)
})

test("test waits for CSS to fail before rendering", async ({ page }) => {
let finishLoadingCSS = (_value?: unknown) => {}
const promise = new Promise((resolve) => {
finishLoadingCSS = resolve
})
page.route("**/*.css", async (route) => {
await promise
route.abort()
})

await page.click("#additional-assets-link")

assert.equal(await isStylesheetEvaluated(page), false)
assert.notEqual(await page.textContent("h1"), "Additional assets")

finishLoadingCSS()

await nextEventNamed(page, "turbo:render")

assert.equal(await page.textContent("h1"), "Additional assets")
assert.equal(await isStylesheetEvaluated(page), false)
})

test("test waits for some time, but renders if CSS takes too much to load", async ({ page }) => {
let finishLoadingCSS = (_value?: unknown) => {}
const promise = new Promise((resolve) => {
finishLoadingCSS = resolve
})
page.route("**/*.css", async (route) => {
await promise
route.continue()
})

await page.click("#additional-assets-link")
await nextEventNamed(page, "turbo:render")

assert.equal(await page.textContent("h1"), "Additional assets")
assert.equal(await isStylesheetEvaluated(page), false)

finishLoadingCSS()
await nextBeat()

assert.equal(await isStylesheetEvaluated(page), true)
})

test("skip evaluates head script elements once", async ({ page }) => {
assert.equal(await headScriptEvaluationCount(page), undefined)

Expand Down
14 changes: 14 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ export function clearBusyState(...elements: Element[]) {
}
}

export function waitForLoad(element: HTMLLinkElement, timeoutInMilliseconds = 2000): Promise<void> {
return new Promise((resolve) => {
const onComplete = () => {
element.removeEventListener("error", onComplete)
element.removeEventListener("load", onComplete)
resolve()
}

manuelpuyol marked this conversation as resolved.
Show resolved Hide resolved
element.addEventListener("load", onComplete, { once: true })
element.addEventListener("error", onComplete, { once: true })
setTimeout(resolve, timeoutInMilliseconds)
})
}

export function getHistoryMethodForAction(action: Action) {
switch (action) {
case "replace":
Expand Down