Skip to content

fix(core): serve preview assets from web dist paths#7820

Merged
chenjiahan merged 2 commits into
mainfrom
chenjiahan/fix-preview-multi-env-assets
Jun 3, 2026
Merged

fix(core): serve preview assets from web dist paths#7820
chenjiahan merged 2 commits into
mainfrom
chenjiahan/fix-preview-multi-env-assets

Conversation

@chenjiahan

Copy link
Copy Markdown
Member

Summary

This PR fixes Rsbuild preview static asset resolution for multi-environment builds where client and server outputs share a common parent, such as dist/client and dist/server. Preview now computes public pathnames like dev and reuses the existing assets middleware against web-target environment outputs before the legacy sirv fallback, so client assets are served before SSR catch-all middleware without exposing server output as static files. It also renames the low-level assets middleware factory for clarity.

Related Links

TanStack/router#7372

Copilot AI review requested due to automatic review settings June 3, 2026 06:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes preview-mode static asset resolution for multi-environment builds (e.g. dist/client + dist/server) by reusing the existing disk assets middleware for web-target outputs and aligning preview’s public pathname computation with dev server behavior. This also includes a small middleware factory rename and an added e2e regression test.

Changes:

  • Add getPublicPathnames utility to normalize/derive public pathnames (including base stripping) and reuse it in dev + preview.
  • In preview, serve assets via the existing assets middleware for web environments before falling back to sirv.
  • Rename the low-level assets middleware factory to createAssetsMiddleware and update call sites; add an e2e case for multi-environment preview asset precedence.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/core/src/server/publicPathnames.ts New helper to compute public pathnames consistently (incl. auto handling + base stripping).
packages/core/src/server/previewServer.ts Preview asset serving now uses the assets middleware for web targets before sirv; computes context.publicPathnames.
packages/core/src/server/devServer.ts Refactors public pathname computation to use the shared helper.
packages/core/src/server/assets-middleware/middleware.ts Renames middleware factory export to createAssetsMiddleware.
packages/core/src/server/assets-middleware/index.ts Updates import/usage to match the renamed factory.
e2e/cases/server/preview/index.test.ts Adds e2e coverage for multi-environment preview asset serving ordering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/server/previewServer.ts
Comment thread e2e/cases/server/preview/index.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

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: 0b134dba46

ℹ️ 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".

Comment thread packages/core/src/server/previewServer.ts
@chenjiahan chenjiahan merged commit c94294f into main Jun 3, 2026
7 checks passed
@chenjiahan chenjiahan deleted the chenjiahan/fix-preview-multi-env-assets branch June 3, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants