-
Notifications
You must be signed in to change notification settings - Fork 0
tanstack start rsbuild #1
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
tanstack start rsbuild #1
Conversation
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0ade15776
ℹ️ 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".
| if (!('options' in value)) { | ||
| return undefined | ||
| } | ||
| return (value as { options?: Record<string, unknown> }).options |
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.
Require redirect marker before serializing
options payloads
getRedirectOptions currently returns any object's options field, and handleServerAction uses that helper for every server-function result; this means a normal return value like { options: {...} } is converted into an isSerializedRedirect response even when no redirect was thrown. In production this will mis-handle valid business payloads and force the client down redirect error handling instead of returning data.
Useful? React with 👍 / 👎.
| const hasLocation = | ||
| typeof candidate.to === 'string' || typeof candidate.href === 'string' | ||
| if (statusCode === undefined || !hasLocation) { |
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.
Stop inferring redirects from generic
code/to objects
parseRedirectFallback treats any payload with a numeric code/statusCode plus string to/href as a redirect, so ordinary JSON results (e.g. domain objects containing those common fields) are thrown as navigation redirects. This broad heuristic introduces false positives for successful server-function responses that previously returned data normally.
Useful? React with 👍 / 👎.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Hi! I'm the It looks like you correctly set up a CI job that uses the autofix.ci GitHub Action, but the autofix.ci GitHub App has not been installed for this repository. This means that autofix.ci unfortunately does not have the permissions to fix this pull request. If you are the repository owner, please install the app and then restart the CI workflow! 😃 |
Implement full Rsbuild support for TanStack Start by porting the Vite plugin, resolving build breaks and enabling Rsbuild e2e parity.
This PR introduces a dedicated Rsbuild plugin architecture to correctly handle configuration, manifest generation, server-side functions, and client-side routing. This addresses the
TS2451duplicate variable declaration error instart-manifest-plugin.tsand fixes the React Rsbuild SPA e2e failures by ensuring proper client-only route rendering, thus achieving functional parity with Vite.