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

Revert "Dispatch visit events on the initiator link or form (#695)" #723

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Revert "Dispatch visit events on the initiator link or form (#695)" #723

merged 1 commit into from
Sep 19, 2022

Conversation

kevinmcconnell
Copy link
Collaborator

The inclusion of the initiator in VisitOptions causes a JS crash in the Turbo iOS adapter.

This is because Turbo iOS's implementation of visitProposedToLocation relies on being able to pass the entire options struct to native code. Initiator is an Element, which can't be passed in this way, resulting in a DataCloneError exception.

There is some discussion on this GitHub issue. This PR reverts the change for the moment, until we have an implementation of it that works with all adapters.

This reverts commit 8871817.

The inclusion of the `initiator` in `VisitOptions` causes a JS crash in
the Turbo iOS adapter.

This is because Turbo iOS's implementation of `visitProposedToLocation`
relies on being able to pass the entire options struct to native code.
Initiator is an `Element`, which can't be passed in this way, resulting
in a `DataCloneError` exception.

There is some discussion on this [GitHub issue][]. We're reverting the
change for the moment, until we have an implementation of it that works
with all adapters.

[GitHub issue]: #720

This reverts commit 8871817.
@dhh dhh merged commit ade31a6 into hotwired:main Sep 19, 2022
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 19, 2022
With the reverting of [hotwired#569][] (merged as
[hotwired#723][]), the "Visit with network error" test coverage
was missed in the diff.

Prior to the revert, the test was listening for
`turbo:fetch-request-error` events dispatched on the _link_ that was
clicked. Since that's no longer the case, this commit changes the test
to listen for `turbo:fetch-request-error` that bubble up to the
document.

[hotwired#569]: hotwired#569
[hotwired#723]: hotwired#723
@seanpdoyle
Copy link
Contributor

I've opened #725 to resolve this PR's CI test failures.

dhh pushed a commit that referenced this pull request Sep 19, 2022
With the reverting of [#569][] (merged as
[#723][]), the "Visit with network error" test coverage
was missed in the diff.

Prior to the revert, the test was listening for
`turbo:fetch-request-error` events dispatched on the _link_ that was
clicked. Since that's no longer the case, this commit changes the test
to listen for `turbo:fetch-request-error` that bubble up to the
document.

[#569]: #569
[#723]: #723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants