Skip to content

Commit

Permalink
Add InstantClick behavior (#1101)
Browse files Browse the repository at this point in the history
* Move doesNotTargetIFrame to util.js

* Move findLinkFromClickTarget to util.js

* Move getLocationForLink to util.js

* Allow request to be intercepted and overriden on turbo:before-fetch-request

* Add instantclick behavior

* Allow customizing the event that triggers prefetching

* Allow customizing the cache time for prefetching

* Rename LinkPrefetchOnMouseoverObserver to LinkPrefetchObserver

Because it is not only triggered on mouseover, but could also be on mousedown, or eventually touchstart.

* Use private methods in LinkPrefetchObserver

* Reorganize methods on LinkPrefetchObserver

* Require a shorter sleep time in the test

Since turbo-prefetch-cache-time is set to 1 millisecond in the html fixture

* Standardize anchor IDs in link_prefetch_observer_tests

anchor_ prefix is used for all anchors in the tests

* Don't try traverse DOM to determine if the target is a link

This is not necessary, since we can just check if the target is an
anchor element with an href attribute.

We were just using findLinkFromClickTarget because it had the selector
we needed, but we can just use the selector directly.

* Keep the closing tag on the same line as the rest of the tag

* Remove unnecessary nesting in tests

* Add missing newline at end of file

* Check for prefetch meta tag before prefetching (on hover event)

* Use FetchRequest to build request for LinkPrefetchObserver

* LinkPrefetchObserver implements the FetchRequest interface, so it can
be used to build a request.
* It also adds this.response to FetchRequest to store the non-awaited
`fetch` response, because we need to FetchRequest#receive() a `fetch`
response, not a FetchRequest.

* Add Turbo Stream header to Accept header when link has data-turbo-stream

* Bring back prefetching links with inner elements

* Add cancelable delay to prefetching links on hover

* Fix clearing cache on every prefetch after b9e82f2

* Add tests for the delay on the meta tag

* Use mouseenter and mouseleave instead of mouseover and mouseout

To avoid having to traverse the DOM to find the link element

* Remove unneeded comment

* Use double quotes instead of single quotes for consistency

* Move link variable declaration inside if statement

Since target is only a link if isLink is true

* Use correct key name for mouseenter event on LinkPrefetchObserver.triggerEvents

On 5078e0b we started using the `mouseenter` event instead of the `mouseover` event to trigger prefetching. However, we forgot to update the key name on the `LinkPrefetchObserver.triggerEvents` object.

* Allow prefetching when visiting page without meta, then visiting one with it

* Allow create and delete posts with comments on the test server

* Clear prefetch cache after form submission

* Add test for nested data-turbo-prefetch=true within data-turbo-prefetch=false

* No longer allow customizing the prefetch trigger event

* No longer allow customizing the prefetch delay

* Add touchstart event to prefetch observer

* Fix flaky tests

This commit fixes the flaky tests by ensuring that each worker has its own database file.

This is done by adding a `worker_id` query parameter to the URLs of the pages that are being tested. This `worker_id` is passed to the database functions, which then use it to determine the name of the database file.

It's necessary because the tests are running in parallel, and the database file is shared between all the workers. This means that if one worker creates a post, the other workers will see that post, and the tests will fail.

* Use double quotes instead of single quotes

* Only cache the link you're currently hovering

Instead of maintaining a cache of all the links that have been hovered
in the last 10 seconds.

This solves issues where the user hovers a link, then performs a non-safe
action and then later clicks the link. In this case, we would be showing
stale content from before the action was performed.

* Remove unused files after ETA template rendered removal

* Remove unused variable

* Clear prefetch cache when the link is no longer hovered

This avoids a flurry of requests when casually scrolling down a page

* Style changes

---------

Co-authored-by: Alberto Fernández-Capel <afcapel@gmail.com>
  • Loading branch information
davidalejandroaguilar and afcapel committed Jan 22, 2024
1 parent bb735bf commit 623a9df
Show file tree
Hide file tree
Showing 16 changed files with 702 additions and 29 deletions.
6 changes: 4 additions & 2 deletions playwright.config.js
Expand Up @@ -8,7 +8,8 @@ const config = {
...devices["Desktop Chrome"],
contextOptions: {
timeout: 60000
}
},
hasTouch: true
}
},
{
Expand All @@ -17,7 +18,8 @@ const config = {
...devices["Desktop Firefox"],
contextOptions: {
timeout: 60000
}
},
hasTouch: true
}
}
],
Expand Down
10 changes: 9 additions & 1 deletion src/core/drive/form_submission.js
Expand Up @@ -2,6 +2,7 @@ import { FetchRequest, FetchMethod, fetchMethodFromString, fetchEnctypeFromStrin
import { expandURL } from "../url"
import { clearBusyState, dispatch, getAttribute, getMetaContent, hasAttribute, markAsBusy } from "../../util"
import { StreamMessage } from "../streams/stream_message"
import { prefetchCache } from "./prefetch_cache"

export const FormSubmissionState = {
initialized: "initialized",
Expand Down Expand Up @@ -126,13 +127,20 @@ export class FormSubmission {
}

requestPreventedHandlingResponse(request, response) {
prefetchCache.clear()

this.result = { success: response.succeeded, fetchResponse: response }
}

requestSucceededWithResponse(request, response) {
if (response.clientError || response.serverError) {
this.delegate.formSubmissionFailedWithResponse(this, response)
} else if (this.requestMustRedirect(request) && responseSucceededWithoutRedirect(response)) {
return
}

prefetchCache.clear()

if (this.requestMustRedirect(request) && responseSucceededWithoutRedirect(response)) {
const error = new Error("Form responses must redirect to another location")
this.delegate.formSubmissionErrored(this, error)
} else {
Expand Down
34 changes: 34 additions & 0 deletions src/core/drive/prefetch_cache.js
@@ -0,0 +1,34 @@
const PREFETCH_DELAY = 100

class PrefetchCache {
#prefetchTimeout = null
#prefetched = null

get(url) {
if (this.#prefetched && this.#prefetched.url === url && this.#prefetched.expire > Date.now()) {
return this.#prefetched.request
}
}

setLater(url, request, ttl) {
this.clear()

this.#prefetchTimeout = setTimeout(() => {
request.perform()
this.set(url, request, ttl)
this.#prefetchTimeout = null
}, PREFETCH_DELAY)
}

set(url, request, ttl) {
this.#prefetched = { url, request, expire: new Date(new Date().getTime() + ttl) }
}

clear() {
if (this.#prefetchTimeout) clearTimeout(this.#prefetchTimeout)
this.#prefetched = null
}
}

export const cacheTtl = 10 * 1000
export const prefetchCache = new PrefetchCache()
13 changes: 13 additions & 0 deletions src/core/session.js
Expand Up @@ -3,6 +3,7 @@ import { CacheObserver } from "../observers/cache_observer"
import { FormSubmitObserver } from "../observers/form_submit_observer"
import { FrameRedirector } from "./frames/frame_redirector"
import { History } from "./drive/history"
import { LinkPrefetchObserver } from "../observers/link_prefetch_observer"
import { LinkClickObserver } from "../observers/link_click_observer"
import { FormLinkClickObserver } from "../observers/form_link_click_observer"
import { getAction, expandURL, locationIsVisitable } from "./url"
Expand All @@ -26,6 +27,7 @@ export class Session {

pageObserver = new PageObserver(this)
cacheObserver = new CacheObserver()
linkPrefetchObserver = new LinkPrefetchObserver(this, document)
linkClickObserver = new LinkClickObserver(this, window)
formSubmitObserver = new FormSubmitObserver(this, document)
scrollObserver = new ScrollObserver(this)
Expand Down Expand Up @@ -53,6 +55,7 @@ export class Session {
if (!this.started) {
this.pageObserver.start()
this.cacheObserver.start()
this.linkPrefetchObserver.start()
this.formLinkClickObserver.start()
this.linkClickObserver.start()
this.formSubmitObserver.start()
Expand All @@ -74,6 +77,7 @@ export class Session {
if (this.started) {
this.pageObserver.stop()
this.cacheObserver.stop()
this.linkPrefetchObserver.stop()
this.formLinkClickObserver.stop()
this.linkClickObserver.stop()
this.formSubmitObserver.stop()
Expand Down Expand Up @@ -199,6 +203,15 @@ export class Session {

submittedFormLinkToLocation() {}

// Link hover observer delegate

canPrefetchRequestToLocation(link, location) {
return (
this.elementIsNavigatable(link) &&
locationIsVisitable(location, this.snapshot.rootLocation)
)
}

// Link click observer delegate

willFollowLinkToLocation(link, location, event) {
Expand Down
13 changes: 11 additions & 2 deletions src/http/fetch_request.js
Expand Up @@ -121,10 +121,17 @@ export class FetchRequest {
async perform() {
const { fetchOptions } = this
this.delegate.prepareRequest(this)
await this.#allowRequestToBeIntercepted(fetchOptions)
const event = await this.#allowRequestToBeIntercepted(fetchOptions)
try {
this.delegate.requestStarted(this)
const response = await fetch(this.url.href, fetchOptions)

if (event.detail.fetchRequest) {
this.response = event.detail.fetchRequest.response
} else {
this.response = fetch(this.url.href, fetchOptions)
}

const response = await this.response
return await this.receive(response)
} catch (error) {
if (error.name !== "AbortError") {
Expand Down Expand Up @@ -186,6 +193,8 @@ export class FetchRequest {
})
this.url = event.detail.url
if (event.defaultPrevented) await requestInterception

return event
}

#willDelegateErrorHandling(error) {
Expand Down
10 changes: 10 additions & 0 deletions src/observers/form_link_click_observer.js
Expand Up @@ -15,6 +15,16 @@ export class FormLinkClickObserver {
this.linkInterceptor.stop()
}

// Link hover observer delegate

canPrefetchRequestToLocation(link, location) {
return false
}

prefetchAndCacheRequestToLocation(link, location) {
return
}

// Link click observer delegate

willFollowLinkToLocation(link, location, originalEvent) {
Expand Down
27 changes: 3 additions & 24 deletions src/observers/link_click_observer.js
@@ -1,5 +1,4 @@
import { expandURL } from "../core/url"
import { findClosestRecursively } from "../util"
import { doesNotTargetIFrame, findLinkFromClickTarget, getLocationForLink } from "../util"

export class LinkClickObserver {
started = false
Expand Down Expand Up @@ -31,9 +30,9 @@ export class LinkClickObserver {
clickBubbled = (event) => {
if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) {
const target = (event.composedPath && event.composedPath()[0]) || event.target
const link = this.findLinkFromClickTarget(target)
const link = findLinkFromClickTarget(target)
if (link && doesNotTargetIFrame(link)) {
const location = this.getLocationForLink(link)
const location = getLocationForLink(link)
if (this.delegate.willFollowLinkToLocation(link, location, event)) {
event.preventDefault()
this.delegate.followedLinkToLocation(link, location)
Expand All @@ -53,24 +52,4 @@ export class LinkClickObserver {
event.shiftKey
)
}

findLinkFromClickTarget(target) {
return findClosestRecursively(target, "a[href]:not([target^=_]):not([download])")
}

getLocationForLink(link) {
return expandURL(link.getAttribute("href") || "")
}
}

function doesNotTargetIFrame(anchor) {
if (anchor.hasAttribute("target")) {
for (const element of document.getElementsByName(anchor.target)) {
if (element instanceof HTMLIFrameElement) return false
}

return true
} else {
return true
}
}

0 comments on commit 623a9df

Please sign in to comment.