refactor: Fetcher for tanstack start - #787
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
🧱 Refactor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5a9eb799c
ℹ️ 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".
| import { | ||
| fetcher as serverFetcher, | ||
| rawFetcher as serverRawFetcher, | ||
| } from "./server"; |
There was a problem hiding this comment.
When this universal entry is compiled for the browser, the transformed .server(...) binding becomes unused, but packages/vitnode/package.json declares **/server.js as side-effectful, so bundlers must retain this new static ./server import. That keeps @tanstack/react-start/server-only, dotenv, and the request-only APIs in the client dependency graph and can fail any browser build importing the universal fetcher. The added test hides the problem by deleting the import itself with a regex rather than exercising the actual Vite client build; move the implementation to a tree-shakeable module or exempt this entry from the side-effect pattern.
AGENTS.md reference: AGENTS.md:L35-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 101d42e5d8
ℹ️ 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".
| title?: React.ReactNode; | ||
| }) => ( | ||
| <section | ||
| className={cn("bg-card rounded-lg border p-4", className)} |
There was a problem hiding this comment.
The new section overrides its background with bg-card but continues inheriting the surrounding page text color. In themes where --card-foreground differs from the page foreground, headings and field content can lose the intended contrast; add the paired text-card-foreground token here and to the other newly card-backed controls.
AGENTS.md reference: AGENTS.md:L63-L65
Useful? React with 👍 / 👎.
| }: React.ComponentProps<"div">) => ( | ||
| <div | ||
| className={cn( | ||
| "grid grid-cols-1 items-start gap-6 lg:grid-cols-[minmax(0,1fr)_20rem]", |
There was a problem hiding this comment.
This new layout hard-codes a 20rem sidebar through an arbitrary Tailwind value, bypassing the repository's required spacing scale. Define a named grid utility or use scale-backed width/grid classes so the form remains aligned with the shared design system.
AGENTS.md reference: AGENTS.md:L64-L66
Useful? React with 👍 / 👎.
| {...props} | ||
| > | ||
| {title ? ( | ||
| <div className="mb-4 flex flex-col gap-1"> |
There was a problem hiding this comment.
Whenever a section has a title, this element combines mb-4 with gap-1, contrary to the repository's spacing invariant. Move the section-to-body separation to a parent gap-4 layout or a wrapper so one element is not using both margin and gap.
AGENTS.md reference: AGENTS.md:L67-L67
Useful? React with 👍 / 👎.
| > | ||
| {title ? ( | ||
| <div className="mb-4 flex flex-col gap-1"> | ||
| <h2 className="text-base leading-none font-semibold">{title}</h2> |
There was a problem hiding this comment.
Long or translated section titles are rendered without text-balance or text-pretty, so they can wrap with an isolated final word in narrow dialog and mobile layouts. Add the required title-wrapping utility to this heading.
AGENTS.md reference: AGENTS.md:L70-L71
Useful? React with 👍 / 👎.
1 participant
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?