Override FetchOptions from event listeners
#691
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seanpdoyle
force-pushed
the
request-event-listeners
branch
4 times, most recently
from
31d7816 to
d0a147e
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
8 times, most recently
from
3a8c75e to
d863abf
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
2 times, most recently
from
92be277 to
ca83668
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
from
ca83668 to
0ca8b9a
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
from
0ca8b9a to
771264f
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
2 times, most recently
from
049fef3 to
dc5ea66
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
from
dc5ea66 to
1ea6912
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
from
1ea6912 to
538fa3e
Compare
seanpdoyle
force-pushed
the
request-event-listeners
branch
4 times, most recently
from
b0ba4fa to
a407922
Compare
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
seanpdoyle
force-pushed
the
request-event-listeners
branch
from
a407922 to
3fb6722
Compare
|
@afcapel @kevinmcconnell I've rebased this branch to account for the removal of types. It's ready for review. |
|
@seanpdoyle looks good, clever refactor! 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem
Prior to this commit, instances of
FormSubmissionandFetchRequestreferenced in dispatched
turbo:submit-startandturbo:before-fetch-requestevents were tedious to modify from a hostapplication.
For example, the
FormSubmissioninstance made available throughevent.detail.formSubmissionexposed properties that seemed writable.Unfortunately, any mutations to
formSubmission.methodorformSubmission.locationfrom within host application event listeners(like
turbo:before-fetch-request) occur after the underlyingFetchRequestinstance is built, which has subtle and not obviousimplications. In practice, modifications to these properties are
unobserved by the underlying
FetchRequestinstance, and have no effecton the ensuing
fetchcall.Luckily, if host applications are aware of these issues and limitations,
they can work around them by mutating the
event.detail.formSubission.fetchRequestinstance.The proposal
After this commit, the
FormSubmission.constructorwill infer therequired data (like
[method]and[action]values) from itsHTMLFormElementand optionalHTMLElementarguments, then use thosevalues to construct the
FetchRequestinstance.After that construction, any changes to the
FormSubmissionpropertieswill delegate to the
FetchRequestinstance.In tandem with those changes, the
FetchRequestwill follow a similarpattern: extract as much information up-front during construction, then
delegate property reads and writes to properties. In most
cases, it will delegate to its
fetchOptions: RequestInitinstance.To achieve this behavior, several module-local functions and type
definitions were shuffled around between the
FormSubmissionandFetchRequestmodules. The order of arguments for theFetchRequestconstructor were also re-arranged to support omitting values that are
made redundant by default arguments.