Root route is rendered before loaders are finished

TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.

https://tanstack.com/router

MIT License

7.84k stars 579 forks source link

Root route is rendered before loaders are finished #1705

Open cristi-lng opened 3 months ago

cristi-lng commented 3 months ago

Describe the bug

It seems that the root route component is rendered before all the loaders finished their work.

If I understand correctly no component should be rendered until all loaders are finished. Is this still true?

When I test with a default pending component the behaviour seems to be correct. So, the problem is only when there is no pending component.

Your Example Website or App

https://stackblitz.com/edit/tanstack-router-vkppeu?file=src%2Fmain.tsx

Steps to Reproduce the Bug or Issue

Check the demo from the link
See how the page is loaded and only the root component is displayed before the loaders finish
You can toggle the defaultPendingComponent to test in pending mode

Expected behavior

If no pending is used, I am expecting to see everything rendered after the loaders are finished.

Screenshots or Videos

No response

Platform

OS: macOS
Browser: Chrome
@tanstack/react-router: 1.34.9

Additional context

No response

TkDodo commented 3 months ago

If I understand correctly no component should be rendered until all loaders are finished. Is this still true?

a route will render as soon as its loaders are finished. So the root route will render as soon as the loaders of the root route are done. It will not wait for loaders of child routes.

there is no aggregation of loaders

cristi-lng commented 3 months ago

Ok, but if this is the case why isn't the layout route from my example rendered as well? Because it doesn't have any loaders.

Also, if for example, the loader from a child route finishes before the one from the parent I still suspect that the child won't be rendered until the parent loader is finished.

I believe that the behaviour changed somehow because if we run with the old version of the router (1.22.9) it waits for all the loaders before rendering.

tannerlinsley commented 2 months ago

This is mainly because of our homemade transition framework that "holds" state updates to the rendered state until everything is ready. Sounds like the root route is escaping that framework. I'll work on this now.

tannerlinsley commented 2 months ago

More information here that needs to be considered, specifically for client-side SPAs:

Currently:

The root route is only rendered immediately on the first paint of SPA (non-SSR) routers. This was decided because it's usually better to see something instead of a blank white/black screen while you wait.

Considerations

If we hold all painting on first render, then we force this blank state on everyone. Not cool
If you'd like to opt-in to this, how could we do that? Automatically? Or manually?

tannerlinsley commented 2 months ago

Relevant code: https://github.com/TanStack/router/blob/37c28cadc5ff43f6d8fdec5e2256e5391b0b1366/packages/react-router/src/router.ts#L1704-L1706

cristi-lng commented 2 months ago

@tannerlinsley Thanks for the detailed answer.

More info about our approach:

I agree that is not ok to display an empty page while loading the data
- This is why we display a placeholder directly in the index.html even before the js is loaded

This is the way I thought that the route segments will load:

If there is no pending component, all the loaders will be awaited and after that the components will be rendered
If there is a pending component at a route segment, the router will wait only for the parent loaders and then display the parent components and the pending component from the current segment
- And then it will go forward until the next segment that has a pending component

Wouldn't this behaviour allow enough flexibility? To choose when to wait for everything or display some pieces earlly? In this case, it wouldn't be limited only to the root route.

Nils-Kolvenbach commented 2 months ago

@tannerlinsley Thanks for the information on this.

I am currently experiencing the same while developing an SPA. My two cents:

I like the idea of beforeLoad to do exactly what it suggests: do all of this before rendering the route - this way I can be sure my component has what it needs
Changing the behavior of beforeLoad depending on where it's used (root vs sub-route / SPA vs SSR) could be confusing and the type of context properties inside the root route would need to change, since they could be not loaded yet
If I would like the user to see something before beforeLoad finished a possible long running request within the root route, I would not use beforeLoad but handle the request on a deeper component level with it's own loading animation. This way the rest of the page is not blocked from rendering

Thanks for all the work on these amazing packages!

cristi-lng commented 2 months ago

@Nils-Kolvenbach In this ticket I was specifically referring to the loader and not the beforeLoad.

In our tests the beforeLoad works as expected. At least in SPA mode. It waits until everything is ready an works in serial fashion.

Do you see something different here? If so, do you have an example?

Nils-Kolvenbach commented 1 month ago

Ah, thanks for clarification! After an update to the latest version (1.45.2 -> 1.49.1) and the corresponding router plugin and devtools everything works as expected for me.

cristi-lng commented 4 weeks ago

I have updated the example app to the latest version of the router (1.51.0) and the issue is still there.

https://stackblitz.com/edit/tanstack-router-vkppeu?file=src%2Fmain.tsx