fix(core): don't noop shared modules (#9828)

* fix(core): don't noop shared modules

* address feedback

* add test

* changeset

* check astro pages

* address feedback

---------

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>

.changeset/real-dodos-worry.md

@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes a case where shared modules among pages and middleware were transformed to a no-op after the build.

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

@@ -56,6 +56,7 @@ export async function collectPagesData(
 				propagatedStyles: new Map(),
 				propagatedScripts: new Map(),
 				hoistedScript: undefined,
+				hasSharedModules: false,
 			};
 
 			clearInterval(routeCollectionLogTimeout);
@@ -80,6 +81,7 @@ export async function collectPagesData(
 			propagatedStyles: new Map(),
 			propagatedScripts: new Map(),
 			hoistedScript: undefined,
+			hasSharedModules: false,
 		};
 	}
 

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

@@ -18,10 +18,58 @@ function vitePluginPrerender(opts: StaticBuildOptions, internals: BuildInternals
 						return 'astro';
 					}
 					const pageInfo = internals.pagesByViteID.get(id);
+					let hasSharedModules = false;
 					if (pageInfo) {
 						// prerendered pages should be split into their own chunk
 						// Important: this can't be in the `pages/` directory!
 						if (getPrerenderMetadata(meta.getModuleInfo(id)!)) {
+							const infoMeta = meta.getModuleInfo(id)!;
+
+							// Here, we check if this page is importing modules that are shared among other modules e.g. middleware, other SSR pages, etc.
+							// we loop the modules that the current page imports
+							for (const moduleId of infoMeta.importedIds) {
+								// we retrieve the metadata of the module
+								const moduleMeta = meta.getModuleInfo(moduleId)!;
+								if (
+									// a shared modules should be inside the `src/` folder, at least
+									moduleMeta.id.startsWith(opts.settings.config.srcDir.pathname) &&
+									// and has at least two importers: the current page and something else
+									moduleMeta.importers.length > 1
+								) {
+									// Now, we have to trace back the modules imported and analyze them;
+									// understanding if a module is eventually shared between two pages isn't easy, because a module could
+									// be imported by a page and a component that is eventually imported by a page.
+									//
+									// Given the previous statement, we only check if
+									// - the module is a page, and it's not pre-rendered
+									// - the module is the middleware
+									// If one of these conditions is met, we need a separate chunk
+									for (const importer of moduleMeta.importedIds) {
+										// we don't want to analyze the same module again, so we skip it
+										if (importer !== id) {
+											const importerModuleMeta = meta.getModuleInfo(importer);
+											if (importerModuleMeta) {
+												// if the module is inside the pages
+												if (importerModuleMeta.id.includes('/pages')) {
+													// we check if it's not pre-rendered
+													if (getPrerenderMetadata(importerModuleMeta) === false) {
+														hasSharedModules = true;
+														break;
+													}
+												}
+												// module isn't an Astro route/page, it could be a middleware
+												else if (importerModuleMeta.id.includes('/middleware')) {
+													hasSharedModules = true;
+													break;
+												}
+											}
+										}
+									}
+								}
+							}
+
+							opts.allPages;
+							pageInfo.hasSharedModules = hasSharedModules;
 							pageInfo.route.prerender = true;
 							return 'prerender';
 						}

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

@@ -371,7 +371,7 @@ async function cleanStaticOutput(
 ) {
 	const allStaticFiles = new Set();
 	for (const pageData of eachPageData(internals)) {
-		if (pageData.route.prerender) {
+		if (pageData.route.prerender && !pageData.hasSharedModules) {
 			const { moduleSpecifier } = pageData;
 			const pageBundleId = internals.pageToBundleMap.get(moduleSpecifier);
 			const entryBundleId = internals.entrySpecifierToBundleMap.get(moduleSpecifier);

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

@@ -29,6 +29,7 @@ export interface PageBuildData {
 	propagatedScripts: Map<string, Set<string>>;
 	hoistedScript: { type: 'inline' | 'external'; value: string } | undefined;
 	styles: Array<{ depth: number; order: number; sheet: StylesheetAsset }>;
+	hasSharedModules: boolean;
 }
 
 export type AllPagesData = Record<ComponentPath, PageBuildData>;

packages/integrations/node/test/fixtures/prerender/src/middleware.ts

@@ -0,0 +1,7 @@
+import { shared } from './shared';
+export const onRequest = (ctx, next) => {
+	ctx.locals = {
+		name: shared,
+	};
+	return next();
+};

packages/integrations/node/test/fixtures/prerender/src/pages/third.astro

@@ -0,0 +1,15 @@
+---
+import { shared} from "../shared";
+export const prerender = false;
+
+const shared = Astro.locals.name;
+---
+
+<html>
+<head>
+	<title>One</title>
+</head>
+<body>
+<h1>{shared}</h1>
+</body>
+</html>

packages/integrations/node/test/fixtures/prerender/src/shared.ts

@@ -0,0 +1 @@
+export const shared = 'shared';

packages/integrations/node/test/prerender.test.js

@@ -201,7 +201,7 @@ describe('Hybrid rendering', () => {
 				adapter: nodejs({ mode: 'standalone' }),
 			});
 			await fixture.build();
-			const { startServer } = await await load();
+			const { startServer } = await load();
 			let res = startServer();
 			server = res.server;
 			await waitServerListen(server.server);
@@ -267,7 +267,7 @@ describe('Hybrid rendering', () => {
 				adapter: nodejs({ mode: 'standalone' }),
 			});
 			await fixture.build();
-			const { startServer } = await await load();
+			const { startServer } = await load();
 			let res = startServer();
 			server = res.server;
 			await waitServerListen(server.server);
@@ -315,4 +315,35 @@ describe('Hybrid rendering', () => {
 			assert.equal($('h1').text(), 'One');
 		});
 	});
+
+	describe('Shared modules', async () => {
+		before(async () => {
+			process.env.PRERENDER = false;
+
+			fixture = await loadFixture({
+				root: './fixtures/prerender/',
+				output: 'hybrid',
+				adapter: nodejs({ mode: 'standalone' }),
+			});
+			await fixture.build();
+			const { startServer } = await load();
+			let res = startServer();
+			server = res.server;
+		});
+
+		after(async () => {
+			await server.stop();
+			await fixture.clean();
+			delete process.env.PRERENDER;
+		});
+
+		it('Can render SSR route', async () => {
+			const res = await fetch(`http://${server.host}:${server.port}/third`);
+			const html = await res.text();
+			const $ = cheerio.load(html);
+
+			expect(res.status).to.equal(200);
+			expect($('h1').text()).to.equal('shared');
+		});
+	});
 });