Using Suspense + Await to resolve deferred loader data breaks pending UI when navigating within the same route

nramovs-sr commented 3 months ago

Reproduction

Go to https://stackblitz.com/edit/remix-run-remix-hj7xu8?file=app%2Froutes%2F_common.tsx Click (server:) "one" Wait for deferred content to load (3 seconds) Click (server:) "two" Within the next 3 seconds (before deferred content loader) click (server:) "three"

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @remix-run/dev: * => 2.9.2 
    @remix-run/node: * => 2.9.2 
    @remix-run/react: * => 2.9.2 
    @remix-run/serve: * => 2.9.2 
    vite: ^5.1.0 => 5.3.1

Used Package Manager

npm

Expected Behavior

After clicking "two" page should update after 200ms After clicking "three", NavLink for "two" should render it's unselected style, and NavLink for "three" should transition to "pending" state.

Actual Behavior

After clicking "two" page doesn't update until deferred value resolves After clicking "three" NavLink stays in pending state for loading "two"

Repeating similar procedure for (client:) navigation, using clientLoader has the same behavior. Without any delay in clientLoader we can break pendingUI even faster: Click (client:) "one" Wait for deferred content to load (3 seconds) Click (client:) "block" -> Now NavLink doesn't transition to pending state, it goes directly to showing "block" as active, but after the 3 second delay to resolve the deferred value.

brophdawg11 commented 3 months ago

Can you try putting a key on the Suspense boundary to make it specific to the location? I think I've seen this behavior before when React re-uses the same component tree across navigations between the same route with different params - it reuse's the Suspense boundary (due to lack of a key), doesn't re-fallback, and because it's in a startTransition under the hood it continues showing the old UI

<Suspense fallback="... loading ..." key={location.key}>

misha-erm commented 1 month ago

Faced the same issue during migration from react-router v6 to remix. Can confirm that using <Suspense key={location.key}> worked for me

@brophdawg11 wdyt is it something that can be fixed within remix/react-router v7 or is it more of an expected behavior?

brophdawg11 commented 1 month ago

Glad that fixes it! This is expected React behavior - where it attempts to re-use existing fallbacks and you can force a new Suspense fallback via key

misha-erm commented 1 month ago

the strangest part is that not all pages are affected by this behavior. Some pages work just fine without this key trick... Can't find what causes it ))

misha-erm commented 1 month ago

@brophdawg11 sorry for pinging still in closed issue, just found a scenario in which the trick doesn't work.

As I can see UI is still blocked for cases when code for nested route was not loaded yet. Tried to wrap Outlet with suspense and provide location's key - no success. Is it expected? This can be improved to some degree with help of "prefetch" but still wonder if there is a way to not block navigation at all

P.S. video was made running in vite's dev mode

https://github.com/user-attachments/assets/99fdba17-5619-4f92-95bf-ddcd44793e8d

brophdawg11 commented 1 month ago

I have no idea what code is powering that video :). Could you please open a new issue with a minimal reproduction?

misha-erm commented 1 month ago

not sure that's an issue, so created a discussion https://github.com/remix-run/remix/discussions/9879

remix-run / remix