fix(page-data): add key to allPages (#10625)

* fix(page-data): add key to allPages * add fryuni's test * replaced object.entries(allpages) * tmp: change pagesByComponents by pagesByKeys * fix pagesByKeys.get() in plugin-ssr & plugin-manifest * remove logs * remove useless generators * another useless generator * use null byte in key * tmp function in pipeline.ts * refactor getVirtualModulePageName * refactor getPageKeyFromVirtualModulePageName * clean & comments * better key and fix build * utils: add makePageDataKey * fix(pipeline): retrieveRoutesToGenerate for ssr * internals: getPageData function * tmp(ssr-split-manifest): fix test ? * fix?: ssr clean static output * internals: getPageDatasWithPublicKey * internals: getPageDatasByHoistedScriptId & getPagesDatasByComponent * remove broken & useless virtualModuleNameFromResolvedId * chore: changeset * fix: sanitize slashes in filepaths * Revert "fix: sanitize slashes in filepaths" This reverts commit 5c3a75f. * fix?: remove route from virtual module name * fix: concat & array.from * update changeset * clean unnecessary change * remove unnecessary pageInfo * add return types to utils functions * revert a comment deletion * fix cleanStaticOutput * changes from ematipico review * moving a todo outside jsdoc (cc @ematipico ) * Update .changeset/great-turtles-greet.md Co-authored-by: Sarah Rainsberger <[email protected]> * Update .changeset/great-turtles-greet.md Co-authored-by: Sarah Rainsberger <[email protected]> * chore: fix merge conflicts * fix: incorrect function * remove logs * revert: codepoint change --------- Co-authored-by: Princesseuh <[email protected]> Co-authored-by: Sarah Rainsberger <[email protected]> Co-authored-by: Emanuele Stoppa <[email protected]>
withastro · May 8, 2024 · 698c2d9 · 698c2d9
1 parent 61f47a6
commit 698c2d9
Show file tree

Hide file tree

Showing 20 changed files with 549 additions and 216 deletions.
diff --git a/.changeset/great-turtles-greet.md b/.changeset/great-turtles-greet.md
@@ -0,0 +1,5 @@
+---
+"astro": minor
+---
+
+Adds the ability for multiple pages to use the same component as an `entrypoint` when building an Astro integration. This change is purely internal, and aligns the build process with the behaviour in the development server.
diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -42,7 +42,7 @@ import { createRequest } from '../request.js';
 import { matchRoute } from '../routing/match.js';
 import { getOutputFilename, isServerLikeOutput } from '../util.js';
 import { getOutDirWithinCwd, getOutFile, getOutFolder } from './common.js';
-import { cssOrder, getPageDataByComponent, mergeInlineCss } from './internal.js';
+import { cssOrder, mergeInlineCss } from './internal.js';
 import { BuildPipeline } from './pipeline.js';
 import type {
 	PageBuildData,
@@ -51,6 +51,8 @@ import type {
 	StylesheetAsset,
 } from './types.js';
 import { getTimeStat, shouldAppendForwardSlash } from './util.js';
+import { getVirtualModulePageName } from './plugins/util.js';
+import { ASTRO_PAGE_MODULE_ID } from './plugins/plugin-pages.js';
 
 function createEntryURL(filePath: string, outFolder: URL) {
 	return new URL('./' + filePath + `?time=${Date.now()}`, outFolder);
@@ -200,7 +202,6 @@ async function generatePage(
 	// prepare information we need
 	const { config, internals, logger } = pipeline;
 	const pageModulePromise = ssrEntry.page;
-	const pageInfo = getPageDataByComponent(internals, pageData.route.component);
 
 	// Calculate information of the page, like scripts, links and styles
 	const styles = pageData.styles
@@ -209,7 +210,7 @@ async function generatePage(
 		.reduce(mergeInlineCss, []);
 	// may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc.
 	const linkIds: [] = [];
-	const scripts = pageInfo?.hoistedScript ?? null;
+	const scripts = pageData.hoistedScript ?? null;
 	if (!pageModulePromise) {
 		throw new Error(
 			`Unable to find the module for ${pageData.component}. This is unexpected and likely a bug in Astro, please report.`

diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts
@@ -3,13 +3,8 @@ import type { RouteData, SSRResult } from '../../@types/astro.js';
 import type { PageOptions } from '../../vite-plugin-astro/types.js';
 import { prependForwardSlash, removeFileExtension } from '../path.js';
 import { viteID } from '../util.js';
-import {
-	ASTRO_PAGE_RESOLVED_MODULE_ID,
-	getVirtualModulePageIdFromPath,
-} from './plugins/plugin-pages.js';
-import { RESOLVED_SPLIT_MODULE_ID } from './plugins/plugin-ssr.js';
-import { ASTRO_PAGE_EXTENSION_POST_PATTERN } from './plugins/util.js';
-import type { AllPagesData, PageBuildData, StylesheetAsset, ViteID } from './types.js';
+import type { PageBuildData, StylesheetAsset, ViteID } from './types.js';
+import { makePageDataKey } from './plugins/util.js';
 
 export interface BuildInternals {
 	/**
@@ -45,7 +40,7 @@ export interface BuildInternals {
 	/**
 	 * A map for page-specific information.
 	 */
-	pagesByComponent: Map<string, PageBuildData>;
+	pagesByKeys: Map<string, PageBuildData>;
 
 	/**
 	 * A map for page-specific output.
@@ -134,7 +129,7 @@ export function createBuildInternals(): BuildInternals {
 		inlinedScripts: new Map(),
 		entrySpecifierToBundleMap: new Map<string, string>(),
 		pageToBundleMap: new Map<string, string>(),
-		pagesByComponent: new Map(),
+		pagesByKeys: new Map(),
 		pageOptionsByPage: new Map(),
 		pagesByViteID: new Map(),
 		pagesByClientOnly: new Map(),
@@ -161,7 +156,7 @@ export function trackPageData(
 	componentURL: URL
 ): void {
 	pageData.moduleSpecifier = componentModuleId;
-	internals.pagesByComponent.set(component, pageData);
+	internals.pagesByKeys.set(pageData.key, pageData);
 	internals.pagesByViteID.set(viteID(componentURL), pageData);
 }
 
@@ -229,16 +224,77 @@ export function* getPageDatasByClientOnlyID(
 	}
 }
 
-export function getPageDataByComponent(
+/**
+ * From its route and component, get the page data from the build internals.
+ * @param internals Build Internals with all the pages
+ * @param route The route of the page, used to identify the page
+ * @param component The component of the page, used to identify the page
+ */
+export function getPageData(
 	internals: BuildInternals,
+	route: string, 
 	component: string
 ): PageBuildData | undefined {
-	if (internals.pagesByComponent.has(component)) {
-		return internals.pagesByComponent.get(component);
-	}
+	let pageData = internals.pagesByKeys.get(makePageDataKey(route, component));
+	if (pageData) { return pageData;}
 	return undefined;
 }
 
+/**
+ * Get all pages datas from the build internals, using a specific component.
+ * @param internals Build Internals with all the pages
+ * @param component path to the component, used to identify related pages
+ */
+export function getPagesDatasByComponent(
+	internals: BuildInternals,
+	component: string
+): PageBuildData[] {
+	const pageDatas: PageBuildData[] = [];
+		internals.pagesByKeys.forEach((pageData) => {
+			if (component === pageData.component) pageDatas.push(pageData);
+	})
+	return pageDatas;
+}
+
+// TODO: Should be removed in the future. (Astro 5?)
+/**
+ * Map internals.pagesByKeys to a new map with the public key instead of the internal key.
+ * This function is only used to avoid breaking changes in the Integrations API, after we changed the way 
+ * we identify pages, from the entrypoint component to an internal key.
+ * If the page component is unique -> the public key is the component path. (old behavior)
+ * If the page component is shared -> the public key is the internal key. (new behavior)
+ * The new behavior on shared entrypoint it's not a breaking change, because it was not supported before.
+ * @param pagesByKeys A map of all page data by their internal key
+ */
+export function getPageDatasWithPublicKey(pagesByKeys: Map<string, PageBuildData>): Map<string, PageBuildData> {
+	// Create a map to store the pages with the public key, mimicking internal.pagesByKeys
+	const pagesWithPublicKey = new Map<string, PageBuildData>();
+
+	const pagesByComponentsArray = Array.from(pagesByKeys.values()).map((pageData) => {
+		return { component: pageData.component, pageData: pageData };
+	});
+
+	// Get pages with unique component, and set the public key to the component.
+	const pagesWithUniqueComponent = pagesByComponentsArray.filter((page) => {
+		return pagesByComponentsArray.filter((p) => p.component === page.component).length === 1;
+	});
+
+	pagesWithUniqueComponent.forEach((page) => {
+		pagesWithPublicKey.set(page.component, page.pageData);
+	});
+
+	// Get pages with shared component, and set the public key to the internal key.
+	const pagesWithSharedComponent = pagesByComponentsArray.filter((page) => {
+		return pagesByComponentsArray.filter((p) => p.component === page.component).length > 1;
+	});
+
+	pagesWithSharedComponent.forEach((page) => {
+		pagesWithPublicKey.set(page.pageData.key, page.pageData);
+	});
+
+	return pagesWithPublicKey;
+}
+
 export function getPageDataByViteID(
 	internals: BuildInternals,
 	viteid: ViteID
@@ -253,44 +309,8 @@ export function hasPageDataByViteID(internals: BuildInternals, viteid: ViteID):
 	return internals.pagesByViteID.has(viteid);
 }
 
-export function* eachPageData(internals: BuildInternals) {
-	yield* internals.pagesByComponent.values();
-}
-
-export function* eachPageFromAllPages(allPages: AllPagesData): Generator<[string, PageBuildData]> {
-	for (const [path, pageData] of Object.entries(allPages)) {
-		yield [path, pageData];
-	}
-}
-
-export function* eachPageDataFromEntryPoint(
-	internals: BuildInternals
-): Generator<[PageBuildData, string]> {
-	for (const [entrypoint, filePath] of internals.entrySpecifierToBundleMap) {
-		// virtual pages can be emitted with different prefixes:
-		// - the classic way are pages emitted with prefix ASTRO_PAGE_RESOLVED_MODULE_ID -> plugin-pages
-		// - pages emitted using `build.split`, in this case pages are emitted with prefix RESOLVED_SPLIT_MODULE_ID
-		if (
-			entrypoint.includes(ASTRO_PAGE_RESOLVED_MODULE_ID) ||
-			entrypoint.includes(RESOLVED_SPLIT_MODULE_ID)
-		) {
-			const [, pageName] = entrypoint.split(':');
-			const pageData = internals.pagesByComponent.get(
-				`${pageName.replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.')}`
-			);
-			if (!pageData) {
-				throw new Error(
-					"Build failed. Astro couldn't find the emitted page from " + pageName + ' pattern'
-				);
-			}
-
-			yield [pageData, filePath];
-		}
-	}
-}
-
 export function hasPrerenderedPages(internals: BuildInternals) {
-	for (const pageData of eachPageData(internals)) {
+	for (const pageData of internals.pagesByKeys.values()) {
 		if (pageData.route.prerender) {
 			return true;
 		}
@@ -350,27 +370,23 @@ export function mergeInlineCss(
 	return acc;
 }
 
-export function isHoistedScript(internals: BuildInternals, id: string): boolean {
-	return internals.hoistedScriptIdToPagesMap.has(id);
-}
-
-export function* getPageDatasByHoistedScriptId(
+/**
+ * Get all pages data from the build internals, using a specific hoisted script id.
+ * @param internals Build Internals with all the pages
+ * @param id Hoisted script id, used to identify the pages using it
+ */
+export function getPageDatasByHoistedScriptId(
 	internals: BuildInternals,
 	id: string
-): Generator<PageBuildData, void, unknown> {
+): PageBuildData[]{
 	const set = internals.hoistedScriptIdToPagesMap.get(id);
+	const pageDatas: PageBuildData[] = [];
 	if (set) {
 		for (const pageId of set) {
-			const pageData = getPageDataByComponent(internals, pageId.slice(1));
-			if (pageData) {
-				yield pageData;
-			}
+			getPagesDatasByComponent(internals, pageId.slice(1)).forEach((pageData) => {
+				pageDatas.push(pageData);
+			});
 		}
 	}
-}
-
-// From a component path such as pages/index.astro find the entrypoint module
-export function getEntryFilePathFromComponentPath(internals: BuildInternals, path: string) {
-	const id = getVirtualModulePageIdFromPath(path);
-	return internals.entrySpecifierToBundleMap.get(id);
+	return pageDatas;
 }
diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts
@@ -4,6 +4,7 @@ import type { AllPagesData } from './types.js';
 
 import * as colors from 'kleur/colors';
 import { debug } from '../logger/core.js';
+import { makePageDataKey } from './plugins/util.js';
 
 export interface CollectPagesDataOptions {
 	settings: AstroSettings;
@@ -35,6 +36,8 @@ export async function collectPagesData(
 	// and is then cached across all future SSR builds. In the past, we've had trouble
 	// with parallelized builds without guaranteeing that this is called first.
 	for (const route of manifest.routes) {
+		// Generate a unique key to identify each page in the build process.
+		const key = makePageDataKey(route.route, route.component);
 		// static route:
 		if (route.pathname) {
 			const routeCollectionLogTimeout = setInterval(() => {
@@ -47,8 +50,8 @@ export async function collectPagesData(
 				clearInterval(routeCollectionLogTimeout);
 			}, 10000);
 			builtPaths.add(route.pathname);
-
-			allPages[route.component] = {
+			allPages[key] = {
+				key: key,
 				component: route.component,
 				route,
 				moduleSpecifier: '',
@@ -70,8 +73,8 @@ export async function collectPagesData(
 			continue;
 		}
 		// dynamic route:
-
-		allPages[route.component] = {
+		allPages[key] = {
+			key: key,
 			component: route.component,
 			route,
 			moduleSpecifier: '',