Skip to content

Commit 4728fc1

Browse files
logaretmlogaretm
authored
fix(vite): Skip code injection for HTML facade chunks (#830)
* test: added testing fixtures * fix: skip code injection in facade and empty code chunks * refactor(tests): tighten assertions and remove unused console.log * refactor: extract file type check * style: format fixture code
1 parent ef6f4bb commit 4728fc1

File tree

8 files changed

+312
-80
lines changed

8 files changed

+312
-80
lines changed

‎packages/bundler-plugin-core/src/index.ts‎

Lines changed: 116 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -206,44 +206,80 @@ type RenderChunkHook = (
206206
code: string,
207207
chunk: {
208208
fileName: string;
209+
facadeModuleId?: string | null;
209210
}
210211
) => {
211212
code: string;
212213
map: SourceMap;
213214
} | null;
214215

216+
/**
217+
* Checks if a file is a JavaScript file based on its extension.
218+
* Handles query strings and hashes in the filename.
219+
*/
220+
function isJsFile(fileName: string): boolean {
221+
const cleanFileName = stripQueryAndHashFromPath(fileName);
222+
return [".js", ".mjs", ".cjs"].some((ext) => cleanFileName.endsWith(ext));
223+
}
224+
225+
/**
226+
* Checks if a chunk should be skipped for code injection
227+
*
228+
* This is necessary to handle Vite's MPA (multi-page application) mode where
229+
* HTML entry points create "facade" chunks that should not contain injected code.
230+
* See: https://github.com/getsentry/sentry-javascript-bundler-plugins/issues/829
231+
*
232+
* @param code - The chunk's code content
233+
* @param facadeModuleId - The facade module ID (if any) - HTML files create facade chunks
234+
* @returns true if the chunk should be skipped
235+
*/
236+
function shouldSkipCodeInjection(code: string, facadeModuleId: string | null | undefined): boolean {
237+
// Skip empty chunks - these are placeholder chunks that should be optimized away
238+
if (code.trim().length === 0) {
239+
return true;
240+
}
241+
242+
// Skip HTML facade chunks
243+
// They only contain import statements and should not have Sentry code injected
244+
if (facadeModuleId && stripQueryAndHashFromPath(facadeModuleId).endsWith(".html")) {
245+
return true;
246+
}
247+
248+
return false;
249+
}
250+
215251
export function createRollupReleaseInjectionHooks(injectionCode: string): {
216252
renderChunk: RenderChunkHook;
217253
} {
218254
return {
219-
renderChunk(code: string, chunk: { fileName: string }) {
220-
if (
221-
// chunks could be any file (html, md, ...)
222-
[".js", ".mjs", ".cjs"].some((ending) =>
223-
stripQueryAndHashFromPath(chunk.fileName).endsWith(ending)
224-
)
225-
) {
226-
const ms = new MagicString(code, { filename: chunk.fileName });
255+
renderChunk(code: string, chunk: { fileName: string; facadeModuleId?: string | null }) {
256+
if (!isJsFile(chunk.fileName)) {
257+
return null; // returning null means not modifying the chunk at all
258+
}
227259

228-
const match = code.match(COMMENT_USE_STRICT_REGEX)?.[0];
260+
// Skip empty chunks and HTML facade chunks (Vite MPA)
261+
if (shouldSkipCodeInjection(code, chunk.facadeModuleId)) {
262+
return null;
263+
}
229264

230-
if (match) {
231-
// Add injected code after any comments or "use strict" at the beginning of the bundle.
232-
ms.appendLeft(match.length, injectionCode);
233-
} else {
234-
// ms.replace() doesn't work when there is an empty string match (which happens if
235-
// there is neither, a comment, nor a "use strict" at the top of the chunk) so we
236-
// need this special case here.
237-
ms.prepend(injectionCode);
238-
}
265+
const ms = new MagicString(code, { filename: chunk.fileName });
239266

240-
return {
241-
code: ms.toString(),
242-
map: ms.generateMap({ file: chunk.fileName, hires: "boundary" }),
243-
};
267+
const match = code.match(COMMENT_USE_STRICT_REGEX)?.[0];
268+
269+
if (match) {
270+
// Add injected code after any comments or "use strict" at the beginning of the bundle.
271+
ms.appendLeft(match.length, injectionCode);
244272
} else {
245-
return null; // returning null means not modifying the chunk at all
273+
// ms.replace() doesn't work when there is an empty string match (which happens if
274+
// there is neither, a comment, nor a "use strict" at the top of the chunk) so we
275+
// need this special case here.
276+
ms.prepend(injectionCode);
246277
}
278+
279+
return {
280+
code: ms.toString(),
281+
map: ms.generateMap({ file: chunk.fileName, hires: "boundary" }),
282+
};
247283
},
248284
};
249285
}
@@ -262,48 +298,48 @@ export function createRollupDebugIdInjectionHooks(): {
262298
renderChunk: RenderChunkHook;
263299
} {
264300
return {
265-
renderChunk(code: string, chunk: { fileName: string }) {
301+
renderChunk(code: string, chunk: { fileName: string; facadeModuleId?: string | null }) {
302+
if (!isJsFile(chunk.fileName)) {
303+
return null; // returning null means not modifying the chunk at all
304+
}
305+
306+
// Skip empty chunks and HTML facade chunks (Vite MPA)
307+
if (shouldSkipCodeInjection(code, chunk.facadeModuleId)) {
308+
return null;
309+
}
310+
311+
// Check if a debug ID has already been injected to avoid duplicate injection (e.g. by another plugin or Sentry CLI)
312+
const chunkStartSnippet = code.slice(0, 6000);
313+
const chunkEndSnippet = code.slice(-500);
314+
266315
if (
267-
// chunks could be any file (html, md, ...)
268-
[".js", ".mjs", ".cjs"].some((ending) =>
269-
stripQueryAndHashFromPath(chunk.fileName).endsWith(ending)
270-
)
316+
chunkStartSnippet.includes("_sentryDebugIdIdentifier") ||
317+
chunkEndSnippet.includes("//# debugId=")
271318
) {
272-
// Check if a debug ID has already been injected to avoid duplicate injection (e.g. by another plugin or Sentry CLI)
273-
const chunkStartSnippet = code.slice(0, 6000);
274-
const chunkEndSnippet = code.slice(-500);
275-
276-
if (
277-
chunkStartSnippet.includes("_sentryDebugIdIdentifier") ||
278-
chunkEndSnippet.includes("//# debugId=")
279-
) {
280-
return null; // Debug ID already present, skip injection
281-
}
282-
283-
const debugId = stringToUUID(code); // generate a deterministic debug ID
284-
const codeToInject = getDebugIdSnippet(debugId);
319+
return null; // Debug ID already present, skip injection
320+
}
285321

286-
const ms = new MagicString(code, { filename: chunk.fileName });
322+
const debugId = stringToUUID(code); // generate a deterministic debug ID
323+
const codeToInject = getDebugIdSnippet(debugId);
287324

288-
const match = code.match(COMMENT_USE_STRICT_REGEX)?.[0];
325+
const ms = new MagicString(code, { filename: chunk.fileName });
289326

290-
if (match) {
291-
// Add injected code after any comments or "use strict" at the beginning of the bundle.
292-
ms.appendLeft(match.length, codeToInject);
293-
} else {
294-
// ms.replace() doesn't work when there is an empty string match (which happens if
295-
// there is neither, a comment, nor a "use strict" at the top of the chunk) so we
296-
// need this special case here.
297-
ms.prepend(codeToInject);
298-
}
327+
const match = code.match(COMMENT_USE_STRICT_REGEX)?.[0];
299328

300-
return {
301-
code: ms.toString(),
302-
map: ms.generateMap({ file: chunk.fileName, hires: "boundary" }),
303-
};
329+
if (match) {
330+
// Add injected code after any comments or "use strict" at the beginning of the bundle.
331+
ms.appendLeft(match.length, codeToInject);
304332
} else {
305-
return null; // returning null means not modifying the chunk at all
333+
// ms.replace() doesn't work when there is an empty string match (which happens if
334+
// there is neither, a comment, nor a "use strict" at the top of the chunk) so we
335+
// need this special case here.
336+
ms.prepend(codeToInject);
306337
}
338+
339+
return {
340+
code: ms.toString(),
341+
map: ms.generateMap({ file: chunk.fileName, hires: "boundary" }),
342+
};
307343
},
308344
};
309345
}
@@ -312,34 +348,34 @@ export function createRollupModuleMetadataInjectionHooks(injectionCode: string):
312348
renderChunk: RenderChunkHook;
313349
} {
314350
return {
315-
renderChunk(code: string, chunk: { fileName: string }) {
316-
if (
317-
// chunks could be any file (html, md, ...)
318-
[".js", ".mjs", ".cjs"].some((ending) =>
319-
stripQueryAndHashFromPath(chunk.fileName).endsWith(ending)
320-
)
321-
) {
322-
const ms = new MagicString(code, { filename: chunk.fileName });
351+
renderChunk(code: string, chunk: { fileName: string; facadeModuleId?: string | null }) {
352+
if (!isJsFile(chunk.fileName)) {
353+
return null; // returning null means not modifying the chunk at all
354+
}
323355

324-
const match = code.match(COMMENT_USE_STRICT_REGEX)?.[0];
356+
// Skip empty chunks and HTML facade chunks (Vite MPA)
357+
if (shouldSkipCodeInjection(code, chunk.facadeModuleId)) {
358+
return null;
359+
}
325360

326-
if (match) {
327-
// Add injected code after any comments or "use strict" at the beginning of the bundle.
328-
ms.appendLeft(match.length, injectionCode);
329-
} else {
330-
// ms.replace() doesn't work when there is an empty string match (which happens if
331-
// there is neither, a comment, nor a "use strict" at the top of the chunk) so we
332-
// need this special case here.
333-
ms.prepend(injectionCode);
334-
}
361+
const ms = new MagicString(code, { filename: chunk.fileName });
335362

336-
return {
337-
code: ms.toString(),
338-
map: ms.generateMap({ file: chunk.fileName, hires: "boundary" }),
339-
};
363+
const match = code.match(COMMENT_USE_STRICT_REGEX)?.[0];
364+
365+
if (match) {
366+
// Add injected code after any comments or "use strict" at the beginning of the bundle.
367+
ms.appendLeft(match.length, injectionCode);
340368
} else {
341-
return null; // returning null means not modifying the chunk at all
369+
// ms.replace() doesn't work when there is an empty string match (which happens if
370+
// there is neither, a comment, nor a "use strict" at the top of the chunk) so we
371+
// need this special case here.
372+
ms.prepend(injectionCode);
342373
}
374+
375+
return {
376+
code: ms.toString(),
377+
map: ms.generateMap({ file: chunk.fileName, hires: "boundary" }),
378+
};
343379
},
344380
};
345381
}

‎packages/integration-tests/fixtures/vite-mpa-extra-modules/build-vite-with-plugin.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { sentryVitePlugin } from "@sentry/vite-plugin";
2+
import * as path from "path";
3+
import * as vite from "vite";
4+
5+
const inputDir = path.join(__dirname, "input");
6+
7+
void vite.build({
8+
clearScreen: false,
9+
root: inputDir,
10+
build: {
11+
sourcemap: true,
12+
outDir: path.join(__dirname, "out", "with-plugin"),
13+
emptyOutDir: true,
14+
rollupOptions: {
15+
input: {
16+
index: path.join(inputDir, "index.html"),
17+
page1: path.join(inputDir, "page1.html"),
18+
page2: path.join(inputDir, "page2.html"),
19+
},
20+
},
21+
},
22+
plugins: [
23+
sentryVitePlugin({
24+
telemetry: false,
25+
// Empty options - the issue says options don't affect the results
26+
}),
27+
],
28+
});

‎packages/integration-tests/fixtures/vite-mpa-extra-modules/build-vite-without-plugin.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import * as path from "path";
2+
import * as vite from "vite";
3+
4+
const inputDir = path.join(__dirname, "input");
5+
6+
void vite.build({
7+
clearScreen: false,
8+
root: inputDir,
9+
build: {
10+
sourcemap: true,
11+
outDir: path.join(__dirname, "out", "without-plugin"),
12+
emptyOutDir: true,
13+
rollupOptions: {
14+
input: {
15+
index: path.join(inputDir, "index.html"),
16+
page1: path.join(inputDir, "page1.html"),
17+
page2: path.join(inputDir, "page2.html"),
18+
},
19+
},
20+
},
21+
plugins: [],
22+
});

‎packages/integration-tests/fixtures/vite-mpa-extra-modules/input/index.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Index Page</title>
6+
</head>
7+
<body>
8+
<h1>Index Page - No Scripts</h1>
9+
<!-- This page has no scripts -->
10+
</body>
11+
</html>

‎packages/integration-tests/fixtures/vite-mpa-extra-modules/input/page1.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Page 1</title>
6+
</head>
7+
<body>
8+
<h1>Page 1 - With Shared Module</h1>
9+
<script type="module" src="./shared-module.js"></script>
10+
</body>
11+
</html>

‎packages/integration-tests/fixtures/vite-mpa-extra-modules/input/page2.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Page 2</title>
6+
</head>
7+
<body>
8+
<h1>Page 2 - With Shared Module</h1>
9+
<script type="module" src="./shared-module.js"></script>
10+
</body>
11+
</html>

‎packages/integration-tests/fixtures/vite-mpa-extra-modules/input/shared-module.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// This is a shared module that is used by multiple HTML pages
2+
export function greet(name) {
3+
// eslint-disable-next-line no-console
4+
console.log(`Hello, ${String(name)}!`);
5+
}
6+
7+
export const VERSION = "1.0.0";
8+
9+
// Side effect: greet on load
10+
greet("World");

0 commit comments

Comments
 (0)