Outlet in root route prevents suspense from bubbling up

[lukasgeiter]

Describe the bug

I would like to use my own loading component at the root level. To do that I'm wrapping the <Outlet> in the root component with a <Suspense> block. Unfortunately, Outlet wraps the children in a <Suspense> block of its own, preventing mine from being used. Since I have no defaultPendingComponent configured, it renders nothing.

I have traced the issue to this part of the code:

router/packages/react-router/src/Match.tsx

Lines 294 to 302 in f7c74bc

	const pendingElement = router.options.defaultPendingComponent ? (
	<router.options.defaultPendingComponent />
	) : null
	if (matchId === rootRouteId) {
	return (
	<React.Suspense fallback={pendingElement}>{nextMatch}</React.Suspense>
	)
	}

Your Example Website or App

https://stackblitz.com/edit/github-s38jue?file=src%2Froutes%2F__root.tsx

Steps to Reproduce the Bug or Issue

1. Run the Stackblitz

Expected behavior

As a user, I expect to see the fallback content Suspense fallback - I expect this to show up, but I'm seeing nothing (besides the navigation)

Screenshots or Videos

No response

Platform

• OS: macOS
• Browser: Arc (Chrome)
• Version: 1.52.0 (51895)

Additional context

Related issue: #1049

[schiller-manuel]

does #2035 solve your issue?

[lukasgeiter]

@schiller-manuel that would certainly be an improvement 👍

Although, I'm still of the opinion that if no pending components are configured, the router shouldn't wrap anything in a suspense at all. I don't think rendering a suspense with null as fallback is a good default.

[schiller-manuel]

like this? 3518ac3

I am not sure which implications this has, so I need reviews.

[lukasgeiter]

Yeah, that's what I had in mind. Not sure if the failing tests mean that this is a bad idea, or if they should be adjusted 😅

[TkDodo]

Although, I'm still of the opinion that if no pending components are configured, the router shouldn't wrap anything in a suspense at all

I’m not sure about that. Usually, we try to determine if wrapping in a suspense boundary is necessary, by checking for the presence of a loader or beforeLoad. This can be force-overwritten with wrapInSuspense: true.

I’m much more inclined to think that, in this case, we shouldn’t wrap in a Suspense Boundary because the index route doesn’t have a loader or a beforeLoad. It shouldn’t be tied to the presence of a pendingComponent.

Also interesting that setting wrapInSuspense: false doesn’t work here. I also think this should overwrite our Suspense wrapper 🤔