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

Don't convert data-turbo-stream links to forms #647

Merged
merged 1 commit into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/core/drive/form_submission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export class FormSubmission {
}

if (this.requestAcceptsTurboStreamResponse(request)) {
headers["Accept"] = [StreamMessage.contentType, headers["Accept"]].join(", ")
request.acceptResponseType(StreamMessage.contentType)
}
}

Expand Down
14 changes: 13 additions & 1 deletion src/core/drive/visit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Adapter } from "../native/adapter"
import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request"
import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { History } from "./history"
import { getAnchor } from "../url"
Expand All @@ -8,6 +8,7 @@ import { PageSnapshot } from "./page_snapshot"
import { Action, ResolvingFunctions } from "../types"
import { getHistoryMethodForAction, uuid } from "../../util"
import { PageView } from "./page_view"
import { StreamMessage } from "../streams/stream_message"

export interface VisitDelegate {
readonly adapter: Adapter
Expand Down Expand Up @@ -49,6 +50,7 @@ export type VisitOptions = {
restorationIdentifier?: string
shouldCacheSnapshot: boolean
frame?: string
acceptsStreamResponse: boolean
}

const defaultOptions: VisitOptions = {
Expand All @@ -58,6 +60,7 @@ const defaultOptions: VisitOptions = {
willRender: true,
updateHistory: true,
shouldCacheSnapshot: true,
acceptsStreamResponse: false,
}

export type VisitResponse = {
Expand Down Expand Up @@ -96,6 +99,7 @@ export class Visit implements FetchRequestDelegate {
response?: VisitResponse
scrolled = false
shouldCacheSnapshot = true
acceptsStreamResponse = false
snapshotHTML?: string
snapshotCached = false
state = VisitState.initialized
Expand All @@ -121,6 +125,7 @@ export class Visit implements FetchRequestDelegate {
willRender,
updateHistory,
shouldCacheSnapshot,
acceptsStreamResponse,
} = {
...defaultOptions,
...options,
Expand All @@ -136,6 +141,7 @@ export class Visit implements FetchRequestDelegate {
this.updateHistory = updateHistory
this.scrolled = !willRender
this.shouldCacheSnapshot = shouldCacheSnapshot
this.acceptsStreamResponse = acceptsStreamResponse
}

get adapter() {
Expand Down Expand Up @@ -335,6 +341,12 @@ export class Visit implements FetchRequestDelegate {

// Fetch request delegate

prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
if (this.acceptsStreamResponse) {
request.acceptResponseType(StreamMessage.contentType)
}
}

requestStarted() {
this.startRequest()
}
Expand Down
18 changes: 16 additions & 2 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { session } from "../index"
import { isAction, Action } from "../types"
import { VisitOptions } from "../drive/visit"
import { TurboBeforeFrameRenderEvent } from "../session"
import { StreamMessage } from "../streams/stream_message"

export class FrameController
implements
Expand Down Expand Up @@ -59,6 +60,7 @@ export class FrameController
private frame?: FrameElement
readonly restorationIdentifier: string
private previousFrameElement?: FrameElement
private currentNavigationElement?: Element

constructor(element: FrameElement) {
this.element = element
Expand Down Expand Up @@ -217,8 +219,12 @@ export class FrameController

// Fetch request delegate

prepareHeadersForRequest(headers: FetchRequestHeaders, _request: FetchRequest) {
prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
headers["Turbo-Frame"] = this.id

if (this.currentNavigationElement?.hasAttribute("data-turbo-stream")) {
request.acceptResponseType(StreamMessage.contentType)
}
}

requestStarted(_request: FetchRequest) {
Expand Down Expand Up @@ -340,7 +346,9 @@ export class FrameController

this.proposeVisitIfNavigatedWithAction(frame, element, submitter)

frame.src = url
this.withCurrentNavigationElement(element, () => {
frame.src = url
})
}

private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
Expand Down Expand Up @@ -505,6 +513,12 @@ export class FrameController
callback()
this.ignoredAttributes.delete(attributeName)
}

private withCurrentNavigationElement(element: Element, callback: () => void) {
this.currentNavigationElement = element
callback()
delete this.currentNavigationElement
}
}

function getFrameElementById(id: string | null) {
Expand Down
4 changes: 3 additions & 1 deletion src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ export class Session

followedLinkToLocation(link: Element, location: URL) {
const action = this.getActionForLink(link)
this.visit(location.href, { action })
const acceptsStreamResponse = link.hasAttribute("data-turbo-stream")

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

// Navigator delegate
Expand Down
4 changes: 4 additions & 0 deletions src/http/fetch_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ export class FetchRequest {
return this.abortController.signal
}

acceptResponseType(mimeType: string) {
this.headers["Accept"] = [mimeType, this.headers["Accept"]].join(", ")
}

private async allowRequestToBeIntercepted(fetchOptions: RequestInit) {
const requestInterception = new Promise((resolve) => (this.resolveRequestPromise = resolve))
const event = dispatch<TurboBeforeFetchRequestEvent>("turbo:before-fetch-request", {
Expand Down
2 changes: 1 addition & 1 deletion src/observers/form_link_click_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class FormLinkClickObserver implements LinkClickObserverDelegate {
willFollowLinkToLocation(link: Element, location: URL, originalEvent: MouseEvent): boolean {
return (
this.delegate.willSubmitFormLinkToLocation(link, location, originalEvent) &&
(link.hasAttribute("data-turbo-method") || link.hasAttribute("data-turbo-stream"))
link.hasAttribute("data-turbo-method")
)
}

Expand Down
1 change: 0 additions & 1 deletion src/tests/fixtures/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ <h2>Frame: Form</h2>
</turbo-frame>
<a href="/src/tests/fixtures/frames/hello.html" data-turbo-method="get" id="link-method-outside-frame">Method link outside frame</a><br />
<a href="/__turbo/messages?content=Link!&type=stream" data-turbo-method="post" id="stream-link-method-outside-frame">Stream link outside frame</a>
<a href="/__turbo/messages?content=Link!&type=stream" data-turbo-stream id="stream-link-outside-frame">Stream link (no method) outside frame</a>
<form>
<a href="/src/tests/fixtures/frames/hello.html" data-turbo-method="get" id="link-method-within-form-outside-frame">Method link within form outside frame</a><br />
<a href="/__turbo/messages?content=Link!&type=stream" data-turbo-method="post" id="stream-link-method-within-form-outside-frame">Stream link within form outside frame</a>
Expand Down
1 change: 1 addition & 0 deletions src/tests/fixtures/visit.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ <h1>Visit</h1>
<hr class="push-below-fold">
<p><a id="below-the-fold-link" href="/src/tests/fixtures/one.html">one.html</a></p>
<hr class="push-below-fold">
<p><a id="stream-link" href="/src/tests/fixtures/one.html?key=value" data-turbo-stream>Stream link with ?key=value</a></p>
</section>
</body>
</html>
11 changes: 2 additions & 9 deletions src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -886,17 +886,10 @@ test("test stream link GET method form submission inside frame", async ({ page }
test("test stream link inside frame", async ({ page }) => {
await page.click("#stream-link-inside-frame")

const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")

assert.ok(fetchOptions.headers["Accept"].includes("text/vnd.turbo-stream.html"))
})

test("test stream link outside frame", async ({ page }) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As links like this are no longer treated as form submissions, I've moved this to the visit tests.

await page.click("#stream-link-outside-frame")

const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")
const { fetchOptions, url } = await nextEventNamed(page, "turbo:before-fetch-request")

assert.ok(fetchOptions.headers["Accept"].includes("text/vnd.turbo-stream.html"))
assert.equal(getSearchParam(url, "content"), "Link!")
})

test("test link method form submission within form inside frame", async ({ page }) => {
Expand Down
9 changes: 9 additions & 0 deletions src/tests/functional/visit_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Page, test } from "@playwright/test"
import { assert } from "chai"
import { get } from "http"
import {
getSearchParam,
isScrolledToSelector,
isScrolledToTop,
nextBeat,
Expand Down Expand Up @@ -178,6 +179,14 @@ test("test turbo:before-fetch-response open new site", async ({ page }) => {
assert.isTrue(fetchResponseResult.responseHTML.indexOf("An element with an ID") > -1)
})

test("test visits with data-turbo-stream include MIME type & search params", async ({ page }) => {
await page.click("#stream-link")
const { fetchOptions, url } = await nextEventNamed(page, "turbo:before-fetch-request")

assert.ok(fetchOptions.headers["Accept"].includes("text/vnd.turbo-stream.html"))
assert.equal(getSearchParam(url, "key"), "value")
})

test("test cache does not override response after redirect", async ({ page }) => {
await page.evaluate(() => {
const cachedElement = document.createElement("some-cached-element")
Expand Down