Fix hoisted script propagation in content collection pages (#6119)

* chore: unskip

* chore: stray console logs

* chore: clarify analyzer comments

* wip: store propagated scripts separately

* Implement getting hoisted scripts for propagation

* Add changeset

* fix

* oops

* update based on code review

---------

Co-authored-by: bholmesdev <hey@bholmes.dev>

.changeset/shaggy-pianos-sleep.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix hoisted script propagation in content collection pages

packages/astro/src/content/index.ts

@@ -3,7 +3,6 @@ export { createContentTypesGenerator } from './types-generator.js';
 export { contentObservable, getContentPaths, getDotAstroTypeReference } from './utils.js';
 export {
 	astroContentAssetPropagationPlugin,
-	astroContentProdBundlePlugin,
 } from './vite-plugin-content-assets.js';
 export { astroContentImportPlugin } from './vite-plugin-content-imports.js';
 export { astroContentVirtualModPlugin } from './vite-plugin-content-virtual-mod.js';

packages/astro/src/content/vite-plugin-content-assets.ts

@@ -1,4 +1,5 @@
 import { pathToFileURL } from 'url';
+import npath from 'node:path';
 import type { Plugin } from 'vite';
 import { moduleIsTopLevelPage, walkParentInfos } from '../core/build/graph.js';
 import { BuildInternals, getPageDataByViteID } from '../core/build/internal.js';
@@ -14,6 +15,8 @@ import {
 	SCRIPTS_PLACEHOLDER,
 	STYLES_PLACEHOLDER,
 } from './consts.js';
+import type { RollupOutput, OutputChunk, StaticBuildOptions } from '../core/build/types';
+import { prependForwardSlash } from '../core/path.js';
 
 function isPropagatedAsset(viteId: string): boolean {
 	const url = new URL(viteId, 'file://');
@@ -73,42 +76,95 @@ export function astroContentAssetPropagationPlugin({ mode }: { mode: string }):
 	};
 }
 
-export function astroContentProdBundlePlugin({ internals }: { internals: BuildInternals }): Plugin {
+export function astroConfigBuildPlugin(options: StaticBuildOptions, internals: BuildInternals): AstroBuildPlugin {
+	let ssrPluginContext: any = undefined;
 	return {
-		name: 'astro:content-prod-bundle',
-		async generateBundle(_options, bundle) {
-			for (const [_, chunk] of Object.entries(bundle)) {
-				if (chunk.type === 'chunk' && chunk.code.includes(LINKS_PLACEHOLDER)) {
-					for (const id of Object.keys(chunk.modules)) {
-						for (const [pageInfo, depth, order] of walkParentInfos(id, this)) {
-							if (moduleIsTopLevelPage(pageInfo)) {
-								const pageViteID = pageInfo.id;
-								const pageData = getPageDataByViteID(internals, pageViteID);
-								if (!pageData) continue;
-								const entryCss = pageData.contentCollectionCss?.get(id);
-								if (!entryCss) continue;
-								chunk.code = chunk.code.replace(
-									JSON.stringify(LINKS_PLACEHOLDER),
-									JSON.stringify([...entryCss])
-								);
+		build: 'ssr',
+		hooks: {
+			'build:before': ({ build }) => {
+				return {
+					vitePlugin: {
+						name: 'astro:content-build-plugin',
+						generateBundle() {
+							if(build === 'ssr') {
+								ssrPluginContext = this;
 							}
 						}
+					},
+				};
+			},
+			'build:post': ({ ssrOutputs, clientOutputs, mutate }) => {
+				const outputs = ssrOutputs.flatMap(o => o.output);
+				for (const chunk of outputs) {
+					if (
+						chunk.type === 'chunk' &&
+						(chunk.code.includes(LINKS_PLACEHOLDER) || chunk.code.includes(SCRIPTS_PLACEHOLDER))
+					) {
+						let entryCSS = new Set<string>();
+						let entryScripts = new Set<string>();
+
+						for (const id of Object.keys(chunk.modules)) {
+							for (const [pageInfo] of walkParentInfos(id, ssrPluginContext)) {
+								if (moduleIsTopLevelPage(pageInfo)) {
+									const pageViteID = pageInfo.id;
+									const pageData = getPageDataByViteID(internals, pageViteID);
+									if (!pageData) continue;
+	
+									const _entryCss = pageData.propagatedStyles?.get(id);
+									const _entryScripts = pageData.propagatedScripts?.get(id);
+									if(_entryCss) {
+										for(const value of _entryCss) {
+											entryCSS.add(value);
+										}
+									}
+									if(_entryScripts) {
+										for(const value of _entryScripts) {
+											entryScripts.add(value);
+										}
+									}
+								}
+							}
+						}
+
+						let newCode = chunk.code;
+						if (entryCSS.size) {
+							newCode = newCode.replace(
+								JSON.stringify(LINKS_PLACEHOLDER),
+								JSON.stringify([...entryCSS])
+							);
+						}
+						if (entryScripts.size) {
+							const entryFileNames = new Set<string>();
+							for(const output of clientOutputs) {
+								for(const clientChunk of output.output) {
+									if(clientChunk.type !== 'chunk') continue;
+									for(const [id] of Object.entries(clientChunk.modules)) {
+										if(entryScripts.has(id)) {
+											entryFileNames.add(clientChunk.fileName);
+										}
+									}
+								}
+							}
+							newCode = newCode.replace(
+								JSON.stringify(SCRIPTS_PLACEHOLDER),
+								JSON.stringify(
+									[...entryFileNames].map((src) => ({
+										props: {
+											src: prependForwardSlash(npath.posix.join(
+												options.settings.config.base,
+												src
+											)),
+											type: 'module'
+										},
+										children: '',
+									}))
+								)
+							);
+						}
+						mutate(chunk, 'server', newCode);
 					}
 				}
 			}
 		},
 	};
 }
-
-export function astroConfigBuildPlugin(internals: BuildInternals): AstroBuildPlugin {
-	return {
-		build: 'ssr',
-		hooks: {
-			'build:before': () => {
-				return {
-					vitePlugin: astroContentProdBundlePlugin({ internals }),
-				};
-			},
-		},
-	};
-}

packages/astro/src/core/build/graph.ts

@@ -40,7 +40,7 @@ export function moduleIsTopLevelPage(info: ModuleInfo): boolean {
 // This could be a .astro page, a .markdown or a .md (or really any file extension for markdown files) page.
 export function* getTopLevelPages(
 	id: string,
-	ctx: { getModuleInfo: GetModuleInfo }
+	ctx: { getModuleInfo: GetModuleInfo },
 ): Generator<[ModuleInfo, number, number], void, unknown> {
 	for (const res of walkParentInfos(id, ctx)) {
 		if (moduleIsTopLevelPage(res[0])) {

packages/astro/src/core/build/page-data.ts

@@ -54,7 +54,8 @@ export async function collectPagesData(
 				route,
 				moduleSpecifier: '',
 				css: new Map(),
-				contentCollectionCss: new Map(),
+				propagatedStyles: new Map(),
+				propagatedScripts: new Map(),
 				hoistedScript: undefined,
 			};
 
@@ -76,7 +77,8 @@ export async function collectPagesData(
 			route,
 			moduleSpecifier: '',
 			css: new Map(),
-			contentCollectionCss: new Map(),
+			propagatedStyles: new Map(),
+			propagatedScripts: new Map(),
 			hoistedScript: undefined,
 		};
 	}

packages/astro/src/core/build/plugins/index.ts

@@ -16,7 +16,7 @@ export function registerAllPlugins({ internals, options, register }: AstroBuildP
 	register(pluginPages(options, internals));
 	register(pluginCSS(options, internals));
 	register(pluginPrerender(options, internals));
-	register(astroConfigBuildPlugin(internals));
+	register(astroConfigBuildPlugin(options, internals));
 	register(pluginHoistedScripts(options, internals));
 	register(pluginSSR(options, internals));
 }

packages/astro/src/core/build/plugins/plugin-analyzer.ts

@@ -4,14 +4,29 @@ import type { PluginMetadata as AstroPluginMetadata } from '../../../vite-plugin
 import type { BuildInternals } from '../internal.js';
 import type { AstroBuildPlugin } from '../plugin.js';
 
-import { prependForwardSlash } from '../../path.js';
-import { getTopLevelPages } from '../graph.js';
+import { prependForwardSlash } from '../../../core/path.js';
+import { getTopLevelPages, moduleIsTopLevelPage, walkParentInfos } from '../graph.js';
 import { getPageDataByViteID, trackClientOnlyPageDatas } from '../internal.js';
+import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js';
+
+function isPropagatedAsset(id: string) {
+	try {
+		return new URL('file://' + id).searchParams.has(PROPAGATED_ASSET_FLAG)
+	} catch {
+		return false;
+	}
+}
 
 export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
 	function hoistedScriptScanner() {
 		const uniqueHoistedIds = new Map<string, string>();
-		const pageScripts = new Map<string, Set<string>>();
+		const pageScripts = new Map<
+			string,
+			{
+				hoistedSet: Set<string>;
+				propagatedMapByImporter: Map<string, Set<string>>;
+			}
+		>();
 
 		return {
 			scan(this: PluginContext, scripts: AstroPluginMetadata['astro']['scripts'], from: string) {
@@ -22,13 +37,36 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
 				}
 
 				if (hoistedScripts.size) {
-					for (const [pageInfo] of getTopLevelPages(from, this)) {
-						const pageId = pageInfo.id;
-						for (const hid of hoistedScripts) {
-							if (pageScripts.has(pageId)) {
-								pageScripts.get(pageId)?.add(hid);
-							} else {
-								pageScripts.set(pageId, new Set([hid]));
+					for (const [parentInfo] of walkParentInfos(from, this, function until(importer) {
+						return isPropagatedAsset(importer);
+					})) {
+						if (isPropagatedAsset(parentInfo.id)) {
+							for (const [nestedParentInfo] of walkParentInfos(from, this)) {
+								if (moduleIsTopLevelPage(nestedParentInfo)) {
+									for (const hid of hoistedScripts) {
+										if (!pageScripts.has(nestedParentInfo.id)) {
+											pageScripts.set(nestedParentInfo.id, {
+												hoistedSet: new Set(),
+												propagatedMapByImporter: new Map(),
+											});
+										}
+										const entry = pageScripts.get(nestedParentInfo.id)!;
+										if (!entry.propagatedMapByImporter.has(parentInfo.id)) {
+											entry.propagatedMapByImporter.set(parentInfo.id, new Set());
+										}
+										entry.propagatedMapByImporter.get(parentInfo.id)!.add(hid);
+									}
+								}
+							}
+						} else if (moduleIsTopLevelPage(parentInfo)) {
+							for (const hid of hoistedScripts) {
+								if (!pageScripts.has(parentInfo.id)) {
+									pageScripts.set(parentInfo.id, {
+										hoistedSet: new Set(),
+										propagatedMapByImporter: new Map(),
+									});
+								}
+								pageScripts.get(parentInfo.id)?.hoistedSet.add(hid);
 							}
 						}
 					}
@@ -36,14 +74,14 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
 			},
 
 			finalize() {
-				for (const [pageId, hoistedScripts] of pageScripts) {
+				for (const [pageId, { hoistedSet, propagatedMapByImporter }] of pageScripts) {
 					const pageData = getPageDataByViteID(internals, pageId);
 					if (!pageData) continue;
 
 					const { component } = pageData;
 					const astroModuleId = prependForwardSlash(component);
 
-					const uniqueHoistedId = JSON.stringify(Array.from(hoistedScripts).sort());
+					const uniqueHoistedId = JSON.stringify(Array.from(hoistedSet).sort());
 					let moduleId: string;
 
 					// If we're already tracking this set of hoisted scripts, get the unique id
@@ -56,13 +94,23 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
 					}
 					internals.discoveredScripts.add(moduleId);
 
+					pageData.propagatedScripts = propagatedMapByImporter;
+
+					// Add propagated scripts to client build,
+					// but DON'T add to pages -> hoisted script map.
+					for (const propagatedScripts of propagatedMapByImporter.values()) {
+						for (const propagatedScript of propagatedScripts) {
+							internals.discoveredScripts.add(propagatedScript);
+						}
+					}
+
 					// Make sure to track that this page uses this set of hoisted scripts
 					if (internals.hoistedScriptIdToPagesMap.has(moduleId)) {
 						const pages = internals.hoistedScriptIdToPagesMap.get(moduleId);
 						pages!.add(astroModuleId);
 					} else {
 						internals.hoistedScriptIdToPagesMap.set(moduleId, new Set([astroModuleId]));
-						internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedScripts);
+						internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedSet);
 					}
 				}
 			},
@@ -132,7 +180,7 @@ export function pluginAnalyzer(internals: BuildInternals): AstroBuildPlugin {
 				return {
 					vitePlugin: vitePluginAnalyzer(internals),
 				};
-			},
+			}
 		},
 	};
 }

packages/astro/src/core/build/plugins/plugin-css.ts

@@ -176,8 +176,8 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 													if (pageData) {
 														for (const css of meta.importedCss) {
 															const existingCss =
-																pageData.contentCollectionCss.get(pageInfo.id) ?? new Set();
-															pageData.contentCollectionCss.set(
+																pageData.propagatedStyles.get(pageInfo.id) ?? new Set();
+															pageData.propagatedStyles.set(
 																pageInfo.id,
 																new Set([...existingCss, css])
 															);

packages/astro/src/core/build/static-build.ts

@@ -245,9 +245,12 @@ async function runPostBuildHooks(
 	clientReturn: Awaited<ReturnType<typeof clientBuild>>
 ) {
 	const mutations = await container.runPostHook(ssrReturn, clientReturn);
+	const config = container.options.settings.config;
 	const buildConfig = container.options.settings.config.build;
 	for (const [fileName, mutation] of mutations) {
-		const root = mutation.build === 'server' ? buildConfig.server : buildConfig.client;
+		const root = config.output === 'server' ?
+			mutation.build === 'server' ? buildConfig.server : buildConfig.client :
+			config.outDir;
 		const fileURL = new URL(fileName, root);
 		await fs.promises.mkdir(new URL('./', fileURL), { recursive: true });
 		await fs.promises.writeFile(fileURL, mutation.code, 'utf-8');

packages/astro/src/core/build/types.ts

@@ -21,7 +21,8 @@ export interface PageBuildData {
 	route: RouteData;
 	moduleSpecifier: string;
 	css: Map<string, { depth: number; order: number }>;
-	contentCollectionCss: Map<string, Set<string>>;
+	propagatedStyles: Map<string, Set<string>>;
+	propagatedScripts: Map<string, Set<string>>;
 	hoistedScript: { type: 'inline' | 'external'; value: string } | undefined;
 }
 export type AllPagesData = Record<ComponentPath, PageBuildData>;

packages/astro/test/content-collections-render.test.js

@@ -46,7 +46,7 @@ describe('Content Collections - render()', () => {
 			// Includes hoisted script
 			expect(
 				[...allScripts].find((script) =>
-					$(script).text().includes('document.querySelector("#update-me")')
+					$(script).attr('src')?.includes('WithScripts')
 				),
 				'`WithScripts.astro` hoisted script missing from head.'
 			).to.not.be.undefined;
@@ -55,9 +55,7 @@ describe('Content Collections - render()', () => {
 			expect($('script[data-is-inline]')).to.have.a.lengthOf(1);
 		});
 
-		// TODO: Script bleed isn't solved for prod builds.
-		// Tackling in separate PR.
-		it.skip('Excludes component scripts for non-rendered entries', async () => {
+		it('Excludes component scripts for non-rendered entries', async () => {
 			const html = await fixture.readFile('/index.html');
 			const $ = cheerio.load(html);