sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.73k stars 1.94k forks source link

Swipe to navigate back on iOS causes flicker of current page when `load` performs asynchronous work #10700

Open quentin-fox opened 1 year ago

quentin-fox commented 1 year ago

Describe the bug

Setup: using adapter-node, you have two pages that both have a load function, regardless of if it's in a +page.(js/ts) or +page.server.(js/ts), and the load functions are not synchronous. You navigate from page A to page B, and then from page B, you swipe "back", to navigate back to page A. Initially, page A is shown, but it will flicker back to page B while the load function for page A is re-running.

Below is an image of what happens visually, using the iOS simulator to demonstrate.

https://github.com/sveltejs/kit/assets/5247826/9d65011e-aef0-4018-b580-21b513effa65

In this reproduction, I've set the timeout in the load function to be 1s in page A to make the flicker/lag in navigation more obvious.

In this second video, I've dropped the timeout in the load function to be 100ms. Here, you can see that if you are holding the back navigation for more time than it takes for the load function to complete, then there is no flicker back to page B. This is consistent with the behaviour that I've seen in production environments.

https://github.com/sveltejs/kit/assets/5247826/4be1cda1-26ee-44b4-89f7-722d9c062498

As a slightly separate, but likely related bug, I've also noticed cases where you swipe back to navigate back from page B to page A, and page A just never renders. However, the browser things the navigation has completed, so swiping back doesn't take you to page A, even though it looks like you're on page B - instead, it takes you to whatever page you were on prior to the initial visit to page A.

I haven't been able to reproduce this bug consistently, but ideally if the main issue is addressed, then this other bug may also be fixed in the process.

EDIT: I think this is related to #10512?

Reproduction

Reproduction:

  1. Clone repo, install deps, build app, then node build
  2. Open http://localhost:3000/ in an iOS simulator
  3. Click on the "sub route" <a /> tag on the main page
  4. Swipe back to navigate to the main page from the /sub route
  5. observe flicker

You can modify the setTimeout in the src/routes/+page.ts to make the flicker longer or shorter. In practice, I've noticed load functions (on heavier pages) taking 50-100ms to complete, so that would be a more accurate representation of what the flicker would look like in a production environment.

https://github.com/quentin-fox/back-navigation

Logs

No logs are generated by the server/browser when reproducing the issue.

System Info

System:
    OS: macOS 11.5.2
    CPU: (8) arm64 Apple M1
    Memory: 104.95 MB / 8.00 GB
    Shell: 3.6.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 20.5.1 - ~/.asdf/installs/nodejs/20.5.1/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.8.0 - ~/.asdf/plugins/nodejs/shims/npm
    Watchman: 2023.08.14.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 116.0.5845.179
    Chrome Canary: 119.0.5999.0
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-node: ^1.3.1 => 1.3.1 
    @sveltejs/adapter-static: ^2.0.3 => 2.0.3 
    @sveltejs/kit: ^1.20.4 => 1.24.1 
    svelte: ^4.0.5 => 4.2.0 
    vite: ^4.4.2 => 4.4.9

Severity

annoyance

Additional Information

No response

quentin-fox commented 1 year ago

Did a bit of digging in the source code, especially the 'popstate' listener and navigate functions in this file:

https://github.com/sveltejs/kit/blob/09dc8d4a437beda64e3b524ba5d390992e6f31e7/packages/kit/src/runtime/client/client.js

This is probably naive, but it seems like what might be required to fix this would somehow be to save the result of the load function associated with each history item, and immediately render the previous page using that result. Then, after we had initialized the page with the previous result of that load, then we would be able to re-run the load functions, for that route, and then update the page with the new content.

I'm not sure if this is possible to detect, but ideally we would only want to enable this behaviour when swiping to go back? When we're navigating back using the back button, then we'd want to wait to show the previous page until the previous page's load functions had finished running, to prevent any other sort of flickering.

Maybe this is just a necessary evil of client-side routing and dealing with Safari's native navigation gestures, but thought worth raising anyways in case it's solvable.

quentin-fox commented 1 year ago

Another stupider solution would be just to allow the before_navigate hook to allow specific pages to rely on the native_navigation functionality instead of using in-app routing? Hacking that together seems to have the desired effect, at least visually:

https://github.com/sveltejs/kit/assets/5247826/56553919-604c-4775-8810-7e37c1b679c6

(the numbers flickering is just because the Math.random call in this example returns a different result when running on the server vs. the browser - in a real environment, the client-side load would essentially be synchronous since any asynchronous work would be cached and inlined into the fetch function, so that flicker wouldn't be present).

quentin-fox commented 1 year ago

This is also probably linked to #9822 - being able to just use the page that is already loaded in the browser's cache, and potentially re-running the load function after the page is restored from memory may be a good solution?

205g0 commented 1 year ago

Thanks for pointing to this and sharing all these nice videos. I have this problem for ages and just disabled back which is terrible but works for some cases. So, would be amazing if this could be addressed somehow since iOS devices are not a minority...

benmccann commented 1 year ago

Can you check if this is fixed in @sveltejs/kit@1.25.2?

quentin-fox commented 1 year ago

Still an issue it seems, just recreated the behaviour above again after upgrading from @svejtejs/kit@1.20.4 to @sveltejs/kit@1.25.2

https://github.com/sveltejs/kit/assets/5247826/31a76a24-9573-4362-86f8-70f7c21af6f7

Joonel commented 11 months ago

Hey, the bug makes UX bad for iOS & iPadOS users.

jasperkelder commented 7 months ago

Nuxt and Next seem to be having similar issues: https://github.com/nuxt/nuxt/issues/6101 / https://github.com/vercel/next.js/issues/57243

I implemented the following workaround for my own app, it triggers SvelteKit's navigation when swiping left or right. Though swiping from the far end of the screen does still trigger the browser's navigation, unfortunately, with the old page flicker.

<script>
    let navigating = false;
    let swipeStart = 0;
</script>

<svelte:window
    on:touchstart={(e) => {
        navigating = false;
        swipeStart = e.touches[0].clientX;
    }}
    on:touchmove={(e) => {
        if (navigating) return;

        const swipePosition = e.touches[0].clientX;
        const offset = swipePosition - swipeStart;

        if (offset > 100) {
            navigating = true;
            window.history.back();
        }
        if (offset < -100) {
            navigating = true;
            window.history.forward();
        }
    }}
/>
s1mpLyy commented 1 week ago

same issue here, i used backward/forward caching ( keepalive ) still the same issue happening