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.

Sign up for GitHub

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

Jump to bottom

Turbo stream refresh takes precedence over user interaction #1209

Open
rbclark opened this issue Mar 1, 2024 · 3 comments · May be fixed by #1213
Open

Turbo stream refresh takes precedence over user interaction #1209

rbclark opened this issue Mar 1, 2024 · 3 comments · May be fixed by #1213

Comments

@rbclark
Copy link

rbclark commented Mar 1, 2024

I am running into a bit of an interesting race condition when using Turbo 8 with broadcasted refresh actions. It seems that the streaming refresh action takes precedence over user interactions on the page, meaning a user could click a link on a page and instead receive the refresh response of the previous page. I've attached a video of the behavior and a simple repository to reproduce the issue. In this video I have simulated an app that is a bit slower to respond by adding a 2 second delay to the index route, and then I am clicking on the headings in order to sort the users list.

Clicks.being.overridden.turbo.mov

In the console at the bottom of the screen you can see my click action fire and then be cancelled by the turbo stream refresh streaming from the server. Also note that the list maintains it's original order.

The demo app in the video can be found at: https://github.com/rbclark/turbo-8-stimulus-bug-demo. I've tried playing around with different ways of disabling turbo refreshes once the user has interacted with the page using the different turbo:before-* events but so far to no avail.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Mar 1, 2024
@seanpdoyle
Prevent Refresh from interrupting ongoing Visit
04e5c65 
Closes [hotwired#1209]

Defers the `Session.refresh` calls while handling `<turbo-stream
action="refresh">` elements if there is a `Visit` that's already
initiated and being handled by the `Navigator`.

[hotwired#1209]: hotwired#1209
@seanpdoyle seanpdoyle linked a pull request Mar 1, 2024 that will close this issue
Open
@seanpdoyle
Copy link
Contributor

Thank you for opening this issue, and thank you for sharing a bug report repository.

If the root of the issue is that receiving a <turbo-stream> while a Visit is in-flight, I've opened #1213 to try and resolve that race condition.

Are you able to consume that fork's branch and test whether or not it resolves your application's issues?

@rbclark
Copy link
Author

rbclark commented Mar 1, 2024

Thank you for the quick reply! I've gone ahead and applied the changes locally and it seems that this.navigator.currentVisit is being set and persisting, causing refreshes to stop working after the first refresh.

only-one-refresh.mov

I went ahead and threw a breakpoint in and it looks like this.navigator.currentVisit is being set after morph.

only-one-refresh-with-debugging.mp4

Full disclosure: I was having a little bit of trouble getting a local build of Turbo that I could point to in my rails project with actioncable working so I just went ahead and directly modified app/assets/javascripts/turbo.js in my local copy of the turbo-rails project in order to test this change. I don't believe that accounts for the new behavior I'm seeing though.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Mar 1, 2024
@seanpdoyle
Prevent Refresh from interrupting ongoing Visit
1f83d52 
Closes [hotwired#1209]

Defers the `Session.refresh` calls while handling `<turbo-stream
action="refresh">` elements if there is a `Visit` that's already
initiated and being handled by the `Navigator`.

[hotwired#1209]: hotwired#1209
@seanpdoyle
Copy link
Contributor

@rbclark thank you for that feedback. I've pushed up a commit to #1213 to ensure that the navigator's currentVisit is unset when the visit completes. Does that resolve your issues?

If you're willing to test it out, could you respond with your feedback directly within the #1213 PR?

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Mar 5, 2024
@seanpdoyle
Prevent Refresh from interrupting ongoing Visit
be4c81f 
Closes [hotwired#1209]

Defers the `Session.refresh` calls while handling `<turbo-stream
action="refresh">` elements if there is a `Visit` that's already
initiated and being handled by the `Navigator`.

[hotwired#1209]: hotwired#1209
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Mar 5, 2024
@seanpdoyle
Prevent Refresh from interrupting ongoing Visit
19267e7 
Closes [hotwired#1209]

Defers the `Session.refresh` calls while handling `<turbo-stream
action="refresh">` elements if there is a `Visit` that's already
initiated and being handled by the `Navigator`.

[hotwired#1209]: hotwired#1209
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Mar 29, 2024
@seanpdoyle
Prevent Refresh from interrupting ongoing Visit
4d8af53 
Closes [hotwired#1209]

Defers the `Session.refresh` calls while handling `<turbo-stream
action="refresh">` elements if there is a `Visit` that's already
initiated and being handled by the `Navigator`.

[hotwired#1209]: hotwired#1209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Prevent Refresh from interrupting ongoing Visit seanpdoyle/turbo
2 participants
@rbclark @seanpdoyle