Fix script injection during build (#12392)

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
withastro · Nov 14, 2024 · 0462219 · 0462219
1 parent 3b3bc9b
commit 0462219
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 37 deletions.
diff --git a/.changeset/slimy-pets-lick.md b/.changeset/slimy-pets-lick.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes an issue where scripts were not correctly injected during the build. The issue was triggered when there were injected routes with the same `entrypoint` and different `pattern`
diff --git a/packages/astro/src/core/build/plugins/index.ts b/packages/astro/src/core/build/plugins/index.ts
@@ -32,7 +32,7 @@ export function registerAllPlugins({ internals, options, register }: AstroBuildP
 	if (options.settings.config.experimental.directRenderScript) {
 		register(pluginScripts(internals));
 	} else {
-		register(pluginHoistedScripts(options, internals));
+		register(pluginHoistedScripts(internals));
 	}
 	register(pluginSSR(options, internals));
 	register(pluginSSRSplit(options, internals));

diff --git a/packages/astro/src/core/build/plugins/plugin-hoisted-scripts.ts b/packages/astro/src/core/build/plugins/plugin-hoisted-scripts.ts
@@ -1,18 +1,14 @@
 import type { BuildOptions, Rollup, Plugin as VitePlugin } from 'vite';
-import type { AstroSettings } from '../../../@types/astro.js';
-import { viteID } from '../../util.js';
 import type { BuildInternals } from '../internal.js';
-import { getPageDataByViteID } from '../internal.js';
+import { getPageDatasByHoistedScriptId } from '../internal.js';
 import type { AstroBuildPlugin } from '../plugin.js';
-import type { StaticBuildOptions } from '../types.js';
 import { shouldInlineAsset } from './util.js';
 
 function virtualHoistedEntry(id: string) {
 	return id.startsWith('/astro/hoisted.js?q=');
 }
 
 export function vitePluginHoistedScripts(
-	settings: AstroSettings,
 	internals: BuildInternals,
 ): VitePlugin {
 	let assetsInlineLimit: NonNullable<BuildOptions['assetsInlineLimit']>;
@@ -73,23 +69,18 @@ export function vitePluginHoistedScripts(
 					shouldInlineAsset(output.code, output.fileName, assetsInlineLimit);
 				let removeFromBundle = false;
 				const facadeId = output.facadeModuleId!;
-				const pages = internals.hoistedScriptIdToPagesMap.get(facadeId)!;
-				for (const pathname of pages) {
-					const vid = viteID(new URL('.' + pathname, settings.config.root));
-					const pageInfo = getPageDataByViteID(internals, vid);
-					if (pageInfo) {
-						if (canBeInlined) {
-							pageInfo.hoistedScript = {
-								type: 'inline',
-								value: output.code,
-							};
-							removeFromBundle = true;
-						} else {
-							pageInfo.hoistedScript = {
-								type: 'external',
-								value: id,
-							};
-						}
+				for (const pageData of getPageDatasByHoistedScriptId(internals, facadeId)) {
+					if (canBeInlined) {
+						pageData.hoistedScript = {
+							type: 'inline',
+							value: output.code,
+						};
+						removeFromBundle = true;
+					} else {
+						pageData.hoistedScript = {
+							type: 'external',
+							value: id,
+						};
 					}
 				}
 
@@ -103,15 +94,14 @@ export function vitePluginHoistedScripts(
 }
 
 export function pluginHoistedScripts(
-	options: StaticBuildOptions,
 	internals: BuildInternals,
 ): AstroBuildPlugin {
 	return {
 		targets: ['client'],
 		hooks: {
 			'build:before': () => {
 				return {
-					vitePlugin: vitePluginHoistedScripts(options.settings, internals),
+					vitePlugin: vitePluginHoistedScripts(internals),
 				};
 			},
 		},

diff --git a/packages/astro/test/fixtures/reuse-injected-entrypoint/src/to-inject.astro b/packages/astro/test/fixtures/reuse-injected-entrypoint/src/to-inject.astro
@@ -1,12 +1,6 @@
 ---
 ---
-<html lang="en">
-	<head>
-		<meta charset="utf-8" />
-		<meta name="viewport" content="width=device-width" />
-		<title>Routing</title>
-	</head>
-	<body>
-		<h1>to-inject.astro</h1>
-	</body>
-</html>
+<h1>to-inject.astro</h1>
+<script>
+	console.log('to-inject.astro');
+</script>
diff --git a/packages/astro/test/reuse-injected-entrypoint.test.js b/packages/astro/test/reuse-injected-entrypoint.test.js
@@ -13,11 +13,13 @@ const routes = [
 		description: 'matches /injected-a to to-inject.astro',
 		url: '/injected-a',
 		h1: 'to-inject.astro',
+		scriptContent: 'console.log("to-inject.astro");',
 	},
 	{
 		description: 'matches /injected-b to to-inject.astro',
 		url: '/injected-b',
 		h1: 'to-inject.astro',
+		scriptContent: 'console.log("to-inject.astro");',
 	},
 	{
 		description: 'matches /dynamic-a/id-1 to [id].astro',
@@ -60,7 +62,7 @@ describe('Reuse injected entrypoint', () => {
 			await fixture.build();
 		});
 
-		routes.forEach(({ description, url, fourOhFour, h1, p, htmlMatch }) => {
+		routes.forEach(({ description, url, fourOhFour, h1, p, htmlMatch, scriptContent }) => {
 			const isEndpoint = htmlMatch && !h1 && !p;
 
 			it(description, async () => {
@@ -85,6 +87,15 @@ describe('Reuse injected entrypoint', () => {
 				if (htmlMatch) {
 					assert.equal(html, htmlMatch);
 				}
+
+				if (scriptContent) {
+					const scriptTags = $('script[type="module"]').toArray();
+					const scriptFound = scriptTags.some((script) => {
+						const scriptText = $(script).text();
+						return scriptText.includes(scriptContent.trim());
+					});
+					assert(scriptFound, `Expected script content to be injected in SSG ${url}`);
+				}
 			});
 		});
 	});
@@ -105,7 +116,7 @@ describe('Reuse injected entrypoint', () => {
 			await devServer.stop();
 		});
 
-		routes.forEach(({ description, url, fourOhFour, h1, p, htmlMatch }) => {
+		routes.forEach(({ description, url, fourOhFour, h1, p, htmlMatch, scriptContent }) => {
 			// checks URLs as written above
 			it(description, async () => {
 				const html = await fixture.fetch(url).then((res) => res.text());
@@ -127,6 +138,17 @@ describe('Reuse injected entrypoint', () => {
 				if (htmlMatch) {
 					assert.equal(html, htmlMatch);
 				}
+
+				if (scriptContent) {
+					const scriptTags = $('script[type="module"]').toArray();
+					const scriptFound = scriptTags.some((script) => {
+						const scriptSrc = $(script).attr('src');
+						return (
+							scriptSrc && scriptSrc.includes('/to-inject.astro?astro&type=script&index=0&lang.ts')
+						);
+					});
+					assert(scriptFound, `Expected script content to be injected in dev ${url}`);
+				}
 			});
 		});
 	});