[Deprecation]: Remove internal objects from public API #1000
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
commented
Sep 12, 2023
| submitter: this.submitter, | ||
|
|
||
| get formSubmission() { | ||
| console.warn("`event.detail.formSubmission` is deprecated. Use `event.target`, `event.detail.submitter`, and `event.detail.request` instead") |
There was a problem hiding this comment.
@afcapel @kevinmcconnell I realize now that the removal of TypeScript in anticipation of an 8.0 release means that a deprecation cycle as part of a minor release is no longer possible.
I'm curious of your thoughts here. Ultimately, we can remove the properties entirely and skip the console.warn calls.
|
Congrats, you're the 1000th PR! You win a prize 💯 * 🔟 🥳 |
seanpdoyle
force-pushed
the
fetch-request-read-only
branch
2 times, most recently
from
6994399 to
ed669b8
Compare
seanpdoyle
commented
Sep 12, 2023
| addEventListener("turbo:submit-start", (({ detail }) => { | ||
| detail.formSubmission.method = "get" | ||
| detail.formSubmission.action = "/src/tests/fixtures/one.html" | ||
| detail.formSubmission.body.set("greeting", "Hello, from an event listener") |
There was a problem hiding this comment.
The built-in Request class is much more immutable than our internal FetchRequest. All of its properties are read-only, so consumer's wouldn't be able to re-write data like the method, URL, or body. Mostly, consumers can re-write Headers.
Since this wasn't quite possible until #691, it's a breaking change to unreleased behavior.
The `FormSubmission` delegate methods refer to arguments of type `FetchRequest` as `request`, and arguments of `FetchResponse` as `response`. Without type annotations to go along with those names, it isn't clear that `request` is not a built-in `Request` and `response` is not a built-in `response`. This commit renames them to communicate their underlying types.
seanpdoyle
force-pushed
the
fetch-request-read-only
branch
from
ed669b8 to
1a1bdff
Compare
seanpdoyle
force-pushed
the
fetch-request-read-only
branch
2 times, most recently
from
7ce2a56 to
647323f
Compare
The `FetchRequest`, `FetchResponse`, and `FormSubmission` objects are internal abstractions that facilitate Turbo's management of HTTP and Form Submission life cycles. Currently, references to instances of those classes are available through `event.detail` for the following events: * `turbo:frame-render` * `turbo:before-fetch-request` * `turbo:before-fetch-response` * `turbo:submit-start` * `turbo:submit-end` Similarly, the `turbo:before-fetch-request` exposes a `fetchOptions` object that is a separate instance from the one used to submit the request. This means that **any** modifications to **any** value made from within an event listener are ineffective and **do not change** the ensuing request. This commit deprecates those properties in favor of their built-in foundations, namely: * [Request][] * [Response][] The properties that expose those instances will remain as deprecations, but will be inaccessible after an `8.0` release. [Request]: https://developer.mozilla.org/en-US/docs/Web/API/Request [Response]: https://developer.mozilla.org/en-US/docs/Web/API/Response
seanpdoyle
force-pushed
the
fetch-request-read-only
branch
from
647323f to
2a9a0c6
Compare
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.
Motivation
The removal of exported TypeScript types and a hesitance to introduce statically enforceable JSDoc comments means that the package's is even more responsible for maintaining backwards compatibility and a stable public interface.
While the majority of the application's "public interface" is driven through HTML
[data-*]attributes and event listeners, there are some Turbo-defined Class interfaces that applications can interact with.This pull request aims to reduce the surface area of those public interfaces, with the purposes of:
Summary
The
FetchRequest,FetchResponse, andFormSubmissionobjects are internal abstractions that facilitate Turbo's management of HTTP and Form Submission life cycles.Currently, references to instances of those classes are available through
event.detailfor the following events:turbo:frame-renderturbo:before-fetch-requestturbo:before-fetch-responseturbo:submit-startturbo:submit-endSimilarly, the
turbo:before-fetch-requestexposes afetchOptionsobject that is a separate instance from the one used to submit the request. This means that any modifications to any value made from within an event listener are ineffective and do not change the ensuing request.Details
This commit deprecates those properties in favor of their built-in foundations, namely:
The properties that expose those instances will remain as deprecations, but will be inaccessible after an
8.0release.