-
Notifications
You must be signed in to change notification settings - Fork 436
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
Remove SubmitEvent
polyfill
#909
base: main
Are you sure you want to change the base?
Conversation
src/polyfills/submit-event.ts
Outdated
return // polyfill not needed | ||
} | ||
|
||
addEventListener("click", clickCaptured, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a related note, capturing a click
event for a submission does not polyfill the built-in behavior for calls to HTMLFormElement.requestSubmit()
with a submitter
argument. For example:
<form>
<button id="the-submitter">Submit</button>
<button type="reset" id="not-the-submitter">Reset</button>
</form>
<script>
const form = document.querySelector("form")
const submitter = document.getElementById("the-submitter")
form.requestSubmit(submitter)
</script>
In this case, the submitter
argument is ignored, since it's never clicked and will not dispatch a click
event.
I believe it is too early to remove it. Many people are still on devices that can't upgrade to Safari 15.4+. See https://caniuse.com/mdn-api_submitevent_submitter with "Usage relative" selected. As a side note, is there a browser support policy for the project? |
Would it be an appropriate stance to consider inlined, built-in polyfills outside the bounds of |
I'm not a maintainer and I realise there is a burden keeping polyfills within the Turbo codebase. That being said, as a user and someone setting up Turbo for several companies, I appreciate the library working out of the box with browsers that still have enough users. |
750ec6c
to
29126ab
Compare
The codebase's [SubmitEvent][] polyfill code involves three parts: * a general purpose polyfill for browsers that do not provide a `window.SubmitEvent` prototype that includes a `submitter` property for their [submit][] events * a Safari-specific bugfix * a TypeScript `type` declaration. As of April 2023, all facets of the `SubmitEvent` have fully support and [Compatibility][] with evergreen browsers. As of 2021-08-30, the [Safari bug][] mentioned in the code comment has been resolved, and a fix has been released as part of Safari 15.4. The TypeScript compiler supports the `SubmitEvent` type declaration. This commit removes the polyfill from the codebase, and relies on the built-in support instead. [submit]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/submit_event [SubmitEvent]: https://developer.mozilla.org/en-US/docs/Web/API/SubmitEvent [Compatibility]: https://developer.mozilla.org/en-US/docs/Web/API/SubmitEvent#browser_compatibility [Safari bug]: https://bugs.webkit.org/show_bug.cgi?id=229660
29126ab
to
6e89b75
Compare
@jorgemanrubia @brunoprietog has enough time passed to consider removing these inlined polyfills? |
The codebase's SubmitEvent polyfill code involves three parts:
window.SubmitEvent
prototype that includes asubmitter
property for their submit eventstype
declaration.As of April 2023, all facets of the
SubmitEvent
have fully support and Compatibility with evergreen browsers.As of 2021-08-30, the Safari bug mentioned in the code comment has been resolved, and a fix has been released as part of Safari 15.4.
The TypeScript compiler supports the
SubmitEvent
type declaration.This commit removes the polyfill from the codebase, and relies on the built-in support instead.