[✨] Critical DX/UX improvement to SPA navigation in Qwik City

[billykwok]

What is it?

• [x] Feature / Enhancement
• [x] Bug / Fixes
• [x] Docs / tests

Overview

This is a relatively straightforward change that addresses a number of interrelated issues around SPA navigation in Qwik City. Some are already mentioned in other issues (#2162, #2050, #3149, #2202, #2203), while some are newly reported here.

Support MPA-like and customizable scroll restoration in SPA

Before: Qwik City always scrolls to the top upon SPA navigation, unless there is a hash segment in the URL. This is not consistent with how browsers handle MPA scroll restoration on popstate and creates bad UX when users return to a page with a resetted scroll position. Moreover, the behavior is hard-coded into the Qwik City navigation logic, even though different applications have their own needs (e.g. app-like and page-like UIs should restore scroll positions differently).

After: The default behavior now mimics how browsers handle scroll restoration in MPA - scroll to the top for new pages (e.g. <Link />), but to the last-visited scroll position for popstate navigation (e.g. back/next button). Moreover, users can customize scroll restoration with the built-in scroll restorer toTopAlways and toLastPositionOnPopState or even their own implementation. The 'before' behavior is equivalent to toTopAlways.

• This provides feature parity against React Router and Remix
• This provides customizability for users to implement #2162 in userland

Race condition when restoring scroll positions

Before: Scroll position is restored after the DOM is 'settled' in order to synchronize content update and scroll position update. However, the current implementation relies on a series of hard-coded setTimeout(), which very often results in either early scrolling or late scrolling.

After: 'DOM Settled' is accurately detected using useLocation() and useVisibleTask$(). Scroll and content updates are now always in-sync.

• This provides feature parity against React Router and Remix
• This fixes #2050 and #3149

Prevent full-page reload when popstate happens before manual navigation

Before: Qwik City registers a temporary popstate event listener to reload the page if there is any popstate navigation before the first pushState is called. This essentially turns all SPAs into MPA whenever users refresh the page. This also makes page navigation inconsistent before and after the first pushState. In addition, there are unnecessary popstate listener registration/unregistration, because the temporary listener will be replaced by a real listener anyway after the first pushState.

After: Instead of a temporary popstate listener that does full-page refreshes, Qwik City now directly sets up the real popstate listener that updates the page without full-page refreshes.

• This makes Qwik City consistent with all other SPA frameworks in the market.
• This also makes page transition animation across 'popstate' possible.

Improve hash navigations

Before: Hash navigation is neither updating the URL hash nor written into the session history. The 'hashchange' event is also never fired.

After: The new behavior now mimics native behavior - clicking <a href="#some-hash" /> now updates the page URL hash without triggering unnecessary re-rendering. It also gets written into session history like what browsers would natively do. The 'hashchange' event is also fired.

• This fixes the hash issue mentioned in #3691.

Backward-compatible API Changes

const navigate = useNavigate();
// New signature that accepts an options object
type Options = { type: 'form' | 'link' | 'popstate'; forceReload: boolean; };
const defaultOptions: Options = { type: 'link', forceReload: false };
navigate('/some/pathname', defaultOptions);

// Old signature still works
navigate('/some/pathname', true /* equivalent { forceReload: true } */);
navigate('/some/pathname', false /* equivalent { forceReload: false } */);

import {
  QwikCityProvider,
  RouterOutlet,
  ServiceWorkerRegister,
  toTopAlways
} from '@builder.io/qwik-city';
import { RouterHead } from './components/router-head/router-head';

export default function Root() {
  return (
    <QwikCityProvider>
      <head>
        <meta charSet="utf-8" />
        <link rel="manifest" href="/manifest.json" />
        <RouterHead />
      </head>
      <body>
        //            ⬇️ New (optional): toTopAlways or toLastPositionOnPopState (default)
        <RouterOutlet restoreScroll$={toTopAlways} />
        <ServiceWorkerRegister />
      </body>
    </QwikCityProvider>
  );
}

Demo

Scroll restoration after popstate

Before the change:

• always to the top

https://user-images.githubusercontent.com/8078716/225979618-ce4e7ec9-996b-4258-915d-1b8dc8799a86.mp4

After the change:

• scroll to the last-visited positions

https://user-images.githubusercontent.com/8078716/225979653-01ea348c-6915-45be-a520-e7cd8fef3f6b.mp4

popstate after reload

Before the change:

• full-page reload the second time

https://user-images.githubusercontent.com/8078716/225979767-131b7914-4b1f-453d-818d-e6ce3a71755a.mp4

After the change:

• re-rendering

https://user-images.githubusercontent.com/8078716/225979800-d8e88838-a56f-44e0-9960-209271830857.mp4

Checklist

• [x] My code follows the developer guidelines of this project
• [x] I have performed a self-review of my own code
• [x] I have made corresponding changes to the documentation
• [x] Added new tests to cover the fix / functionality

[bolt-new-by-stackblitz[bot]]

Run & review this pull request in StackBlitz Codeflow.

[zanettin]

Hi @billykwok 👋 thank you very much for creating this PR 🙏

I just have a question based on the description:

Currently, pages are always scrolled to the top (or hashed element if specified in URLs) upon navigation. While this makes sense for <Link />-triggered navigation, popstate-triggered navigation fails to restore previous scroll positions. This does not match the native behavior of browsers nor that of competing frameworks.

you mention, that the restore for MPA (popstate) navigations does not work atm, which i can't confirm so far. At least not on the available browsers 😄 The <Link> component driven navigations are the SPA (pushstate) ones which you also show in the video.

MPA navigation scroll position restore

https://user-images.githubusercontent.com/3241476/222718122-f5b9a9a5-d70e-4c3a-aba8-df2f0917d680.mp4

Is the description wrong in that case or do i miss something? 👼 Thanks again for your time and contribution 🙏

[billykwok]

Thanks for checking out my change. It is intended for SPA scrolling only. When I said popstate, I was referring to popstate navigation in SPA (e.g. same-page re-rendering due to pressing back button after pushState). MPA does not need manual scroll restoration because browsers handle it by default. Hope this clarifies the change.

[billykwok]

Converted to draft for now as I found a new way to implement the change that is more flexible and robust. Will update it with the new implementation in the coming days.

[billykwok]

@adamdbradley The new implementation is now complete. I also added additional e2e tests to cover the new change. Please take a look and let me know if you have any questions. Thank you in advance!

[billykwok]

@zanettin @adamdbradley I know that the core team is probably quite busy at the moment and has its own priorities, but I have already needed to resolve merge conflicts more than a couple of times. I would really appreciate it if there is anyone who can at least spend a few minutes looking at this. I'm happy to make changes if the approach of this PR does not align with the team's vision. Thank you in advance.

[slowisfast168]

In my humble opinion, this is a MUST feature for any SPA. So glad @billykwok made this pull request. @qwik, @adamdbradley, @zanettin core team, merge it please! We need this awesome feature.

[billykwok]

Now there seems to be a bug in CI that prevents the GitHub API token from being read. I think clearing the GitHub Actions cache may resolve the issue.

[zanettin]

thanks again @billykwok 🙏 i try to bring some attention to this PR. and sorry for the delay. the team is hard at work 👼

[shairez]

thanks @billykwok for this thoughtful PR!

I say - to save your time, don't merge conflicts until @adamdbradley or @manucorporat will review this change. And once they'll do, we'll try to give the final reviews more focus so it'll get merged as soon as it's approved.

That way it won't take more of your time, and it'll give the team a chance to carefully review it without the pressure of the other things they're currently trying to get done.

Sounds reasonable?

[manucorporat]

Wonder if we can ship this without new APIs: <SinglePageNavigation restoreScroll$={toTopAlways} />

can it be part of the QwikCity provider?

or RouterOutlet

[billykwok]

Thanks for the suggestion. Yes, this can be done, for example, by putting <SinglePageNavigation /> inside <RouterOutlet /> and rendering it conditionally based on an MPA/SPA prop. What do you think about the following API?

<body>
  <RouterOutlet
    navigation="spa"
    restoreScroll$={toLastPositionOnPopstate} // omitted when navigation="mpa"
  />
</body>

[billykwok]

I just made a change to directly integrate <SinglePageNavigation /> into <RouterOutlet />. Now the public API should be simpler.

[billykwok]

Thank you. Having navigation set to 'spa' will result in a popstate listener being registered and a marker for detecting "dom update settled" being rendered. All SPAs work like MPA until JS is loaded.

If navigation is set to 'mpa' (i.e. opting out SPA), the listener and marker code will never be run. Such behavior is suitable for complete MPA like the Qwik Doc site.

Regarding automatic flipping to SPA with <Link />, Qwik City had a similar feature a few versions ago. I personally think that it sounds like a good idea but is hard to implement correctly. Some considerations include:

• Although not common and resulting in navigation dead end (bad UX), SPA can sometimes have pages without <Link />s (e.g. the leaf nodes of the site tree/graph).
• Most of the time, "whether a page/site is SPA or MPA" is known at SSR time or even compile time. The runtime cost of determining SPA vs MPA becomes pure browser-side overhead in those cases.
• Users who write their own <Link /> implementation using useNavigate() need to be covered as well.

Maybe some of these are the reasons why that feature was removed. But if the related concerns are addressed, I am open to this idea too.

[shairez]

thanks @billykwok

I agree with @jordanw66 I think we should strive for a MPA that turns into SPA as I really believe that most MPA (which aren't small documentation websites) will want to become SPAs after initial load (to preserve better UX), I think Qwik should make the transition seamless as much as possible.

[billykwok]

This PR does not make it harder to turn MPA into SPA.

Looks like the navigation props cause some confusion. The 'mpa' option is meant for optimizing sites that for sure don't want to be SPAs. Any other sites should be configured with 'spa', which makes a site both MPA-first and SPA-enhanced.

If pure MPA is not a use case that Qwik wants to optimize for, I also agree that it is best to further simplify the API by removing the navigation option and just including SPA-specific initialization logic even for pure MPA pages.

[billykwok]

I just realized Qwik has been able to avoid running most of my updated SPA initialization code all along. The popstate and SPA-specific listeners will only be downloaded and run when the first event arrives (which will never happen for pure MPA). It removes the need for opting in/out MPA/SPA. The SPA functionalities are loaded incrementally and naturally as users interact with the page.

@adamdbradley @manucorporat I pushed another (and probably the last) update to this PR to reflect this change.

@jordanw66 Regarding the edge case that you mentioned for leaf nodes/dead ends, it has been one of the motivations of this PR from the very beginning. It replaces the original MPA refresh with a soft SPA re-render when users trigger popstate after a manual refresh at a dead end. (Refer to "Prevent full-page reload when popstate happens before manual navigation")

[billykwok]

Would you mind clarifying your question? As far as I know, the View Transition API leaves the DOM update part to the users.

If you are implying that View Transition API may be used for detecting DOM 'settled' time / restore scroll position, it probably can't. It relies on users to tell how and when the DOM is changed so that it can set up the corresponding pseudo-DOM for transition. It is not related to the DOM settled detection mechanism in this PR, which is caused by the asynchronous nature of Qwik and the inability to tell directly when the DOM is actually updated after a rerun of the component function.

If you are implying that View Transition API can be used for enriching Qwik City navigation in the future, absolutely! However, I guess the required change needs to be implemented in Qwik in addition to Qwik City. Only Qwik has information about when the DOM change is flushed to the actual DOM, and the View Transition API requires such information.

[manucorporat]

So, i would like to push the limits a bit more, i dont like users to choose between SPA and MPA, qwik is kind of designed to blur the lines a bit. Is it really necessary for developers to choose spa? what's the disadvantage?

[manucorporat]

I would really like to merge this API, i am a bit worry about a regression in initial loaded JS and the mutation observer code, not entirely sure, why we need it. But not a fan of that part.

[billykwok]

I would really like to merge this API, i am a bit worry about a regression in initial loaded JS and the mutation observer code, not entirely sure, why we need it. But not a fan of that part.

Regarding the initial JS load, I don't have the numbers yet, but I don't think this would have any impact since all codes are either executed on the server or downloaded after users click the back/forward button or <Link />. If anything, the change actually reduced the initial load by replacing the 'somewhat-eagerly-loaded' 'qinit' listener with the 'lazily-loaded' 'popstate' and 'qnav' listeners in <RouterOutlet />. Would like to know which part of the change you think would increase the initial JS load.

For MutationObserver, I think both Point 3 in the PR and the comment provide an explanation. Could you clarify which part you don't understand? I would love to find another way to reliably detect when Qwik flushes DOM update to the actual DOM tree. Since Qwik does not expose such information, a short-lived MutationObserver is so far the only solution I can think of. Any suggestion is welcome.

[billykwok]

I just pushed an update to address all of the comments so far.

• There is now an even simpler way to detect DOM settlement accurately based on just useLocation() and useVisibleTask$(). The MutationObserver is no longer there.
• The hash issue mentioned in #3691 is now addressed.
• I removed the proposed 'qinit'/'qsettled' event API to reduce the scope.

@manucorporat Thank you for spending time looking at it previously. Hope you manage to squeeze some time on it again.

[manucorporat]

I want to merge this PR, but there are so problems, the thing i am more against is the usage of useVisibleTask and useWindowOn() in RouterOutlet, right now, QwikCity apps download no js, adding this changes the baseline, which affects demos and simple apps.

[billykwok]

Thanks for following up. I appreciate the pursuit of avoiding the initial JS load at all costs. However, there is probably no simple way to achieve the intended result of this PR with Qwik's current design/APIs.

While I agree that the current implementation may not fully align with Qwik's vision, the UX issues in scroll restoration mentioned in the original post should still be fixed ASAP. It is not desirable and hurts adoption if this is left hanging. There might be some clever tricks that I'm not aware of (perhaps through some unknown internal APIs). Unfortunately, I've decided to move on from this.

For the hard refresh issue specifically, it may not be that big of a concern after all. Since Qwik embraces and optimizes for MPA more than most JS frameworks (except Astro), maybe triggering a hard page refresh on popstate is acceptable for most Qwik adopters (unfortunately I am not among this crowd).

Qwik has introduced so many innovative solutions to improve the initial page load experience, and it has so much potential IMO. I am happy to revisit it one day when it is more compatible with SPA. But until then, if someone cares enough about the SPA Scrolling UX in Qwik and has a solution that fits into Qwik's vision, please feel free to pick this up and fix it once and for all.

I am closing this PR for now. Thank everyone who has put time into it.

[manucorporat]

Hey! actually i am working on it, i think it's possible

[manucorporat]

We will get it merged!

[manucorporat]

Just pushed a bunch of changes! can you review and get your thoughts? @billykwok

[manucorporat]

Do you think we could merge and iterate, seems like this PR would fix some issues + add a cool feature, what if we leave for a second PR:

• Fix flickering
• Fix the popstate (very tricky one)

[manucorporat]

This PR tried to do many things at the same time and it makes it very hard to merge, might be a good moment to merge and iterate

[manucorporat]

let me try to find him in discord!