Skip to content

fix(react-router): ResolvedSuspense to always use React.Suspense never the SafeFragment#1822

Merged
schiller-manuel merged 22 commits intoTanStack:mainfrom
mta-trackunit:fix-rerender-issue
Aug 9, 2024
Merged

fix(react-router): ResolvedSuspense to always use React.Suspense never the SafeFragment#1822
schiller-manuel merged 22 commits intoTanStack:mainfrom
mta-trackunit:fix-rerender-issue

Conversation

@mta-trackunit
Copy link
Copy Markdown
Contributor

If you create any hook that relies on a tanstack router hook - it becomes hard to test since the rerender recreates the elements again, I traced it down to this part in matches.tsx
const ResolvedSuspense = !router.state.matches.length ? React.Suspense : SafeFragment
It makes state changes with only the React.Suspense it works, dont really know if its intended or not? if so please explain how we could get around it for testing.

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented Jun 25, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 8164183. 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


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Comment thread packages/react-router/tests/link.test.tsx Outdated
Comment thread packages/react-router/tests/link.test.tsx
@SeanCassiere SeanCassiere changed the title Added test to prove rerender test for hooks is not possible right now fix(react-router): ResolvedSuspense to always use React.Suspense never the SafeFragment Jun 25, 2024
@SeanCassiere

This comment was marked as outdated.

@mta-trackunit

This comment was marked as outdated.

… would end up redirecting back to dashboard/invoices if using this.state.resolvedLocation
Comment thread packages/react-router/src/router.ts Outdated
@mta-trackunit

This comment was marked as outdated.

@mta-trackunit

This comment was marked as outdated.

Comment thread examples/react/kitchen-sink-file-based/src/routes/dashboard.users.tsx Outdated
@SeanCassiere

This comment was marked as resolved.

@SeanCassiere

This comment was marked as resolved.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jul 17, 2024

commit: 8164183

@tanstack/history

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

@tanstack/react-cross-context

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

@tanstack/react-router

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

@tanstack/react-router-with-query

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

@tanstack/router-cli

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

@tanstack/router-devtools

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-vite-plugin

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

@tanstack/start

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

@tanstack/start-vite-plugin

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

Open in Stackblitz

More templates

@mta-trackunit
Copy link
Copy Markdown
Contributor Author

@SeanCassiere how come you removed the
history: createMemoryHistory({ initialEntries: ['/'] }),

@mta-trackunit
Copy link
Copy Markdown
Contributor Author

@SeanCassiere I believe this is ready for release now?

@SeanCassiere
Copy link
Copy Markdown
Member

@SeanCassiere how come you removed the history: createMemoryHistory({ initialEntries: ['/'] }),

Mostly so we emulate the other tests and use the browser history.

Copy link
Copy Markdown
Member

@SeanCassiere SeanCassiere left a comment

Choose a reason for hiding this comment

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

As mentioned in this discord thread, I've tested this locally with the basic, basic-file-based, kitchen-sink-file-based, and start-basic examples, and I have observed no visible regressions.

@schiller-manuel @tannerlinsley anything blocking this from your side?

@mta-trackunit
Copy link
Copy Markdown
Contributor Author

As mentioned in this discord thread, I've tested this locally with the basic, basic-file-based, kitchen-sink-file-based, and start-basic examples, and I have observed no visible regressions.

@schiller-manuel @tannerlinsley anything blocking this from your side?

you are linking to a private channel - so I cannot follow along :) - any update?

@schiller-manuel schiller-manuel merged commit ca401e6 into TanStack:main Aug 9, 2024
SeanCassiere added a commit that referenced this pull request Aug 10, 2024
SeanCassiere added a commit that referenced this pull request Aug 12, 2024
…erToReadableStream` (#2117)

* using the a TextEncoder

* trying without the changes from #1822

* revert that last commit

* use TextDecoder on the client-side of the stream

* undo

* temporary disable

* re-enable

* style: eslint warnings

* log error events

* gitignore wrangler outputs
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.

3 participants