fix(build,azure-swa): prefer index.mjs serverEntry; fix Azure SWA request URL and body#4352
Open
tonoizer wants to merge 4 commits into
Open
fix(build,azure-swa): prefer index.mjs serverEntry; fix Azure SWA request URL and body#4352tonoizer wants to merge 4 commits into
tonoizer wants to merge 4 commits into
Conversation
…RL and body - extract resolveNitroServerEntryFileName: when multiple isEntry chunks exist (e.g. Module Federation expose chunks), prefer the one matching index.mjs instead of taking the first chunk arbitrarily - resolveAzureSwaRequestUrl: use x-ms-original-url as-is (full absolute URL) or construct an absolute URL via new URL(..., "http://nitro.local") — new Request() requires an absolute URL in Node.js - materialise response.body as text before assigning to context.res.body; Azure Functions v3 cannot serialise a ReadableStream - add unit tests for both helpers
|
@tonoizer is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughUpdates build metadata to select the configured server entry chunk, with fallbacks for ChangesServer Entry Filename Resolution
Azure SWA URL and Body Handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/presets/azure/runtime/azure-swa.ts`:
- Around line 27-28: The response body conversion in azure-swa.ts is using
`.text()` which corrupts binary responses by assuming UTF-8 encoding. Instead of
unconditionally calling `.text()` on the Response object, inspect the
content-type header from the response to determine the correct handling: for
binary content types (images, PDFs, etc.), use `arrayBuffer()` and convert the
result to a Buffer that Azure Functions v3 expects, and for text content types
use `.text()`. Follow the same pattern already implemented in the AWS Lambda
preset to maintain consistency across presets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34ecfe6a-7c6d-44a2-9946-b082cf2dc6e0
📒 Files selected for processing (4)
src/build/info.tssrc/presets/azure/runtime/azure-swa.tstest/unit/azure-swa.test.tstest/unit/build-info.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/presets/azure/runtime/_utils.ts`:
- Around line 68-70: The `isTextType` function's regex pattern only matches
media types with `/json`, `/xml`, and `/javascript` but misses the `+json` and
`+xml` suffixes commonly used in structured media types like
`application/problem+json`. Update the regex pattern in the `isTextType`
function on Line 69 to include both the slash variants `/(javascript|json|xml)`
and the plus variants `\+(json|xml)` so that types with either separator are
correctly identified as text and not returned as binary Buffer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7dbf344-fb20-4eec-8ce2-b8ce200b0e1a
📒 Files selected for processing (3)
src/presets/azure/runtime/_utils.tssrc/presets/azure/runtime/azure-swa.tstest/unit/azure-swa.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/presets/azure/runtime/azure-swa.ts
…on content types and add corresponding unit test
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/build/info.ts (1)
106-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an options object for the second parameter in multi-argument functions.
As per coding guidelines, multi-argument functions should use an options object as the second parameter. The internal helpers and their call site should be updated to adhere to this rule.
src/build/info.ts#L106-L128: UpdateresolveNitroServerEntryFileNameandisNitroEntrysignatures to accept an options object (e.g.,{ nitroEntry: string }) instead of a raw string.src/build/info.ts#L51-L51: Update this call site to passnitro.options.entrywithin an options object.♻️ Proposed fix
--- src/build/info.ts +++ src/build/info.ts @@ -50,2 +50,2 @@ - const serverEntryName = resolveNitroServerEntryFileName(output, nitro.options.entry); + const serverEntryName = resolveNitroServerEntryFileName(output, { nitroEntry: nitro.options.entry }); @@ -106,4 +106,4 @@ function resolveNitroServerEntryFileName( output: RolldownOutput | RollupOutput | undefined, - nitroEntry: string + options: { nitroEntry: string } ): string | undefined { @@ -113,3 +113,3 @@ (entry) => - entry.type === "chunk" && entry.isEntry && isNitroEntry(entry.facadeModuleId, nitroEntry) + entry.type === "chunk" && entry.isEntry && isNitroEntry(entry.facadeModuleId, { nitroEntry: options.nitroEntry }) ) ?? @@ -122,5 +122,5 @@ -function isNitroEntry(facadeModuleId: string | null, nitroEntry: string): boolean { +function isNitroEntry(facadeModuleId: string | null, options: { nitroEntry: string }): boolean { if (!facadeModuleId) return false; - if (facadeModuleId === nitroEntry) return true; + if (facadeModuleId === options.nitroEntry) return true; const extension = extname(facadeModuleId); - return !!extension && facadeModuleId.slice(0, -extension.length) === nitroEntry; + return !!extension && facadeModuleId.slice(0, -extension.length) === options.nitroEntry; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/build/info.ts` around lines 106 - 128, Update src/build/info.ts lines 106-128 so resolveNitroServerEntryFileName and isNitroEntry accept an options object containing nitroEntry, and update src/build/info.ts line 51 to pass nitro.options.entry through that object; preserve the existing entry-resolution behavior.Source: Coding guidelines
test/unit/build-info.test.ts (1)
58-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding tests for fallback entry selection logic.
The current tests verify the primary logic where the chunk's
facadeModuleIdmatches the Nitro entry. To ensure comprehensive coverage, consider adding test cases for the fallback behaviors inresolveNitroServerEntryFileName:
- When no
facadeModuleIdmatches, but a chunk hasfileName: "index.mjs".- When neither a
facadeModuleIdmatch nor an"index.mjs"chunk exists, falling back to the first availableisEntrychunk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/build-info.test.ts` around lines 58 - 91, Add unit tests covering the fallback branches of resolveNitroServerEntryFileName: first verify that an isEntry chunk named index.mjs is selected when no facadeModuleId matches, then verify that the first available isEntry chunk is selected when neither a matching facadeModuleId nor index.mjs exists. Assert the resulting buildInfo.serverEntry values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/build/info.ts`:
- Around line 106-128: Update src/build/info.ts lines 106-128 so
resolveNitroServerEntryFileName and isNitroEntry accept an options object
containing nitroEntry, and update src/build/info.ts line 51 to pass
nitro.options.entry through that object; preserve the existing entry-resolution
behavior.
In `@test/unit/build-info.test.ts`:
- Around line 58-91: Add unit tests covering the fallback branches of
resolveNitroServerEntryFileName: first verify that an isEntry chunk named
index.mjs is selected when no facadeModuleId matches, then verify that the first
available isEntry chunk is selected when neither a matching facadeModuleId nor
index.mjs exists. Assert the resulting buildInfo.serverEntry values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75965414-eddf-46fb-a9b2-47c334c2ece6
📒 Files selected for processing (3)
src/build/info.tstest/unit/azure-swa.test.tstest/unit/build-info.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/azure-swa.test.ts
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
1 participant
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.
Problem
Three bugs that compound when deploying a Nitro fullstack app with Vite Module Federation
exposes(and when using the Azure Static Web Apps preset).Reproduction demo
Demo repo: https://github.com/tonoizer/demo-nitro-vite-module-federation-serverEntry-bug
Minimal repro for the
serverEntry/vite previewissue:pnpm devworks — only production preview is broken because it relies on.output/nitro.json.1. Wrong
serverEntryinnitro.jsonwriteBuildInfoused.find()to pick the firstisEntrychunk from Rollup output. Vite Module Federation marks expose chunks as entry points too, so when they appear beforeserver/index.mjsin the output array,nitro.jsonpoints at e.g.server/_chunks/app.mjsinstead of the server bundle.vite previewthen loads a file with nofetchhandler → 404 everywhere.2. Azure SWA: relative URL passed to
new Request()azure-swa.tsconstructed a relative path (/api/...orpathname + search) and passed it directly tonew Request(url, ...). Node.js requires an absolute URL — this throws in production and breaks all/api/*routes.3. Azure SWA: response body handling
context.res.bodywas set incorrectly for Azure Functions v3 (which expectsstring | Buffer). Text responses could be empty/wrong; binary responses were corrupted when unconditionally calling.text().Fix
resolveNitroServerEntryFileName— when multipleisEntrychunks exist, prefer the one matching/(^|\/)index\.mjs$/(Nitro's server entry convention) instead of taking array-first.resolveAzureSwaRequestUrl— usex-ms-original-urlas-is (it's already an absolute URL) or constructnew URL(path, "http://nitro.local").hreffor direct/api/*calls.azureResponseBody— inspectcontent-typeand return UTF-8 string for text types orBufferfor binary (same pattern as AWS Lambda preset).Test plan
pnpm vitest run test/unit/build-info.test.ts test/unit/azure-swa.test.tspnpm build && pnpm checkpasses;pnpm previewserves SPA +/api/hello/api/*routes return real JSON/HTML; binary routes return correct bytes