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 |
…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. |
|
@schiller-manuel @SeanCassiere I was able to avoid the externalising of the VIRTUAL_MODULES by bubbling up an additional export for the core ssr functions from start-server-core all the way through to react-start. This is in addition to current exports and does not alter those at all. If this method is acceptable then I'm wondering if this should not be split into separate PRs probably since this is now more than just fixing an example. Alternatively, these functions can possible be split into a separate package that can be re-exported by start-server-core and hence not affect the current packages but can be used by a pure router only project for SSR purposes. Again, open to your thoughts on this |
|
interesting! why does the separate export solve this? |
|
My understanding is it has to do with how the bundler traverses the barrel exports coming from the dependencies. (see blog post written by TKDodo: Please Stop Using Barrel Files) When running standalone (not part of the router mono-repo) these packages (and therefore the virtual modules) are included in the NODE_MODULES and by default don't get traversed. When part of the mono-repo it seems it is traversing the packages, and it then hits the VIRTUAL_MODULES which it can't find and the error occurs. Since the examples is part of the mono-repo the dependencies are resolved from the mono-repo and not from node_modules. As per vite's documentation: "Vite automatically detects dependencies that are not resolved from node_modules and treats the linked dep as source code." Monorepos and Linked Dependencies. This is not a problem in pure start examples since that uses the start plugin that initializes the VIRTUAL_MODULES but for pure router examples these don't get initialized and it breaks the build. When doing a separate export for only the essential modules that is required for SSR, the loadVirtualModules.js file is not traversed as it doesn't form part of the export and hence the problem is avoided. |
|
The real problem here seems to be that we are using the start package to do the SSR for the router project. It blurs the already grey line separating start from router even more. I assume you would not be particularly keen on introducing a new package, but the cleanest and most modular solution is probably to move these SSR essentials to a separate package (for example router-ssr). It could even be moved to router-core but this would muddy the responsibilities that router-core has. Start-server-core could depend on and re-export as it does currently for any project/package that depends on it while router projects would then depend on this "router-ssr" project and would not need to depend on start and hence the import tree is kept clean from start only functionality. |
|
i like the idea of having separate packages. we just need to find a clean separation. For example, where should the serialization be put? |
|
The broad idea would be to separate anything that is generic ssr into this package. This is probably a huge over simplification, but I will work on a proposal tonight and send it through. |
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.