Repository navigation
fix(pages): exclude server filenames from client bundle detection - #559
Merged
Brooooooklyn merged 3 commits intoOct 26, 2026
Merged
Brooooooklyn merged 3 commits into
Brooooooklyn merged 3 commits into
Conversation
yangshun
marked this pull request as draft
October 23, 2026 05:31
yangshun
marked this pull request as ready for review
October 23, 2026 12:02
Brooooooklyn
self-requested a review
October 26, 2026 04:14
Brooooooklyn
force-pushed
the
codex/fix-island-only-client-bundles
branch
from
October 26, 2026 04:44
5b4b2ff to
82d2bab
Compare
Member
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The check skipped any name containing `.server.`, so a page such as `status.server.v2.tsx` lost its pages-client bundle in an island-only app, while Void's page scanner still serves it as a page. Match the scanner: only `<name>.server.tsx` and `<name>.server.jsx` are server companions. Drop the islands plugin checks from the new test: they pass with or without this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brooooooklyn
force-pushed
the
codex/fix-island-only-client-bundles
branch
from
October 26, 2026 04:44
82d2bab to
3bd6d0b
Compare
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Brooooooklyn
approved these changes
Oct 26, 2026
Brooooooklyn
added a commit
that referenced
this pull request
Oct 26, 2026
…561) ## Summary This adds support for `.tsx` and `.jsx` in HTTP/WebSocket route files and recognizes `.server.tsx`/`.server.jsx` page companions. It also fixes existing `.server.tsx` companions being discovered as standalone pages such as `/foo.server` and included in generated client component manifests. The extension enables JSX syntax; the file's location and naming determine its role. This PR ended up larger than I imagined, mainly because there are many cases to handle. Half of the changes are tests and docs updates. There's also a fair bit of consolidation going on. P.S. TBH I didn't start out wanting to support JSX for WebSockets, but the route conventions + extensions logic was quite entangled and it was hard to untangle them. Adding support for JSX WebSockets was the easiest way forward while not making the code messy. One use case I convinced myself to add support for JSX in WebSockets is chat apps that sends rendered HTML over WebSockets (hopefully that's not too much of a stretch to imagine!). > [!WARNING] > This PR updates the docs with a new feature, and I think docs are published at merge time, which can result in unreleased features appearing in the docs. ## Motivations It's only a matter of time before someone requests for this, given the precedents for JSX-capable HTTP route files. - **[Vercel OG image generation](https://vercel.com/docs/og-image-generation):** the official examples use `api/og.tsx` and `api/og.jsx`, as well as Next.js `app/api/og/route.tsx`, to generate images from JSX inside HTTP handlers. - **[TanStack Start server routes](https://tanstack.com/start/latest/docs/framework/react/guide/server-routes):** the documentation defines HTTP `server.handlers` in `routes/hello.tsx`, alongside a JSX page component. Christoph also mentioned that he [prefers `.tsx`](https://github.com/nkzw-tech/server-template#why-does-this-template-use-tsx-as-a-file-extension). ## Scope | File | Behavior | | --- | --- | | `routes/api.tsx` / `routes/api.jsx` | HTTP handlers at `/api` | | `routes/api.dev.tsx` / `routes/api.prod.jsx` | HTTP handlers restricted to development / production | | `routes/chat.ws.tsx` / `routes/chat.ws.jsx` | WebSocket handlers at `/chat` | | `routes/chat.dev.ws.tsx` / `routes/chat.prod.ws.jsx` | WebSocket handlers restricted to development / production | | `pages/foo.server.tsx` / `pages/foo.server.jsx` | Loaders, actions, and page configuration for the matching page, excluded from standalone page discovery and generated client component manifests | | `pages/foo.tsx` / `pages/foo.jsx` | Existing page component support; no new component formats | Both JSX extensions support `.dev` and `.prod` restrictions. For WebSocket routes, the environment suffix goes before `.ws`. HTTP and WebSocket discovery, filename parsing, page companion lookup, and route binding inference share `ROUTE_EXTENSIONS` (`ts`, `tsx`, `js`, `jsx`, `mjs`, `mts`) from `shared/route-extensions.ts`. The page scanner uses the derived `ROUTE_EXTENSIONS_SET` for membership checks. Both route scanners use the fixed `listRouteFiles()` helper and the existing WebSocket filename matcher to distinguish their roles. `listConventionFiles()` retains its existing script extensions for crons and queues. Discovery and binding inference share the `.well-known` glob policy, including configured source directories. Environment filtering applies only to route files, so `_`-prefixed helpers retain their bindings in both environments. Companion exclusion applies only to supported script extensions, preserving page components such as `web.server.md`. WebSocket resource-name inference strips JSX extensions so changing `chat.ws.ts` to `chat.ws.tsx` or `chat.ws.jsx` preserves the inferred Durable Object identity. Void sets no JSX options: route JSX compiles with the app's own JSX settings (framework plugins, tsconfig for TS files, or `oxc.jsx`). The docs show Hono's JSX via `oxc.jsx` for apps without a JSX framework. The documentation covers supported HTTP, WebSocket, and companion extensions. General enforcement against manually importing `.server.*` modules from client code remains a separate feature. ## Validation - `vp run check`: all 33 workspaces passed. - `vp run test`: 11,214 passed, 31 skipped. - `vp test run --maxWorkers=4`: 14,594 passed, 31 skipped. - Formatting and lint passed. Regression coverage executes JSX HTTP handlers and WebSocket messages using React and Hono runtimes, checks environment filtering and generated types/bindings (including `.well-known` routes and underscored helpers), and executes companion loaders/actions. It also verifies that Vue, Svelte, and Markdown pages with a `.server` suffix remain discoverable. Production-build assertions confirm companion code is present on the server and absent from the generated client graph and assets. ## Follow-up [#559](#559) (merged) excludes server companions from React and Solid's client bundle detection, so island-only apps do not emit the full `pages-client` entry. Until this PR lands, `main` still lists a companion such as `index.server.tsx` as its own page (`/index.server`). ## Review and polish (rebased on `main`) Rebased onto `main` (the merge commit is dropped; the four commits are unchanged). A deep review found the code correct; one docs commit was added: ``` "uses your app's configured JSX runtime" → Void sets no JSX runtime React / Solid app framework JSX → render with its server renderer API only / Vue / Svelte no runtime (Vue's jsx: preserve leaves JSX raw → build fails) → docs now show oxc.jsx with hono/jsx (needs hono in the app's deps) ``` Checked in real apps with that setup, `.tsx` and `.jsx` routes, dev and `vp preview`: | App | `/hello` (.tsx) | `/hello2` (.jsx) | existing pages | |---|---|---|---| | Vue pages | 200 | 200 | 200 | | Svelte pages | 200 | 200 | 200 | | API only (with `hono` dep) | 200 | 200 | 200 | Also in that commit: the WebSocket and companion docs link to the JSX section; the companion table drops the `routes/` row and the repeated pair; `reference/structure.md` lists companion extensions; `websockets.md` says `chat.ws.dev.ts` is not a WebSocket route (it is served as an HTTP route at `/chat.ws`). Mutation checks: reverting `ROUTE_EXTENSIONS` to the old list fails 93 tests; removing the `.server` page skip fails 20. Affected tests (`vp run test --base origin/main`, 518 files) pass except one unrelated platform browser test that times out at 5s under full load (passes alone); that is fixed separately. ### Left for the maintainer (product calls) - Docs go live at merge, before the feature ships (as noted above). - `.tsx`/`.jsx` files already in `routes/` that were ignored before now become routes; a release note could tell users to prefix non-route files with `_`. - Two companions for one page (`foo.server.ts` + `foo.server.tsx`) silently use the first by extension order, as `.ts` + `.tsx` already did on `main`. - Older, not from this PR: `a.ts` + `a.tsx` (like `a.ts` + `a.js` today) register two handlers for `/a` without an error. --------- Co-authored-by: LongYinan <github@lyn.one> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Depends on #561, which adds JSX server companion support. Merge after #561, or together with it. On
mainwithout #561, the page scanner still listsindex.server.tsxas a page of its own.React and Solid decide whether to build the
pages-clientbundle withhasNonIslandPages(). That check counted server companions (index.server.tsx,index.server.jsx) as ordinary pages. So an island-only app still emitted apages-clientbundle it never used.The skip matches #561's page scanner exactly: a name that ends in
.server.tsxor.server.jsxis a companion. A name that only contains.server.(status.server.v2.tsx) is still a page.Tests
packages/{react,solid}/test/unit/plugin-pages.test.ts:.server.tsxor.server.jsxfile → nopages-client;about.tsx,about.jsx,blog/…,status.server.v2.tsx) →pages-clientkept..server." rule, the 2status.server.v2.tsxcases fail.pages-clientchunk;vp previewserves only the islands script, and the island hydrates (a button click updates it).vp run test, 508 files), lint and typecheck pass.Not changed here (older behavior)
hasNonIslandPages()checks only the file name. So a file in a_-prefixed folder (e.g._components/Button.tsx) still counts as a page, while the scanner skips it. Switching the adapters to the scanner's own result would fix that; it's a separate change.🤖 Generated with Claude Code