fix(vite-plugin-ruby): reuse an already-emitted asset instead of fingerprinting it twice#612
Open
vitalyliber wants to merge 2 commits into
Open
fix(vite-plugin-ruby): reuse an already-emitted asset instead of fingerprinting it twice#612vitalyliber wants to merge 2 commits into
vitalyliber wants to merge 2 commits into
Conversation
…erprinting it twice
fingerprintRemainingAssets re-reads and re-emits any asset matched by
additionalEntrypoints (e.g. app/frontend/{assets,images,...}/**/*),
even if the same file was already emitted by Vite's own asset pipeline
because it's also imported from JS/CSS or referenced from an HTML
entrypoint.
Two independent emitFile calls for byte-identical content aren't
guaranteed to resolve to the same content hash — this can differ
between Rollup and Rolldown (Vite v8's default bundler). When they
diverge, manifest-assets.json (which wins when vite_ruby merges
manifest files, since it's loaded last) ends up pointing at a hashed
filename that vite-plugin-ruby computed but never actually wrote to
disk, while a different file with the correct content sits under the
hash from manifest.json. Rails then 404s on any vite_asset_path call
for that entry.
Scan the bundle for an OutputAsset whose originalFileNames (or the
deprecated singular originalFileName) already contains the absolute
path before emitting again, and reuse its fileName. This removes the
double-emit for any asset that's both entrypoint-globbed and
otherwise referenced, regardless of which bundler is in use, and also
avoids writing the same file to the output directory twice.
See ElMassimo#611 for the
originating report and a full production repro.
Verified the previous commit's fix against the real production app from issue ElMassimo#611 (Vite 8 + a large Vue2/Rails app) by pointing its package.json at this branch. The mismatch still reproduced — the dedup lookup never matched. Traced it to a difference in what Rolldown reports for OutputAsset#originalFileNames: Rollup's types document it as absolute paths, and that holds for most assets in this build, but for the one matched by additionalEntrypoints AND imported from a Vue SFC (bluethumb-nav-logo.svg), Rolldown reported it as a single path relative to config.root ("assets/bluethumb-nav-logo.svg") instead of absolute. findExistingAsset compared that directly against the absolute filename from vite-plugin-ruby's own glob and never matched, so it fell through to emitFile and re-triggered the original bug. Resolve non-absolute originalFileNames against config.root before comparing. Added a regression test for this exact shape, and confirmed against the real app: manifest.json and manifest-assets.json now agree on a single hash across repeated clean builds, and the asset resolves (HTTP 200) when served.
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
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.
Summary
Closes #611.
fingerprintRemainingAssetsunconditionally re-reads and re-emitFiles every asset matched byadditionalEntrypoints(the default~/{assets,fonts,icons,images}/**/*glob), even when the exact same file was already emitted once by Vite's own asset pipeline — e.g. because it's alsoimported from JS/CSS, or referenced from an HTML entrypoint.Two independent
emitFilecalls for byte-identical content aren't guaranteed to resolve to the same content hash. In a production app that upgraded from Vite 7 to Vite 8 (which defaults to Rolldown instead of Rollup), this manifested as:Same source file, two different hashes across
manifest.jsonandmanifest-assets.json, and only one of the two ever gets written to disk. SinceViteRuby::Manifest#load_manifestmerges manifest files withHash#mergeandmanifest-assets.jsonis loaded last, it wins — sovite_asset_pathresolves to a file that doesn't exist, and Rails 404s. Full repro and versions are in #611.Fix
Before fingerprinting a
remainingAsset, scan thebundlefor anOutputAssetwhoseoriginalFileNames(or the deprecated singularoriginalFileName) already contains the source path, and reuse itsfileNameinstead of emitting again. This sidesteps the hash-mismatch entirely, regardless of which bundler is in use, and also avoids writing the same file to the output directory twice when it's already covered by Vite's own pipeline.Second commit — found while verifying against the real app: the first version of this fix compared
originalFileNamesdirectly against the absolute source path, per Rollup's documented (and typed) contract thatoriginalFileNamesare absolute. I pointed the reporting app'spackage.jsonat this branch to confirm the fix before writing this up, and the mismatch still reproduced — the lookup never matched. Turned out Rolldown doesn't always honor that contract: for the specific asset from the report (matched byadditionalEntrypointsand imported from a Vue SFC), it reportedoriginalFileNames: ["assets/bluethumb-nav-logo.svg"]— relative toconfig.root, not absolute. The second commit resolves non-absolute entries againstconfig.rootbefore comparing, with a regression test for that exact shape.Test plan
vite-plugin-ruby/tests/manifest.spec.ts, unit-testingassetsManifestPlugindirectly against a fakebundle/PluginContext— including a case withoriginalFileNamesrelative toconfig.root. Confirmed each case is red without its corresponding piece of the fix, green with it.pnpm -C vite-plugin-ruby build && pnpm -C vite-plugin-ruby test --run— 17/17 passing.pnpm -C vite-plugin-ruby linton the touched files — clean.pnpm -r build && pnpm -C vite-plugin-rails test --runagainstexamples/rails(Vite 8) —images/logo.png(matched by bothadditionalEntrypointsand theindex.html<link>tag) resolves to the same hash in both manifests before and after the fix in this fixture, so no regression there. I wasn't able to get this specific example to reproduce the hash mismatch itself, which is why the unit tests above exercise the dedup logic directly.package.jsonat this branch, ranRAILS_ENV=production bin/vite buildfrom a cleanpublic/vitetwice in a row —manifest.jsonandmanifest-assets.jsonnow agree on a single hash for the previously-mismatched asset, the file exists on disk, and it servesHTTP 200from a running Rails server.tests/build.spec.ts's snapshot test failed for me on bothmainand this branch with an unrelated chunk-hash diff forvue.js(vue-CoJ_KGkH.jsvs a freshly-built hash) — pre-existing flakiness in my local environment, not caused by this change.