Skip to content

Commit

Permalink
Let developers access the initiator element in visit/fetch events (#2)
Browse files Browse the repository at this point in the history
* Prevent errors when communicating with iOS webview.
To successfully communicate with the iOS view, messages need to be of a type supported by the structuredClone algorithm.
The new TransferableVisitOptions type ensures that VisitOptions passed to the adapter conform to this type. The StructuredCloneValue type has been created based on https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm#supported_types

* Help clarify VisitOptions values unsupported by postMessage

* Let developers access the initiator element in visit/fetch events

* Ensure all original VisitOptions are passed to Visit constructor

* Update frame test event targets

* Fix errors when navigating back/forward.
Ensure Navigator#currentVisitOptions is always present

* Contain VisitOption munging within Navigator

* Lint

* Reset currentVisitOptions even if visit proposal fails

* Private methods

* Raise error if visit options are not transferable to native adapters

* Clarify proposed visit options

* Lint

* Sanitize all values

* Ensure proposedVisitOptions is always set

* Dispatch visit events on form elements

* Fix merge

* Unnecessary eslint rule
  • Loading branch information
domchristie authored Sep 26, 2023
1 parent f01d95d commit c458b47
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 15 deletions.
30 changes: 28 additions & 2 deletions src/core/drive/navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ import { Visit } from "./visit"
import { PageSnapshot } from "./page_snapshot"

export class Navigator {
proposedVisitOptions = {}

constructor(delegate) {
this.delegate = delegate
}

proposeVisit(location, options = {}) {
if (this.delegate.allowsVisitingLocationWithAction(location, options.action)) {
if (this.delegate.allowsVisitingLocation(location, options)) {
if (locationIsVisitable(location, this.view.snapshot.rootLocation)) {
this.delegate.visitProposedToLocation(location, options)
this.#withTransferableVisitOptions(options, (transferableOptions) => {
this.delegate.visitProposedToLocation(location, transferableOptions)
})
} else {
window.location.href = location.toString()
}
Expand All @@ -23,6 +27,7 @@ export class Navigator {
this.stop()
this.currentVisit = new Visit(this, expandURL(locatable), restorationIdentifier, {
referrer: this.location,
...this.proposedVisitOptions,
...options
})
this.currentVisit.start()
Expand Down Expand Up @@ -80,6 +85,7 @@ export class Navigator {
const { statusCode, redirected } = fetchResponse
const action = this.getActionForFormSubmission(formSubmission)
const visitOptions = {
initiator: formSubmission.formElement,
action,
shouldCacheSnapshot,
response: { statusCode, responseHTML, redirected }
Expand Down Expand Up @@ -154,4 +160,24 @@ export class Navigator {
getActionForFormSubmission({ submitter, formElement }) {
return getVisitAction(submitter, formElement) || "advance"
}

#withTransferableVisitOptions(options, callback) {
this.proposedVisitOptions = options
try {
callback.call(this, this.#sanitizeVisitOptionsForTransfer(options))
} finally {
this.proposedVisitOptions = {}
}
}

#sanitizeVisitOptionsForTransfer(options) {
return Object.entries(options).reduce((sanitized, [key, value]) => {
try {
sanitized[key] = window.structuredClone(value)
} catch (_) {
// Ignore non-transferable values
}
return sanitized
}, {})
}
}
6 changes: 4 additions & 2 deletions src/core/drive/visit.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ export class Visit {
updateHistory,
shouldCacheSnapshot,
acceptsStreamResponse,
direction
direction,
initiator
} = {
...defaultOptions,
...options
Expand All @@ -91,6 +92,7 @@ export class Visit {
this.shouldCacheSnapshot = shouldCacheSnapshot
this.acceptsStreamResponse = acceptsStreamResponse
this.direction = direction || Direction[action]
this.initiator = initiator
}

get adapter() {
Expand Down Expand Up @@ -166,7 +168,7 @@ export class Visit {
if (this.hasPreloadedResponse()) {
this.simulateRequest()
} else if (this.shouldIssueRequest() && !this.request) {
this.request = new FetchRequest(this, FetchMethod.get, this.location)
this.request = new FetchRequest(this, FetchMethod.get, this.location, undefined, this.initiator)
this.request.perform()
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/core/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,16 @@ export class Session {
const action = this.getActionForLink(link)
const acceptsStreamResponse = link.hasAttribute("data-turbo-stream")

this.visit(location.href, { action, acceptsStreamResponse })
this.visit(location.href, { action, acceptsStreamResponse, initiator: link })
}

// Navigator delegate

allowsVisitingLocationWithAction(location, action) {
return this.locationWithActionIsSamePage(location, action) || this.applicationAllowsVisitingLocation(location)
allowsVisitingLocation(location, options) {
return (
this.locationWithActionIsSamePage(location, options.action) ||
this.applicationAllowsVisitingLocation(location, options)
)
}

visitProposedToLocation(location, options) {
Expand All @@ -189,7 +192,7 @@ export class Session {
}
extendURLWithDeprecatedProperties(visit.location)
if (!visit.silent) {
this.notifyApplicationAfterVisitingLocation(visit.location, visit.action, visit.direction)
this.notifyApplicationAfterVisitingLocation(visit.location, visit.action, visit.direction, visit.initiator)
}
}

Expand Down Expand Up @@ -294,8 +297,8 @@ export class Session {
return !event.defaultPrevented
}

applicationAllowsVisitingLocation(location) {
const event = this.notifyApplicationBeforeVisitingLocation(location)
applicationAllowsVisitingLocation(location, options) {
const event = this.notifyApplicationBeforeVisitingLocation(location, options.initiator)
return !event.defaultPrevented
}

Expand All @@ -307,15 +310,16 @@ export class Session {
})
}

notifyApplicationBeforeVisitingLocation(location) {
notifyApplicationBeforeVisitingLocation(location, target) {
return dispatch("turbo:before-visit", {
target,
detail: { url: location.href },
cancelable: true
})
}

notifyApplicationAfterVisitingLocation(location, action, direction) {
return dispatch("turbo:visit", { detail: { url: location.href, action, direction } })
notifyApplicationAfterVisitingLocation(location, action, direction, target) {
return dispatch("turbo:visit", { target, detail: { url: location.href, action, direction } })
}

notifyApplicationBeforeCachingSnapshot() {
Expand Down
4 changes: 2 additions & 2 deletions src/tests/functional/frame_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,8 @@ test("test navigating a frame targeting _top from an outer link fires events", a
await page.click("#outside-navigate-top-link")

await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:click")
await nextEventOnTarget(page, "html", "turbo:before-fetch-request")
await nextEventOnTarget(page, "html", "turbo:before-fetch-response")
await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:before-fetch-request")
await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:before-fetch-response")
await nextEventOnTarget(page, "html", "turbo:before-render")
await nextEventOnTarget(page, "html", "turbo:render")
await nextEventOnTarget(page, "html", "turbo:load")
Expand Down
25 changes: 25 additions & 0 deletions src/tests/functional/navigation_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
nextBody,
nextEventNamed,
noNextEventNamed,
nextEventOnTarget,
pathname,
pathnameForIFrame,
readEventLogs,
Expand Down Expand Up @@ -492,3 +493,27 @@ test("test ignores forms with a [target] attribute that target an iframe with [n

assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
})

test("test visit events are dispatched on links", async ({ page }) => {
await page.click("#same-origin-unannotated-link")
await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:before-visit")
await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:visit")
})

test("test visit events are dispatched on forms", async ({ page }) => {
await page.click("#same-origin-unannotated-form button")
await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:before-visit")
await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:visit")
})

test("test fetch events are dispatched on links", async ({ page }) => {
await page.click("#same-origin-unannotated-link")
await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:before-fetch-request")
await nextEventOnTarget(page, "same-origin-unannotated-link", "turbo:before-fetch-response")
})

test("test fetch events are dispatched on forms", async ({ page }) => {
await page.click("#same-origin-unannotated-form button")
await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:before-fetch-request")
await nextEventOnTarget(page, "same-origin-unannotated-form", "turbo:before-fetch-response")
})

0 comments on commit c458b47

Please sign in to comment.