feat: Add plugin routes to tanstack start - #764
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b0b3ba863
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const claimed = new Set(appPaths) | ||
| const collision = specs.find((spec) => claimed.has(spec.path)) |
There was a problem hiding this comment.
Compare route match shapes instead of parameter names
When a plugin route and an app route use different names for the same dynamic segment, this exact string comparison misses the collision. For example, the existing /api/$ splat and a plugin declaration such as /api/:slug both match /api/foo, while /api/$ and /api/$slug are treated as distinct here; similarly, an app /posts/$id and plugin /posts/:slug conflict. Normalize TanStack parameters and splats into comparable match shapes before mounting the plugin subtree, otherwise routing precedence decides which handler or page serves the URL.
Useful? React with 👍 / 👎.
| routeTree: TRouteTree, | ||
| specs: PluginRouteSpec[], | ||
| ): TRouteTree => { | ||
| if (specs.length === 0) return routeTree |
There was a problem hiding this comment.
Remove the previous subtree when the route list becomes empty
When the last plugin route is removed during development, this early return leaves the previously mounted _plugins child on the cached routeTree. The implementation explicitly supports repeated composition against that singleton, but transitioning from a non-empty list to [] therefore keeps the disabled plugin page reachable until the process is restarted. Rebuild the root children without the old plugin container even when specs is empty.
Useful? React with 👍 / 👎.
| const loaded = await import(pathToFileURL(file).href) | ||
| const declarations = routeDeclarationsFromManifest(loaded, specifier) |
There was a problem hiding this comment.
Bust the module cache when reloading a changed manifest
When a watched compiled route manifest changes, regenerate() imports this identical file URL again, so Node returns the already-cached ESM namespace instead of evaluating the new contents. Consequently edits to an existing manifest do not update either generated file despite the watcher firing, and developers must restart the Vite process. Import through a cache-busted URL or another loader that invalidates the module between regenerations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fcee194a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * the bundle by construction rather than by a filter somebody has to remember. | ||
| */ | ||
| const readConfiguredPluginIds = async (): Promise<string[]> => { | ||
| const jiti = createJiti(import.meta.url, { interopDefault: true }) |
There was a problem hiding this comment.
Disable Jiti caching for watched config reloads
When src/vitnode.config.ts changes during development, configureServer calls this loader again expecting the configured plugin list to refresh, but Jiti uses its module cache by default; creating another Jiti instance does not evict the config from the underlying module cache. Adding or removing a plugin can therefore regenerate from the previous configuration until Vite is restarted. Load watched configuration with moduleCache: false or explicitly invalidate it before importing.
Useful? React with 👍 / 👎.
| component: lazyRouteComponent(async () => | ||
| assertPluginRouteModule(await spec.load(), spec.route.id), | ||
| ), |
There was a problem hiding this comment.
Expose matched params to dynamic plugin pages
When a plugin declares a supported dynamic path such as /blog/:slug, this registration hands TanStack only the loaded zero-prop component. TanStack route components do not receive match parameters as props, and the repository adds no framework-neutral adapter or context, so the documented framework-neutral page cannot determine which slug matched without importing TanStack-specific hooks. Dynamic plugin pages therefore cannot render or SSR the requested resource; expose normalized parameters through the plugin route contract or reject dynamic declarations until that contract exists.
Useful? React with 👍 / 👎.
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?