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

replace nextAnimationFrame by nextMicroTask #1042

Merged
merged 19 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/core/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { Session } from "./session"
import { Cache } from "./cache"
import { PageRenderer } from "./drive/page_renderer"
import { PageSnapshot } from "./drive/page_snapshot"
import { FrameRenderer } from "./frames/frame_renderer"
import { FormSubmission } from "./drive/form_submission"

const session = new Session()
const { cache, navigator } = session
const cache = new Cache(session)
const { navigator } = session
michelson marked this conversation as resolved.
Show resolved Hide resolved
export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer }

export { StreamActions } from "./streams/stream_actions"
Expand Down
2 changes: 0 additions & 2 deletions src/core/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { clearBusyState, dispatch, findClosestRecursively, getVisitAction, markA
import { PageView } from "./drive/page_view"
import { FrameElement } from "../elements/frame_element"
import { Preloader } from "./drive/preloader"
import { Cache } from "./cache"

export class Session {
navigator = new Navigator(this)
Expand All @@ -34,7 +33,6 @@ export class Session {
formLinkClickObserver = new FormLinkClickObserver(this, document.documentElement)
frameRedirector = new FrameRedirector(this, document.documentElement)
streamMessageRenderer = new StreamMessageRenderer()
cache = new Cache()

drive = true
enabled = true
Expand Down
4 changes: 2 additions & 2 deletions src/elements/stream_element.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { StreamActions } from "../core/streams/stream_actions"
import { nextAnimationFrame } from "../util"
import { nextRepaint } from "../util"

// <turbo-stream action=replace target=id><template>...

Expand Down Expand Up @@ -43,7 +43,7 @@ export class StreamElement extends HTMLElement {
const event = this.beforeRenderEvent

if (this.dispatchEvent(event)) {
await nextAnimationFrame()
nextRepaint()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to await this? Should nextRepaint() return the next-prefixed Promise utilities instead of await-ing them?

await event.detail.render(this)
}
})())
Expand Down
28 changes: 14 additions & 14 deletions src/tests/unit/stream_element_tests.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { StreamElement } from "../../elements"
import { nextAnimationFrame } from "../../util"
import { nextRepaint } from "../../util"
import { DOMTestCase } from "../helpers/dom_test_case"
import { assert } from "@open-wc/testing"

Expand Down Expand Up @@ -34,13 +34,13 @@ test("action=append", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()
michelson marked this conversation as resolved.
Show resolved Hide resolved

assert.equal(subject.find("#hello")?.textContent, "Hello Turbo Streams")
assert.isNull(element.parentElement)

subject.append(element2)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "Hello Turbo Streams and more")
assert.isNull(element2.parentElement)
Expand All @@ -56,13 +56,13 @@ test("action=append with children ID already present in target", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "Hello Turbo First tail1 ")
assert.isNull(element.parentElement)

subject.append(element2)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "Hello Turbo tail1 New First Second tail2 ")
})
Expand All @@ -73,13 +73,13 @@ test("action=prepend", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "Streams Hello Turbo")
assert.isNull(element.parentElement)

subject.append(element2)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "and more Streams Hello Turbo")
assert.isNull(element.parentElement)
Expand All @@ -95,13 +95,13 @@ test("action=prepend with children ID already present in target", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "First tail1 Hello Turbo")
assert.isNull(element.parentElement)

subject.append(element2)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "New First Second tail2 tail1 Hello Turbo")
})
Expand All @@ -111,7 +111,7 @@ test("action=remove", async () => {
assert.ok(subject.find("#hello"))

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.notOk(subject.find("#hello"))
assert.isNull(element.parentElement)
Expand All @@ -123,7 +123,7 @@ test("action=replace", async () => {
assert.ok(subject.find("div#hello"))

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")
assert.notOk(subject.find("div#hello"))
Expand All @@ -136,7 +136,7 @@ test("action=update", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.textContent, "Goodbye Turbo")
assert.isNull(element.parentElement)
Expand All @@ -147,7 +147,7 @@ test("action=after", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.nextSibling?.textContent, "After Turbo")
assert.ok(subject.find("div#hello"))
Expand All @@ -160,7 +160,7 @@ test("action=before", async () => {
assert.equal(subject.find("#hello")?.textContent, "Hello Turbo")

subject.append(element)
await nextAnimationFrame()
await nextRepaint()

assert.equal(subject.find("#hello")?.previousSibling?.textContent, "Before Turbo")
assert.ok(subject.find("div#hello"))
Expand Down
8 changes: 8 additions & 0 deletions src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ export function dispatch(eventName, { target, cancelable, detail } = {}) {
return event
}

export async function nextRepaint() {
if (document.visibilityState === "hidden") {
await nextEventLoopTick()
} else {
await nextAnimationFrame()
}
}
michelson marked this conversation as resolved.
Show resolved Hide resolved

export function nextAnimationFrame() {
return new Promise((resolve) => requestAnimationFrame(() => resolve()))
}
Expand Down