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

ViewTransitions causes out of order navigation when interacting with slow-to-load non-prerendered routes #10807

Closed
1 task
kelvindecosta opened this issue Apr 17, 2024 · 2 comments · Fixed by #10900
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: view transitions Related to the View Transitions feature (scope)

Comments

@kelvindecosta
Copy link

Astro Info

Astro                    v4.6.2
Node                     v18.18.0
System                   Linux (x64)
Package Manager          unknown
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Pages that use the ViewTransitions routing component can be rendered out of order during navigation, if certain pages take longer to render.

I've noticed this behavior with dynamic routes that take a long time to load.

Note
The issue isn't with the fact that the load time is long (this can be intentional/expected when data-fetching, for example).

To reproduce this behavior:

  1. Visit the attached reproduction on StackBlitz
  2. From the Home page, click the Blog link. This begins the navigation to the Blog route, which is intentionally slow and waits 10 seconds to provide a response.
  3. Before the Blog page is rendered, click the About link. This should immediately render the About page.
  4. Wait for a while.

After the wait, the Blog page is rendered, albeit out of order. This shouldn't have happened.

What's the expected result?

The behavior should be similar to that when not using the ViewTransitions component.
In this case, there is no out-of-order navigation.

To experience the expected behavior in the reproduction on StackBlitz:

  1. Disable the ViewTransitions component (say, by commenting it out).
  2. Repeat steps 2-4 from before.

Now, the About page persists and there are no further route changes.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-orshkz?file=src%2Flayouts%2Froot-layout.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Apr 17, 2024
@martrapp martrapp added - P3: minor bug An edge case that only affects very specific usage (priority) feat: view transitions Related to the View Transitions feature (scope) and removed needs triage Issue needs to be triaged labels Apr 20, 2024
@martrapp
Copy link
Member

martrapp commented Apr 20, 2024

Hi @kelvindecosta, thank you for opening this issue! I'm sorry you had to wait so long for an initial response, but somehow this entry didn't appear on my radar.

The behavior of parallel calls to navigate is undefined. But I agree that mirroring the behavior of the case without view transitions might be the right solution.

Technically it is easy to detect the attempt for a parallel navigation. We should also be able to abort the current one. I'm just a bit unsure whether or how this would require an extension of the API e.g. to allow code to react to or at least recognize the situation.

@martrapp
Copy link
Member

I'm using this comment to discuss possible extensions to Astro's view transitions to close this issue.

These are the async parts of the router:

  • the loader (built-in implementation calling fetch)
  • awaiting old animations in the simulation for browsers without native view transition support
  • the update callback of startViewTransition, even though our swap() is currently full synchronous

Up to the point where where we update the browser history, we are on the "old page", after that on the "new page".

A parallel call to navigate() can occur anytime as one trigger would be the history buttons of the browser UI.

The minimal solution for the above issue would be to silently discard a navigation after the preparation phase if another one startet in the meanwhile. But when we tackle that now, we should also be prepared for calls to navigate() after the preparation phase.

If the second call starts after the preparation phase, there should be some way to prevent a second view transition being started in parallel to an existing one as this will raise errors / aborts in the view transition API.

It would be beneficial not only to ignore but also to abort the outstanding request when we detect a parallel navigation.

Proposed changes:

  • Add a current navigation to the state, which is only defined during an active navigation up to the point right after after-swap is called (i.e. after DOM update, history update, and scrolling).
  • If navigate is called with an active current navigation, abort the current navigation and make the new navigation current.
  • Add the signal of an AbortController to the preparation event (e.g. to be used with fetch)
  • If the current navigation get's aborted, do not fallback to the browsers default behavior, i.e. do not set location.href = event.to
  • Do not start a new view transition if there is another active one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: view transitions Related to the View Transitions feature (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants