presets(vercel): support immutable static files - #4432
Conversation
Co-Authored-By: Rihan <me@file.properties>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesVercel immutable asset flow
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
commit: |
d1f4dab to
b6ab335
Compare
b6ab335 to
4472a85
Compare
9bc6c67 to
d2f3916
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/presets/vercel.test.ts (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
try/finallyto guarantee environment restoration.If the build/setup phase fails, the environment variable won't be restored, potentially leaking into cleanup hooks or other test suites in the same file. Consider wrapping the setup phase (between the override and the restoration) in a
try/finallyblock to ensureprocess.env.VERCEL_HASH_SALTis always restored.🤖 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/presets/vercel.test.ts` around lines 46 - 50, Wrap the test setup and execution between the VERCEL_HASH_SALT override and restoration in a try/finally block, ensuring the existing restoration logic runs even when setup or assertions fail. Keep the undefined case deleting process.env.VERCEL_HASH_SALT and the defined case restoring its previous value.
🤖 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 `@test/presets/vercel.test.ts`:
- Around line 46-50: Wrap the test setup and execution between the
VERCEL_HASH_SALT override and restoration in a try/finally block, ensuring the
existing restoration logic runs even when setup or assertions fail. Keep the
undefined case deleting process.env.VERCEL_HASH_SALT and the defined case
restoring its previous value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 888ef528-c2dd-417d-a99c-a4b161934e0e
📒 Files selected for processing (3)
src/build/vite/env.tssrc/build/vite/plugin.tstest/presets/vercel.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/build/vite/env.ts
- src/build/vite/plugin.ts
| // `useLongerAssetHashes` in `plugin.ts`) so a `?url` asset import | ||
| // resolves to the same filename on both sides. | ||
| ...(buildAssetsDir | ||
| ? { assetFileNames: `${buildAssetsDir}/[name]-[hash:16][extname]` } |
There was a problem hiding this comment.
TODO: We are still blindly overriding assetFileNames for server envs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/presets/vercel/immutable.ts (1)
60-75: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winSort the glob results before serializing the manifest.
tinyglobbydoes not guarantee file order, soObject.fromEntries(files.map(...))can produce differentimmutable.jsonbytes for the same asset set. Sortingfilesfirst keeps the manifest stable and avoids cache churn.🤖 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/presets/vercel/immutable.ts` around lines 60 - 75, Sort the files array returned by glob before mapping it into the hashes object in the immutable manifest generation flow. Update the files processing near ImmutableManifest so identical asset sets always produce deterministic entry ordering and stable immutable.json output.
🧹 Nitpick comments (1)
src/build/vite/plugin.ts (1)
493-527: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an options object for the helper configuration.
useLongerAssetHashes(build, isRolldown, assetsDir)has multiple positional configuration arguments. Use{ isRolldown, assetsDir }as the second parameter to make call sites clearer and avoid future signature churn.As per coding guidelines, multi-argument functions should use an options object as the second parameter.
Suggested shape
-export function useLongerAssetHashes(build, isRolldown, assetsDir) +export function useLongerAssetHashes( + build, + { isRolldown, assetsDir }: { isRolldown: boolean | undefined; assetsDir: string } +)🤖 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/vite/plugin.ts` around lines 493 - 527, Update useLongerAssetHashes to accept a single options object containing isRolldown and assetsDir instead of separate positional arguments, and adjust every call site to pass those named properties while preserving the existing hash-upgrade behavior.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/presets/vercel/immutable.ts`:
- Around line 60-75: Sort the files array returned by glob before mapping it
into the hashes object in the immutable manifest generation flow. Update the
files processing near ImmutableManifest so identical asset sets always produce
deterministic entry ordering and stable immutable.json output.
---
Nitpick comments:
In `@src/build/vite/plugin.ts`:
- Around line 493-527: Update useLongerAssetHashes to accept a single options
object containing isRolldown and assetsDir instead of separate positional
arguments, and adjust every call site to pass those named properties while
preserving the existing hash-upgrade behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 717b4f1c-51cd-4e18-9da6-c50ea050707a
📒 Files selected for processing (5)
docs/2.deploy/20.providers/vercel.mdsrc/build/vite/plugin.tssrc/presets/vercel/immutable.tssrc/presets/vercel/preset.tssrc/presets/vercel/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/presets/vercel/types.ts
- docs/2.deploy/20.providers/vercel.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/build/vite/plugin.ts (1)
511-515: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an options object for helper configuration.
useLongerAssetHashesadds configuration as positional parameters instead of using an options object as the second parameter, contrary to the repository guideline. Refactor the signature and its call sites before adding further options.Suggested signature
export function useLongerAssetHashes( build: NonNullable<EnvironmentOptions["build"]>, - isRolldown: boolean | undefined, - assetsDir: string, - opts?: { assetsOnly?: boolean } + { + isRolldown, + assetsDir, + assetsOnly = false, + }: { + isRolldown?: boolean; + assetsDir: string; + assetsOnly?: boolean; + } ): void {🤖 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/vite/plugin.ts` around lines 511 - 515, Refactor useLongerAssetHashes to accept a single options object for all helper configuration, including the existing isRolldown, assetsDir, and optional assetsOnly values. Update every call site to pass the equivalent named properties, preserving current behavior and parameter values.Source: Coding guidelines
🤖 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/build/vite/plugin.ts`:
- Around line 505-510: The output-options handling around the client/SSR
configuration must preserve explicitly configured entryFileNames,
chunkFileNames, and assetFileNames, including patterns containing [hash]. Only
populate fields that are undefined; move any longer-hash defaulting before
custom output patterns are applied, and update both affected branches without
rewriting user or plugin values.
---
Nitpick comments:
In `@src/build/vite/plugin.ts`:
- Around line 511-515: Refactor useLongerAssetHashes to accept a single options
object for all helper configuration, including the existing isRolldown,
assetsDir, and optional assetsOnly values. Update every call site to pass the
equivalent named properties, preserving current behavior and parameter values.
🪄 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: 0446596e-868a-480f-99ef-8b7a08e80e4d
📒 Files selected for processing (1)
src/build/vite/plugin.ts
- disable immutable static files (with a warning) on a non-root baseURL, since they must be served from the reserved `/_vercel/immutable/` path - warn instead of silently writing an empty manifest when no assets are emitted under `buildAssetsDir` - normalize `buildAssetsDir` in config and when deriving the immutable dir - correct manifest comments and docs: values are file paths (explicitly allowed by the spec) and `VERCEL_HASH_SALT` is factored into the asset paths rather than the content hashes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 participants
Draft docs:
https://nitrobuild-git-feat-vercel-immutable-nitrojs.vercel.app/deploy/providers/vercel#immutable-static-files
Preview package for e2e testing:
Then opt-in via
vercel.immutableStaticFiles: truenitro config orNITRO_VERCEL_IMMUTABLE_STATIC_FILES_ENABLEDLocal testing in examples:
v2 backport: #4434