Skip to content

fix(react-router): do not wrap children in outlet in suspense unless (default)pendingComponent is configured#2035

Closed
schiller-manuel wants to merge 2 commits into
mainfrom
fix-root-pendingComponent-Outlet
Closed

fix(react-router): do not wrap children in outlet in suspense unless (default)pendingComponent is configured#2035
schiller-manuel wants to merge 2 commits into
mainfrom
fix-root-pendingComponent-Outlet

Conversation

@schiller-manuel

Copy link
Copy Markdown
Collaborator

No description provided.

@nx-cloud

nx-cloud Bot commented Jul 26, 2024

Copy link
Copy Markdown
Contributor

☁️ Nx Cloud Report

CI is running/has finished running commands for commit bc81168. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx affected --targets=test:eslint,test:unit,test:e2e,test:types,test:build,build --parallel=3
✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@schiller-manuel schiller-manuel force-pushed the fix-root-pendingComponent-Outlet branch from 1f62d54 to 16a42fd Compare July 26, 2024 21:18
@pkg-pr-new

pkg-pr-new Bot commented Jul 26, 2024

Copy link
Copy Markdown

commit: bc81168

@tanstack/history

pnpm add https://pkg.pr.new/@tanstack/history@2035

@tanstack/react-cross-context

pnpm add https://pkg.pr.new/@tanstack/react-cross-context@2035

@tanstack/react-router

pnpm add https://pkg.pr.new/@tanstack/react-router@2035

@tanstack/react-router-with-query

pnpm add https://pkg.pr.new/@tanstack/react-router-with-query@2035

@tanstack/router-arktype-adapter

pnpm add https://pkg.pr.new/@tanstack/router-arktype-adapter@2035

@tanstack/router-cli

pnpm add https://pkg.pr.new/@tanstack/router-cli@2035

@tanstack/router-devtools

pnpm add https://pkg.pr.new/@tanstack/router-devtools@2035

@tanstack/router-generator

pnpm add https://pkg.pr.new/@tanstack/router-generator@2035

@tanstack/router-plugin

pnpm add https://pkg.pr.new/@tanstack/router-plugin@2035

@tanstack/router-valibot-adapter

pnpm add https://pkg.pr.new/@tanstack/router-valibot-adapter@2035

@tanstack/router-vite-plugin

pnpm add https://pkg.pr.new/@tanstack/router-vite-plugin@2035

@tanstack/router-zod-adapter

pnpm add https://pkg.pr.new/@tanstack/router-zod-adapter@2035

@tanstack/start

pnpm add https://pkg.pr.new/@tanstack/start@2035

@tanstack/start-vite-plugin

pnpm add https://pkg.pr.new/@tanstack/start-vite-plugin@2035

Open in Stackblitz

More templates

@SeanCassiere

This comment was marked as resolved.

@SeanCassiere SeanCassiere changed the title fix: prefer pendingComponent over defaultPendingComponent when wrapping Outlet in Suspense fix(react-router): prefer pendingComponent over defaultPendingComponent when wrapping Outlet in Suspense Jul 27, 2024
@schiller-manuel

Copy link
Copy Markdown
Collaborator Author

it maybe solves #2026

@SeanCassiere

Copy link
Copy Markdown
Member

it maybe solves #2026

Whoops, my bad, linked the wrong issue.

@schiller-manuel

Copy link
Copy Markdown
Collaborator Author

@tannerlinsley please have a look if omitting this suspense is ok, check #2026 for the motivation.

one thing I dislike is that there is no way to opt out of suspense here if defaultPendingComponent is set.

@schiller-manuel schiller-manuel changed the title fix(react-router): prefer pendingComponent over defaultPendingComponent when wrapping Outlet in Suspense fix(react-router): do not wrap children in outlet in suspense unless (default)pendingComponent is configured Jul 27, 2024

@SeanCassiere SeanCassiere left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing I dislike is that there is no way to opt out of suspense here if defaultPendingComponent is set.

This could be changed so that we don't use the defaultPendingComponent for the rootRoute and only rely on the rootRoute.pendingComponent. So if PendingComponent===undefined then just return nextMatch.

The only issue here is that this could be a breaking change for anyone currently relying on that default behaviour of not requiring a Suspense boundary above the RouterProvider.
We can ofc, get around this by introducing a future flag and stuff this new behaviour under that flag (and remove the old behaviour in v2).

@lukasgeiter

Copy link
Copy Markdown

Is there anything I can do to move this along?
Maybe we could merge the first commit which I would consider less of a breaking change?

@schiller-manuel schiller-manuel force-pushed the fix-root-pendingComponent-Outlet branch from 3518ac3 to bc81168 Compare August 23, 2024 21:52
@lukasgeiter

Copy link
Copy Markdown

@SeanCassiere @schiller-manuel @tannerlinsley Is there any chance of getting at least the first commit merged?
That would allow me to work around this bug without having to patch the package itself.

@dy0gu

dy0gu commented Apr 30, 2025

Copy link
Copy Markdown

Any updates on getting this merged? We would really appreciate it, some workarounds are getting messy!

@LadyBluenotes

Copy link
Copy Markdown
Member

Closing this since the issue was addressed in #5495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants