-
-
Notifications
You must be signed in to change notification settings - Fork 1k
fix react-basic-ssr-streaming-file-based example #4429
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
base: main
Are you sure you want to change the base?
fix react-basic-ssr-streaming-file-based example #4429
Conversation
thanks! can you please rename package.disabled.json back to package.json? |
View your CI Pipeline Execution ↗ for commit 1a45da5.
☁️ Nx Cloud last updated this comment at |
Will do. Was wondering why it was named as such but didn't want to change it. |
…ample' into fix-react-basic-ssr-streaming-example
the name was just a way to disable it from building (and failing) in CI |
needs updating the lockfile as well |
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: |
I see it fails a the build test if '@tanstack/react-start' is not added to the ssr external array in vite for this example. I've added it to the PR |
please don't merge this yet, we need to still figure out how to handle the ssr noExternal setting properly |
Trying this out locally, whilst this does work in |
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.
Blocking this till we figure out both Manuel's comment and mine.
…t-basic-ssr-streaming-example
…ample' into fix-react-basic-ssr-streaming-example
Thanks for looking at this. I've made a couple of updates to resolve the ssr build actions for both server and client builds as well as serving in production. When building this outside of the mono-repo this builds perfectly without the external clause and runs fine in prod and dev modes. However as soon as we try and build it in the mono repo the below appears: Honestly not too sure what might be the cause other than its clearly not able to resolve the virtual import in the mono-repo. If you have any ideas where to start looking I would be more than happy to continue scratching around and resolving it. |
Scratched a bit further. From what I can see the dependencies flow from start-server-core => react-start-server => react-start/server. The virtual modules is part of start-server-core but does not get imported/used in the functions needed for the router ssr (non-start) functionality. However, this is included in the barrel exports and also marked as external dependencies in start-server-core. My guess is this is where it falls over when building in the mono-repo but passes when building for standalone. By specifically marking the VIRTUAL_MODULES as external the build passes in both the mono-repo and standalone. And since this is not used (from what I can see) for SSR to function in a pure SPA router app this should not be a problem. Also ran this through the test suite and it passes all tests. Ideally this should not be making its way into the build process at all but I'm not familiar enough with nx mono-repos to understand how to exclude this specifically in the build and other than stripping it out into either separate exports or alternative package this seems to be the path of least resistance. Open to any suggestions you might have. |
The react basic-ssr-streaming-file-based example is a bit outdated and no longer working.
This fix updates the server.js and entry-server.tsx to match those of the basic-ssr-file-based example as these were using the new createRequestHandler and defaultStreamHandler.
This also addresses an issue with the index route where the deferred value is causing an error when accessing the getDate() function. The value when streamed in is not being transformed back into a date. I assume this will be addressed with the serializer re-write/seroval but until then this at least allows the example to run.