Skip to content

Commit b486b8d

Browse files
fix: address code review findings
- ETag regex: use lastIndexOf('-') to extract only the final hash segment, avoiding capture of filename fragments in multi-hyphenated names (e.g. `before-pop-state-D3R3Ien6` now extracts `D3R3Ien6`) - Progress bar: track processed count (including skipped files) so the bar always reaches 100% - Tests: update ETag test for filename-hash behavior, add coverage for mtime fallback on non-hashed and unhashed-in-assets files
1 parent 166d0f9 commit b486b8d

File tree

3 files changed

+33
-6
lines changed

3 files changed

+33
-6
lines changed

packages/vinext/src/build/precompress.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,17 @@ export async function precompressAssets(
9898
}
9999

100100
// Process in chunks to bound concurrent CPU-heavy compressions
101+
let processed = 0;
101102
for (let i = 0; i < filePaths.length; i += CONCURRENCY) {
102103
const chunk = filePaths.slice(i, i + CONCURRENCY);
103104
await Promise.all(
104105
chunk.map(async (fullPath) => {
105106
const content = await fsp.readFile(fullPath);
106-
if (content.length < MIN_SIZE) return;
107+
processed++;
108+
if (content.length < MIN_SIZE) {
109+
onProgress?.(processed, filePaths.length, path.basename(fullPath));
110+
return;
111+
}
107112

108113
result.filesCompressed++;
109114
result.totalOriginalBytes += content.length;
@@ -136,7 +141,7 @@ export async function precompressAssets(
136141
await Promise.all(writes);
137142

138143
result.totalCompressedBytes += brContent.length;
139-
onProgress?.(result.filesCompressed, filePaths.length, path.basename(fullPath));
144+
onProgress?.(processed, filePaths.length, path.basename(fullPath));
140145
}),
141146
);
142147
}

packages/vinext/src/server/static-file-cache.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,11 @@ export class StaticFileCache {
236236
*/
237237
function etagFromFilenameHash(relativePath: string, ext: string): string | null {
238238
const basename = path.basename(relativePath, ext);
239-
const match = basename.match(/-([a-zA-Z0-9_-]{6,})$/);
240-
return match ? `W/"${match[1]}"` : null;
239+
const lastDash = basename.lastIndexOf("-");
240+
if (lastDash === -1 || lastDash === basename.length - 1) return null;
241+
const suffix = basename.slice(lastDash + 1);
242+
// Vite emits 8-char base64url hashes; allow 6-12 for other bundlers
243+
return suffix.length >= 6 && suffix.length <= 12 ? `W/"${suffix}"` : null;
241244
}
242245

243246
function buildVariant(

tests/static-file-cache.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,32 @@ describe("StaticFileCache", () => {
104104
expect(entry!.original.headers["Cache-Control"]).toBe("public, max-age=3600");
105105
});
106106

107-
it("generates weak etag from size and mtime", async () => {
107+
it("generates weak etag from filename hash for hashed assets", async () => {
108108
await writeFile(clientDir, "assets/app-abc123.css", ".body { margin: 0; }");
109109

110110
const cache = await StaticFileCache.create(clientDir);
111111
const entry = cache.lookup("/assets/app-abc123.css");
112112

113-
// Etag should be W/"<size>-<mtime>" format (like sirv)
113+
// Hashed assets use the content hash from the filename
114+
expect(entry!.etag).toBe('W/"abc123"');
115+
});
116+
117+
it("falls back to mtime etag for non-hashed files", async () => {
118+
await writeFile(clientDir, "favicon.ico", "icon");
119+
120+
const cache = await StaticFileCache.create(clientDir);
121+
const entry = cache.lookup("/favicon.ico");
122+
123+
expect(entry!.etag).toMatch(/^W\/"\d+-\d+(\.\d+)?"$/);
124+
});
125+
126+
it("falls back to mtime etag for assets without hash suffix", async () => {
127+
await writeFile(clientDir, "assets/logo.svg", "<svg></svg>");
128+
129+
const cache = await StaticFileCache.create(clientDir);
130+
const entry = cache.lookup("/assets/logo.svg");
131+
132+
// No hash in filename — falls back to mtime-based ETag
114133
expect(entry!.etag).toMatch(/^W\/"\d+-\d+(\.\d+)?"$/);
115134
});
116135

0 commit comments

Comments
 (0)