-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: use seroval for SSR #4600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
also stream all queries in react-router-with-query
View your CI Pipeline Execution ↗ for commit 9220a3e
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-with-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
type: 'stream' | ||
value?: ReadableStream & { controller?: ReadableStreamDefaultController } | ||
export interface TsrSsrGlobal { | ||
r?: DehydratedRouter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it makes sense for the matches
to have keys be shortened (i.e. i
, b
, l
, etc.), I think these single callers may benefit from just being left with having descriptive names to represent their purpose (i.e. router/dehydratedRouter
, close
, whatever_v_is
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed r to router, v is gone
added a comment to c ( the cleanup function), this is called per script injection and thus benefits from a short name
|
||
export interface TsrSerializer { | ||
export interface StartSerializer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the Serializer get moved to Start? why not keep it in router-core
.
Wouldn't this same serializer benefit normal basic-file-based-ssr-streaming
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR only uses seroval for SSR
for server functions, we still use the old serializer. in a followup PR, we'll move server functions over to seroval as well
} from '@tanstack/react-query' | ||
import { isRedirect } from '@tanstack/router-core' | ||
import '@tanstack/router-core/ssr/client' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like extra import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the issue here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended to import this module like side effect without import specific functions, etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the import is necessary to get the type import. is there any problem with that import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for disturbing.
I expected to see something like:
import { type SomeType } from '@tanstack/router-core/ssr/client'
instead of import '@tanstack/router-core/ssr/client'
@schiller-manuel this breaks Clerk with Tanstack Start. The Clerk starter isn't working in the latest start version. The getAuth() function throws: https://github.com/TanStack/router/tree/main/examples/react/start-clerk-basic The reason I reach out here is that I want to point the Clerk team towards something if the fix lies no their end. |
Is there a way to configure a custom serializer? seems like firebase' Timestamp is broken
|
@AhmedBaset not yet. there will be a way to add custom seroval plugins |
Any fixes at the moment except downgrading? |
don't return anything from loader / beforeLoad that contains custom classes |
@schiller-manuel for instance "clerkInitialState" is a custom class (or maybe QueryClient)?
|
also stream all queries in react-router-with-query