-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: ssr setting per route #4565
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
View your CI Pipeline Execution ↗ for commit 7c279e5
☁️ 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: |
|
||
export function isRecoverableError(error: unknown): boolean { | ||
if (error instanceof Error) { | ||
return error.message.includes(RECOVERABLE_ERROR) |
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.
Maybe make this a more specific?
startsWith(`${RECOVERABLE_ERROR}: `)
|
||
export function tanStackStartRouter(config: Config): Array<PluginOption> { | ||
return [ | ||
tanstackRouterGenerator({ | ||
...config, | ||
plugins: [serverRoutesPlugin(), routesManifestPlugin()], | ||
environmentName: VITE_ENVIRONMENT_NAMES.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.
Rename this to to something like viteEnvironmentName
or something like viteApplyEnvironment
?
Purely since it wouldn't have any effect on the other bundlers.
|
||
const route = this.looseRoutesById[routeId]! | ||
|
||
const pendingMs = | ||
route.options.pendingMs ?? this.options.defaultPendingMs | ||
|
||
if (this.isServer) { |
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.
Could use some comments 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.
Works as I'd expect
4379017
to
b2fad94
Compare
785de2e
to
c2b3fe4
Compare
disable this in the server env
only use addHmr
… in React instead, use the same approach as in Solid also ensure that hydration is not blocked while router loads in SPA mode. keep the outermost pending component shown upon hydration to avoid flashes.
c2b3fe4
to
a0eb984
Compare
fcf9223
to
2bd41c0
Compare
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.
.
return <div className="p-2">hello world</div>; | ||
} | ||
}); | ||
if (import.meta.hot) { |
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 it necessary to have both development
and production
snapshots related testing for the "delete-nodes" functionality?
Currently, the difference between 'development' and 'production', is the inclusion of the HMR setup using import.meta.hot.accept
.
}, | ||
] | ||
|
||
const frameworks: Array<Config['target']> = ['react', 'solid'] |
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 should probably now become a shared variable between code-splitter.test.ts
and delete-nodes.test.ts
so that any additions in the future (of other frameworks) don't cause drift.
/** | ||
* @default true | ||
*/ | ||
addHmr?: boolean |
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 use-case would cause the user to set this to false
?
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.
see start-plugin-core. on the server there is no hmr added for example
@@ -138,27 +143,51 @@ export function compileCodeSplitReferenceRoute( | |||
|
|||
if ( | |||
!( | |||
path.node.callee.name === 'createRoute' || | |||
path.node.callee.name === 'createFileRoute' | |||
path.node.callee.name === 'createFileRoute' || |
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.
path.node.callee.name === 'createFileRoute' || | |
['createFileRoute', 'createRootRoute', 'createRootRouteWithContext'].includes(path.node.callee.name) |
}, | ||
) | ||
} | ||
if (createRouteFn !== 'createFileRoute') { |
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.
I think its worth adding a comment here for future adventurers stating that the code-splitting we're exiting early here since the rootRoute
is never code-split.
Perhaps the exit condition should just check for createRootRoute
or createRootRouteWithContext
.
export const Route = createFileRoute('/posts/$postId')({ | ||
validateSearch: z.object({ postId: ssrSchema }), | ||
ssr: ({ search }) => { | ||
if (typeof window !== 'undefined') { |
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.
Not sure how important this is, but we can probably share the content of the ssr
, beforeLoad
, and component
between the routes yes?
60628c3
to
7c279e5
Compare
No description provided.