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

Reintroduce Visit identifier property #718

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Reintroduce Visit identifier property #718

merged 1 commit into from
Sep 19, 2022

Conversation

kevinmcconnell
Copy link
Collaborator

This property is required by turbo-ios, so removing it causes crashes within that library.

E.g. https://github.com/hotwired/turbo-ios/blob/main/Source/WebView/WebViewBridge.swift#L129

This reverts commit d81e24d.

Copy link
Member

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kevinmcconnell! That also explains why it was unused throughout the @hotwired/turbo codebase...

Would you mind adding a comment for this, so that we don't run into the same issue again?

This property is required by turbo-ios, so removing it causes crashes
within that library.

E.g. https://github.com/hotwired/turbo-ios/blob/main/Source/WebView/WebViewBridge.swift#L129

This reverts commit d81e24d.
@kevinmcconnell
Copy link
Collaborator Author

@marcoroth yeah, same thing just occurred to me after pushing 👍 Added now.

@dhh dhh merged commit fb05e84 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#718][]), 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#718]: hotwired#718
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.

None yet

3 participants