Skip to content

Commit

Permalink
Override FetchOptions from event listeners
Browse files Browse the repository at this point in the history
The problem
---

Prior to this commit, instances of `FormSubmission` and `FetchRequest`
referenced in dispatched `turbo:submit-start` and
`turbo:before-fetch-request` events were tedious to update.

For example, the `FormSubmission` instance made available through
`event.detail.formSubmission` exposed properties that _seemed_ readable
and writable.

However, mutations to `formSubmission.method` or
`formSubmission.location` occur _after_ the underlying `FetchRequest`
instance is built. This timing of that instance's construction has
subtle and not obvious implications. In practice, writes to these
properties are unobserved by the underlying `FetchRequest` instance, and
have no effect on the `fetch` call.

If calling applications are aware of these issues and limitations, they
can work around them by mutating the
`event.detail.formSubission.fetchRequest` instance.

The proposal
---

After this commit, the `FormSubmission.constructor` will deduce the
required data (like `[method]` and `[action]` values) then use those
values to construct the `FetchRequest` instance. After that
construction, the reads and writes to the `FormSubmission` properties
will _delegate_ to the `FetchRequest` instance.

In tandem with those changes, the `FetchRequest` will follow a similar
pattern: extract as much information up-front during construction, then
delegate property reads and writes to the correct instance. In most
cases, it will delegate to its [`fetchOptions:
RequestInit`][RequestInit] instance.

To achieve this behavior, several module-local functions and type
definitions were shuffled around between the `FormSubmission` and
`FetchRequest` modules. The order of arguments for the `FetchRequest`
constructor were also re-arranged to support omitting values that are
made redundant by default arguments.

[RequestInit]: https://developer.mozilla.org/en-US/docs/Web/API/fetch#parameters
  • Loading branch information
seanpdoyle committed Sep 14, 2022
1 parent e592476 commit d863abf
Show file tree
Hide file tree
Showing 7 changed files with 253 additions and 109 deletions.
126 changes: 61 additions & 65 deletions src/core/drive/form_submission.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { FetchRequest, FetchMethod, fetchMethodFromString, FetchRequestHeaders } from "../../http/fetch_request"
import {
FetchEnctype,
FetchMethod,
FetchRequest,
FetchRequestHeaders,
fetchMethodFromString,
fetchEnctypeFromString,
isIdempotent,
} from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { expandURL } from "../url"
import { dispatch, getAttribute, getMetaContent, hasAttribute } from "../../util"
Expand All @@ -23,35 +31,17 @@ export enum FormSubmissionState {
stopped,
}

enum FormEnctype {
urlEncoded = "application/x-www-form-urlencoded",
multipart = "multipart/form-data",
plain = "text/plain",
}

export type TurboSubmitStartEvent = CustomEvent<{ formSubmission: FormSubmission }>
export type TurboSubmitEndEvent = CustomEvent<
{ formSubmission: FormSubmission } & { [K in keyof FormSubmissionResult]?: FormSubmissionResult[K] }
>

function formEnctypeFromString(encoding: string): FormEnctype {
switch (encoding.toLowerCase()) {
case FormEnctype.multipart:
return FormEnctype.multipart
case FormEnctype.plain:
return FormEnctype.plain
default:
return FormEnctype.urlEncoded
}
}

export class FormSubmission {
readonly delegate: FormSubmissionDelegate
readonly formElement: HTMLFormElement
readonly submitter?: HTMLElement
readonly formData: FormData
readonly location: URL
readonly fetchRequest: FetchRequest
readonly enctype: FetchEnctype
readonly mustRedirect: boolean
state = FormSubmissionState.initialized
result?: FormSubmissionResult
Expand All @@ -70,53 +60,44 @@ export class FormSubmission {
submitter?: HTMLElement,
mustRedirect = false
) {
const method = getMethod(formElement, submitter)
const action = getAction(getFormAction(formElement, submitter), method)
const body = getFormData(formElement, submitter)
const enctype = getEnctype(formElement, submitter)
this.delegate = delegate
this.fetchRequest = new FetchRequest(this, action, formElement, method, body, enctype)
this.formElement = formElement
this.submitter = submitter
this.formData = buildFormData(formElement, submitter)
this.location = expandURL(this.action)
if (this.method == FetchMethod.get) {
mergeFormDataEntries(this.location, [...this.body.entries()])
}
this.fetchRequest = new FetchRequest(this, this.method, this.location, this.body, this.formElement)
this.enctype = enctype
this.mustRedirect = mustRedirect
}

get method(): FetchMethod {
const method = this.submitter?.getAttribute("formmethod") || this.formElement.getAttribute("method") || ""
return fetchMethodFromString(method.toLowerCase()) || FetchMethod.get
get location(): URL {
return this.fetchRequest.url
}

get action(): string {
const formElementAction = typeof this.formElement.action === "string" ? this.formElement.action : null
get method() {
return this.fetchRequest.method
}

if (this.submitter?.hasAttribute("formaction")) {
return this.submitter.getAttribute("formaction") || ""
} else {
return this.formElement.getAttribute("action") || formElementAction || ""
}
set method(value: FetchMethod | string) {
this.fetchRequest.method = value
}

get body() {
if (this.enctype == FormEnctype.urlEncoded || this.method == FetchMethod.get) {
return new URLSearchParams(this.stringFormData)
} else {
return this.formData
}
get action(): string {
return this.fetchRequest.url.toString()
}

get enctype(): FormEnctype {
return formEnctypeFromString(this.submitter?.getAttribute("formenctype") || this.formElement.enctype)
set action(value: string) {
this.fetchRequest.url = expandURL(value)
}

get isIdempotent() {
return this.fetchRequest.isIdempotent
get body() {
return this.fetchRequest.body
}

get stringFormData() {
return [...this.formData].reduce((entries, [name, value]) => {
return entries.concat(typeof value == "string" ? [[name, value]] : [])
}, [] as [string, string][])
get isIdempotent() {
return this.fetchRequest.isIdempotent
}

// The submission process
Expand Down Expand Up @@ -220,7 +201,36 @@ export class FormSubmission {
}
}

function buildFormData(formElement: HTMLFormElement, submitter?: HTMLElement): FormData {
function getFormAction(formElement: HTMLFormElement, submitter: HTMLElement | undefined): string {
const formElementAction = typeof formElement.action === "string" ? formElement.action : null

if (submitter?.hasAttribute("formaction")) {
return submitter.getAttribute("formaction") || ""
} else {
return formElement.getAttribute("action") || formElementAction || ""
}
}

function getAction(formAction: string, fetchMethod: FetchMethod): URL {
const action = expandURL(formAction)

if (isIdempotent(fetchMethod)) {
action.search = ""
}

return action
}

function getMethod(formElement: HTMLFormElement, submitter: HTMLElement | undefined): FetchMethod {
const method = submitter?.getAttribute("formmethod") || formElement.getAttribute("method") || ""
return fetchMethodFromString(method.toLowerCase()) || FetchMethod.get
}

function getEnctype(formElement: HTMLFormElement, submitter: HTMLElement | undefined): FetchEnctype {
return fetchEnctypeFromString(submitter?.getAttribute("formenctype") || formElement.enctype)
}

function getFormData(formElement: HTMLFormElement, submitter: HTMLElement | undefined): FormData {
const formData = new FormData(formElement)
const name = submitter?.getAttribute("name")
const value = submitter?.getAttribute("value")
Expand All @@ -246,17 +256,3 @@ function getCookieValue(cookieName: string | null) {
function responseSucceededWithoutRedirect(response: FetchResponse) {
return response.statusCode == 200 && !response.redirected
}

function mergeFormDataEntries(url: URL, entries: [string, FormDataEntryValue][]): URL {
const searchParams = new URLSearchParams()

for (const [name, value] of entries) {
if (value instanceof File) continue

searchParams.append(name, value)
}

url.search = searchParams.toString()

return url
}
4 changes: 2 additions & 2 deletions 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, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { History } from "./history"
import { getAnchor } from "../url"
Expand Down Expand Up @@ -226,7 +226,7 @@ export class Visit implements FetchRequestDelegate {
if (this.hasPreloadedResponse()) {
this.simulateRequest()
} else if (this.shouldIssueRequest() && !this.request) {
this.request = new FetchRequest(this, FetchMethod.get, this.location, undefined, this.initiator)
this.request = new FetchRequest(this, this.location, this.initiator)
this.request.perform()
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
FrameLoadingStyle,
FrameElementObservedAttribute,
} from "../../elements/frame_element"
import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
import {
Expand Down Expand Up @@ -339,7 +339,7 @@ export class FrameController
// Private

private async visit(url: URL) {
const request = new FetchRequest(this, FetchMethod.get, url, new URLSearchParams(), this.element)
const request = new FetchRequest(this, url, this.element)

this.currentFetchRequest?.cancel()
this.currentFetchRequest = request
Expand Down
Loading

0 comments on commit d863abf

Please sign in to comment.