Conversation
🦋 Changeset detectedLatest commit: fa4123e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request updates the arc-vite package to support Vite version 8. A new changeset entry is created to document a patch-level release. The vite peer dependency range in package.json is modified from "4 - 7" to "4 - 8", expanding compatibility to include the latest Vite major version while maintaining support for earlier versions. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 78: The generateBundle hook in src/plugins/build-web.ts currently mutates
the bundle (calls delete bundle[fileName]) which breaks compatibility with Vite
8; update the hook to stop mutating the bundle and instead re-emit assets using
this.emitFile() (create a new asset with the same content/name or mark files to
keep and emit corrected versions), or alternatively tighten the peer dependency
range to exclude Vite 8 and add Vite 8 to the CI matrix to prevent regressions;
locate generateBundle and any logic referencing delete bundle[fileName] and
replace the deletion/mutation with this.emitFile() usage and/or adjust
package.json peerDependencies and CI config accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 778a4d0f-d26c-4add-a5c5-e335192d6f85
📒 Files selected for processing (2)
.changeset/cute-rocks-unite.mdpackage.json
| "peerDependencies": { | ||
| "arc-server": "^3.0.2", | ||
| "vite": "4 - 7" | ||
| "vite": "4 - 8" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vite 8, were there breaking changes to plugin APIs configResolved, generateBundle, or config.experimental.renderBuiltUrl compared to Vite 7?
💡 Result:
-
configResolved: No breaking change in Vite 8 vs Vite 7 is documented for this hook; Vite 8’s migration guide still shows usingconfigResolved(config)normally (e.g., to read compatibility-layer-resolved options). [1] -
generateBundle: The hook still exists, but there are breaking/behavioral changes to thebundleobject passed togenerateBundle/writeBundleunder Rolldown (Vite 8), including: assigningbundle[foo]is not supported (usethis.emitFile()), thebundlereference isn’t shared across hooks, andstructuredClone(bundle)can now throw (clone{ ...bundle }instead). [1] -
config.experimental.renderBuiltUrl: No Vite-8-specific breaking change is called out in the Vite 8 migration guide, and it remains documented as an experimental option. [1][2]
Sources: Vite “Migration from v7” (Vite 8) guide [1], Vite “Building for Production” docs (documents experimental.renderBuiltUrl) [2].
🏁 Script executed:
# Find the plugin file mentioned in the review
find . -name "build-web.ts" -type fRepository: marko-js/arc-vite
Length of output: 87
🏁 Script executed:
# Check package.json structure and Vite version
head -100 package.json | tail -50Repository: marko-js/arc-vite
Length of output: 1676
🏁 Script executed:
# Look for CI configuration files
find . -name ".github" -type d
ls -la .github/workflows/ 2>/dev/null || echo "No GitHub workflows found"Repository: marko-js/arc-vite
Length of output: 237
🏁 Script executed:
# Read the plugin code around lines 55-79
cat -n ./src/plugins/build-web.ts | sed -n '50,85p'Repository: marko-js/arc-vite
Length of output: 1412
🏁 Script executed:
# Read CI configuration
cat -n ./.github/ci.ymlRepository: marko-js/arc-vite
Length of output: 109
🏁 Script executed:
# List all files in .github
ls -la ./.github/Repository: marko-js/arc-vite
Length of output: 436
🏁 Script executed:
# Search for bundle object assignments in the entire plugin file
cat -n ./src/plugins/build-web.ts | grep -E "(bundle\[|this\.emitFile|generateBundle|writeBundle)"Repository: marko-js/arc-vite
Length of output: 382
🏁 Script executed:
# Read the generateBundle hook context (lines 410-445)
cat -n ./src/plugins/build-web.ts | sed -n '410,445p'Repository: marko-js/arc-vite
Length of output: 1572
🏁 Script executed:
# Check CI workflows for Vite version testing
cat -n ./.github/workflows/*.ymlRepository: marko-js/arc-vite
Length of output: 3006
Fix generateBundle hook to be compatible with Vite 8 before widening peer dependency.
The code at line 440 (delete bundle[fileName]) mutates the bundle object directly, which is incompatible with Vite 8. Vite 8 plugin API breaking change: assigning or deleting from the bundle object is no longer supported—use this.emitFile() instead. Additionally, CI does not test Vite 8 compatibility despite the peer dependency claiming support for versions 4–8.
Either fix the generateBundle hook in src/plugins/build-web.ts to avoid bundle mutations, add Vite 8 to the CI test matrix to catch regressions, or narrow the peer dependency range to exclude Vite 8 until the code is updated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 78, The generateBundle hook in src/plugins/build-web.ts
currently mutates the bundle (calls delete bundle[fileName]) which breaks
compatibility with Vite 8; update the hook to stop mutating the bundle and
instead re-emit assets using this.emitFile() (create a new asset with the same
content/name or mark files to keep and emit corrected versions), or
alternatively tighten the peer dependency range to exclude Vite 8 and add Vite 8
to the CI matrix to prevent regressions; locate generateBundle and any logic
referencing delete bundle[fileName] and replace the deletion/mutation with
this.emitFile() usage and/or adjust package.json peerDependencies and CI config
accordingly.
There was a problem hiding this comment.
Hmm... I didn't get that far, I only forced installed v8, and saw that nothing broke. Might have to dig deeper here...
There was a problem hiding this comment.
@vwong, that's a fair point to investigate more carefully! The concern here is that the breakage may be silent rather than a hard crash. The Vite 8 migration guide explicitly documents that assigning/deleting from bundle[foo] is not supported (this is also discouraged by Rollup), and the recommended fix is to use this.emitFile() instead.
The key risk with delete bundle[fileName] (line 440 of src/plugins/build-web.ts) is not that it throws an error — it's that under Rolldown it may silently no-op, meaning the file you're trying to suppress would still end up written to disk. A force-install smoke test might not catch this unless the test assertions explicitly verify which output files are (and aren't) present.
To get more confidence, it would be worth checking:
- Does your test suite verify the output file list? Specifically, does it assert that the files removed via
delete bundle[fileName]are absent from the build output? - What is the actual build output with Vite 8? Running a build and diffing the output files vs. Vite 7 would confirm whether the deletion is being honored or silently ignored.
If the deletion is indeed being silently ignored, the fix would be to avoid the mutation altogether — since Rolldown doesn't support removing entries from the bundle object, an alternative approach would be to use the shouldTransformCachedModule hook, or restructure the plugin so the files are never emitted in the first place (e.g., by controlling them at the generateBundle phase without relying on deletion). Would you like me to propose a concrete fix based on the plugin's current logic?
There was a problem hiding this comment.
Yes please propose a fix.
There was a problem hiding this comment.
🧩 Analysis chain
Let me gather the full context of the generateBundle hook to propose a proper fix.
🏁 Script executed:
#!/bin/bash
# Get the full generateBundle hook and surrounding context
cat -n ./src/plugins/build-web.ts
echo "---"
# Also check all bundle mutations
rg -n "bundle\[" ./src/plugins/build-web.ts
Length of output: 22048
@vwong, here's the proposed fix. The core idea is to stop mutating the bundle object entirely and instead delete the intermediate .arc.html files from disk in the writeBundle hook, which fires after Rollup/Rolldown has already written all outputs. This is fully compatible with both Vite 7 and Vite 8.
src/plugins/build-web.ts
@@ around line 37-50, add arcHtmlFilesToDelete to the shared closure @@
let globalIds = new Map<string, string>();
let metaForProxy = new Map<string, ProxyMeta>();
let adaptiveImporters = new Map<string, Map<string, string>>();
let bindingsByAdaptiveId = new Map<string, Set<string> | true>();
let metaForAdaptiveChunk = new Map<
string,
{
entryId: string;
adaptiveImports: Map<string, string>;
}
>();
let proxyModuleId = 0;
let initModuleId = 0;
let basePath = "/";
+ let arcHtmlFilesToDelete = new Set<string>();@@ around line 80-87, reset the new set in closeBundle @@
closeBundle() {
proxyModuleId = initModuleId = 0;
globalIds = new Map();
adaptiveImporters = new Map();
bindingsByAdaptiveId = new Map();
metaForAdaptiveChunk = new Map();
metaForProxy = new Map();
+ arcHtmlFilesToDelete = new Set();
},@@ line 440, stop mutating the bundle @@
- delete bundle[fileName];
+ arcHtmlFilesToDelete.add(fileName);@@ after the generateBundle hook (around line 484), add writeBundle to arc-vite:build-web:post @@
+ async writeBundle(options) {
+ if (!options.dir) return;
+ await Promise.all(
+ [...arcHtmlFilesToDelete].map((fileName) =>
+ fs.unlink(path.join(options.dir!, fileName)).catch(() => {}),
+ ),
+ );
+ arcHtmlFilesToDelete = new Set();
+ },Why this works:
generateBundlestill reads each arc HTML asset'ssourceto build the manifest — that part is unchanged.- Instead of
delete bundle[fileName](which Rolldown does not support), we just record the file name. writeBundlefires after all outputs have been written to disk, sofs.unlinkreliably removes the intermediate.arc[.flagset].htmlfiles that should not be in the final output.- The
generateBundlehook already assertsisWrite === true(lines 415–419), sowriteBundlewill always follow. - The
.catch(() => {})is a safe guard for any edge case where the file wasn't written (e.g., Rolldown-internal caching); it won't mask real errors since the manifest logic has already succeeded by this point.
Would you like me to kick off a full code change plan with this diff?
There was a problem hiding this comment.
🧩 Analysis chain
@vwong, kicking off the full code change plan now!
Task: Fix generateBundle bundle mutation for Vite 8 compatibility
- Add
arcHtmlFilesToDeleteset to shared closure - Reset it in
closeBundle - Replace
delete bundle[fileName]with tracking the file name - Add
writeBundlehook to delete the tracked files from disk after writing
🏁 Script executed:
#!/bin/bash
# Read the full file with line numbers to get exact context
cat -n ./src/plugins/build-web.ts
Length of output: 21907
@vwong, here's the full, precise diff based on the exact file content:
--- a/src/plugins/build-web.ts
+++ b/src/plugins/build-web.ts
@@ -50,6 +50,7 @@ export function pluginBuildWeb({
let proxyModuleId = 0;
let initModuleId = 0;
let basePath = "/";
+ let arcHtmlFilesToDelete = new Set<string>();
let resolveAssetURL: (fileName: string, from: string) => string = (
fileName: string,
) => basePath + fileName;
@@ -83,6 +84,7 @@ export function pluginBuildWeb({
bindingsByAdaptiveId = new Map();
metaForAdaptiveChunk = new Map();
metaForProxy = new Map();
+ arcHtmlFilesToDelete = new Set();
},
@@ -437,7 +438,7 @@ export function pluginBuildWeb({
const arcHTMLChunkMatch = arcChunkFileNameReg.exec(chunk.fileName);
if (!arcHTMLChunkMatch) continue;
- delete bundle[fileName];
+ arcHtmlFilesToDelete.add(fileName);
const [, entryName, flagSetStr] = arcHTMLChunkMatch;
@@ -482,6 +483,15 @@ export function pluginBuildWeb({
await Promise.all(
serverEntryFiles.map((file) => fs.appendFile(file, manifestCode)),
);
},
+ async writeBundle(options) {
+ if (!options.dir) return;
+ await Promise.all(
+ [...arcHtmlFilesToDelete].map((fileName) =>
+ fs
+ .unlink(path.join(options.dir!, fileName))
+ .catch(() => {}),
+ ),
+ );
+ arcHtmlFilesToDelete = new Set();
+ },
},
];Summary of all four changes:
| Line | Change |
|---|---|
| After line 50 | Declare let arcHtmlFilesToDelete = new Set<string>(); alongside the other shared state |
Line 86 (in closeBundle) |
Reset arcHtmlFilesToDelete = new Set() so watch-mode rebuilds start clean |
| Line 440 | Replace delete bundle[fileName] (unsupported in Rolldown/Vite 8) with arcHtmlFilesToDelete.add(fileName) |
After line 484 (in arc-vite:build-web:post) |
Add writeBundle hook that calls fs.unlink for each tracked file, then clears the set |
No new imports are needed — fs (line 2) and path (line 3) are already imported. The .catch(() => {}) is a safe guard in case Rolldown skips writing a specific asset; it won't mask logic errors since manifest generation has already completed successfully inside generateBundle by that point.
Failed to handle agent chat message. Please try again.
There was a problem hiding this comment.
@DylanPiercey I think I need your input from here on - this is a bit beyond me now.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
Closing in favor of #28 |
2 participants
Bump peer dependency Vite to v8