Add MutationObserver to Turbo Drive Preloader#911
Add MutationObserver to Turbo Drive Preloader#911julianrubisch wants to merge 2 commits intohotwired:mainfrom
Conversation
022787b to
8ae96cf
Compare
|
heads up: I've rewritten this in pure JS because I really think it'd be beneficial 🙌 |
8ae96cf to
06d66fc
Compare
|
Added a test ✌ |
| this.preloadLinkObserver = new MutationObserver((mutationList, observer) => { | ||
| mutationList.forEach((mutation) => { | ||
| if (mutation.attributeName !== "data-turbo-preload" || mutation.target.tagName !== "A") return | ||
|
|
||
| this.preloadURL(mutation.target) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
While I appreciate that this PR's code matches the style of the surrounding code, I wonder if the introduction of the MutationObserver poses an opportunity to re-structure how the Preloader monitors the page.
There's some precedent for code tied to the document's structure, nodes, and attributes to live in the src/observers directory as a class with a .start() and .stop() method and a context-specific delegate interface (when the project was built with TypeScript, enforcing that delegate interface was fairly automatic).
In this case, calling .start() could attach the MutationObserver (maybe even with attributeFilter: "data-turbo-preload"), and calling .stop() could detach it.
I'm imagining a PreloadObserver resembling something like:
class PreloadObserver {
started = false
attributeName = "data-turbo-preload"
constructor(delegate, element) {
this.delegate = delegate
this.element = element
this.mutationObserver = new MutationObserver(this.#notifyDelegateOfMutations)
}
start() {
if (!this.started) {
this.mutationObserver.observe(this.element, { attributeFilter: [this.attributeName], subtree: true })
for (const element of this.element.querySelectorAll(`[${this.attributeName}]`)) {
this.#notifyDelegate(element)
}
this.started = true
}
}
stop() {
if (this.started) {
this.mutationObserver.disconnect()
this.started = false
}
}
#notifyDelegateOfMutations = (mutationList) => {
for (const { target } of mutationList) {
this.#notifyDelegate(target)
}
}
#notifyDelegate = (element) => {
if (element instanceof HTMLAnchorElement) {
// this is a bad method name, but communicates the point
this.delegate.canPreloadAnchor(element)
}
}
}Then, the Session instance could build an observer (and manage its .start() and .stop() calls with the rest of its observers:
export class Session {
// ...
preloadObserver = new PreloadObserver(this, document.body)
start() {
// ...
this.preloadObserver.start()
// ...
}
stop() {
// ...
this.preloadObserver.stop()
// ...
}
// Preload observer delegate
canPreloadAnchor(anchor) {
this.preloader.preloadURL(anchor)
}
}There was a problem hiding this comment.
I very much like this suggestion, but please forgive me if I don't lift a finger until I hear back from one of the maintainers.
|
I think this is a great patch! I'm curious if you're interested in exploring #911 (comment) while we wait for a maintainer to grant the branch permission to execute CI. |
|
This is great! Having a mutation observer in place to handle preloading would allow dynamically controlling preloading of links on hover, eg. similar to "hoverintent". We use a similar pattern today but misuse the private |
|
While I think the |
|
And another one gone... |
Alternative to #910
@domchristie and @marcoroth both pointed out in private conversations that exposing the preloader as public API could lead to a lot of maintenance overhead
so I took a spike at rather enhancing it with a
MutationObserverthat preloads links of anchor elements (and only those) whendata-turbo-preloadis added to them dynamically.Let me know what you think!