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.
|
📝 WalkthroughWalkthroughAdds optional Vercel immutable static-file support. Presets relocate generated assets, Vite aligns client and SSR paths with longer hashes, and builds emit an ChangesVercel immutable asset flow
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
pi0
changed the title
pi0
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
src/presets/vercel/types.ts (1)
137-149: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the documented default match the opt-in implementation.
Line 145 says this is enabled by default, but Line 147 declares
@default false, and the preset only enables it whenVERCEL_IMMUTABLE_STATIC_FILES_ENABLEDis set. Document it as disabled by default and opt-in, or change the implementation to default-on.🤖 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/types.ts` around lines 137 - 149, Update the `immutableStaticFiles` documentation in the Vercel preset options to consistently describe the current opt-in behavior: state that it is disabled by default and enabled only when configured, and change the conflicting `@default` annotation accordingly. Leave the implementation unchanged.src/presets/vercel/preset.ts (1)
124-124: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftFix the manifest hash values before enabling this hook.
generateImmutableManifestcurrently populateshasheswith[url, url]; the values are URLs, not full content hashes as required by this feature’s contract and documentation. The resultingimmutable.jsonis incorrect for both the serverless and static presets. Compute each emitted file’s digest before writing the manifest.Also applies to: 160-162
🤖 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/preset.ts` at line 124, Update generateImmutableManifest and its callers in the serverless and static preset flows to compute a full content digest for each emitted file before writing immutable.json, rather than populating hashes with [url, url]. Ensure every manifest entry contains the file URL and its computed hash, and only enable the hook after the corrected manifest generation is in place.
🤖 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/preset.ts`:
- Line 124: Update generateImmutableManifest and its callers in the serverless
and static preset flows to compute a full content digest for each emitted file
before writing immutable.json, rather than populating hashes with [url, url].
Ensure every manifest entry contains the file URL and its computed hash, and
only enable the hook after the corrected manifest generation is in place.
In `@src/presets/vercel/types.ts`:
- Around line 137-149: Update the `immutableStaticFiles` documentation in the
Vercel preset options to consistently describe the current opt-in behavior:
state that it is disabled by default and enabled only when configured, and
change the conflicting `@default` annotation accordingly. Leave the
implementation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75f5c735-1087-44b8-898b-b4ba0a7a018e
📒 Files selected for processing (4)
docs/2.deploy/20.providers/vercel.mdsrc/presets/vercel/immutable.tssrc/presets/vercel/preset.tssrc/presets/vercel/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/presets/vercel/immutable.ts
There was a problem hiding this comment.
Actionable comments posted: 1
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/preset.ts (1)
29-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winParse the environment flag instead of coercing presence to boolean.
!!converts"false"and"0"intotrue, so users cannot disable immutable assets through the environment variable.
src/presets/vercel/preset.ts#L29-L29: explicitly parseVERCEL_IMMUTABLE_STATIC_FILES_ENABLED.src/presets/vercel/preset.ts#L142-L142: apply the same parsing logic tovercel-static, preferably through a shared helper.🤖 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/preset.ts` at line 29, Parse VERCEL_IMMUTABLE_STATIC_FILES_ENABLED as an actual environment boolean instead of coercing its presence with !!, treating values such as "false" and "0" as disabled. Apply the same parsing logic to the vercel-static configuration at src/presets/vercel/preset.ts lines 29-29 and 142-142, preferably via a shared helper.
🧹 Nitpick comments (1)
test/presets/vercel.test.ts (1)
467-468: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert manifest contents, not only its shape.
hashes: {}would satisfy these assertions, so the test would not catch a broken glob or URL mapping. Assert at least one expected asset entry and its generated URL.🤖 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 467 - 468, Strengthen the manifest assertions in the test around manifest.version and manifest.hashes by verifying that hashes contains at least one expected asset entry and that the entry has the correct generated URL. Keep the existing version assertion and object-shape check while ensuring broken glob or URL mapping behavior fails the test.
🤖 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 `@test/presets/vercel.test.ts`:
- Line 462: The buildAssetsDir assertion in the Vercel preset test is hard-coded
despite immutableDir() incorporating VERCEL_HASH_SALT. Update the expectation to
derive the immutable path using the active VERCEL_HASH_SALT value, or isolate
the test by clearing that environment variable and restoring it afterward.
---
Outside diff comments:
In `@src/presets/vercel/preset.ts`:
- Line 29: Parse VERCEL_IMMUTABLE_STATIC_FILES_ENABLED as an actual environment
boolean instead of coercing its presence with !!, treating values such as
"false" and "0" as disabled. Apply the same parsing logic to the vercel-static
configuration at src/presets/vercel/preset.ts lines 29-29 and 142-142,
preferably via a shared helper.
---
Nitpick comments:
In `@test/presets/vercel.test.ts`:
- Around line 467-468: Strengthen the manifest assertions in the test around
manifest.version and manifest.hashes by verifying that hashes contains at least
one expected asset entry and that the entry has the correct generated URL. Keep
the existing version assertion and object-shape check while ensuring broken glob
or URL mapping behavior fails the test.
🪄 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: ba51474d-6bcd-4384-bae0-46ff51f619a5
📒 Files selected for processing (4)
src/presets/vercel/immutable.tssrc/presets/vercel/preset.tstest/fixture/nitro.config.tstest/presets/vercel.test.ts
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
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 orVERCEL_IMMUTABLE_STATIC_FILES_ENABLEDenv var (vercel CI platform enables this by default so it is practically an opt-out!)Local testing in examples:
v2 backport: #4434