fix: sitemap issues (#22)

astrolicious · May 1, 2024 · 2f00eb3 · 2f00eb3
1 parent bc8fcd1
commit 2f00eb3
Show file tree

Hide file tree

Showing 9 changed files with 121 additions and 55 deletions.
diff --git a/.changeset/breezy-games-attack.md b/.changeset/breezy-games-attack.md
@@ -0,0 +1,5 @@
+---
+"@astrolicious/i18n": patch
+---
+
+Fixes a case where non pages were included in the sitemap
diff --git a/.changeset/cyan-ghosts-decide.md b/.changeset/cyan-ghosts-decide.md
@@ -0,0 +1,5 @@
+---
+"@astrolicious/i18n": patch
+---
+
+Fixes trailing slash handling in sitemap
diff --git a/.changeset/fast-dolphins-fry.md b/.changeset/fast-dolphins-fry.md
@@ -0,0 +1,5 @@
+---
+"@astrolicious/i18n": patch
+---
+
+Fixes duplicated urls with complex routes
diff --git a/.changeset/nice-eyes-roll.md b/.changeset/nice-eyes-roll.md
@@ -0,0 +1,5 @@
+---
+"@astrolicious/i18n": patch
+---
+
+Fixes a case where invalid dynamic params would cause wrong alternates to be generated
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -2,5 +2,8 @@
 	"editor.defaultFormatter": "biomejs.biome",
 	"[mdx]": {
 		"editor.defaultFormatter": "unifiedjs.vscode-mdx"
+	},
+	"[typescript]": {
+		"editor.defaultFormatter": "biomejs.biome"
 	}
 }
diff --git a/package/package.json b/package/package.json
@@ -38,10 +38,7 @@
 		"./components/I18nClient.astro": "./assets/components/I18nClient.astro",
 		"./components/I18nHead.astro": "./assets/components/I18nHead.astro"
 	},
-	"files": [
-		"dist",
-		"assets"
-	],
+	"files": ["dist", "assets"],
 	"scripts": {
 		"dev": "tsup --watch",
 		"build": "tsup"

diff --git a/package/src/sitemap/generate-sitemap.ts b/package/src/sitemap/generate-sitemap.ts
@@ -1,24 +1,12 @@
+import type { AstroConfig } from "astro";
 import type { LinkItem, SitemapItemLoose } from "sitemap";
 import type { Route } from "./integration.js";
 import type { SitemapOptions } from "./options.js";
-import { createImpossibleError } from "./utils.js";
-
-const normalizeDynamicParams = (
-	_params: Route["sitemapOptions"][number]["dynamicParams"],
-) => {
-	if (!_params) {
-		return [];
-	}
-
-	if (Array.isArray(_params)) {
-		return _params;
-	}
-
-	return Object.entries(_params).map(([locale, params]) => ({
-		locale,
-		params,
-	}));
-};
+import {
+	createImpossibleError,
+	handleTrailingSlash,
+	normalizeDynamicParams,
+} from "./utils.js";
 
 type NoUndefinedField<T> = {
 	[P in keyof T]-?: NonNullable<T[P]>;
@@ -29,6 +17,7 @@ export function generateSitemap(
 	routes: Array<Route>,
 	_finalSiteUrl: string,
 	opts: SitemapOptions,
+	config: AstroConfig,
 ) {
 	const { changefreq, priority, lastmod: lastmodSrc } = opts;
 	const lastmod = lastmodSrc?.toISOString();
@@ -57,27 +46,37 @@ export function generateSitemap(
 			for (const equivalentRoute of equivalentRoutes) {
 				links.push({
 					lang: equivalentRoute.route.locale,
-					url: `${new URL(page).origin}${
-						equivalentRoute.route.injectedRoute.pattern
-					}`,
+					url: handleTrailingSlash(
+						`${new URL(page).origin}${
+							equivalentRoute.route.injectedRoute.pattern
+						}`,
+						config,
+					),
 				});
 			}
 
 			return [...links].sort((a, b) =>
-				a.url.localeCompare(b.url, "en", { numeric: true }),
+				a.lang.localeCompare(b.lang, "en", { numeric: true }),
 			);
 		}
 
 		const index = route.pages.indexOf(page);
-		const sitemapOptions = route.sitemapOptions[index];
-		if (!sitemapOptions) {
+		const sitemapOptions = route.sitemapOptions.filter(
+			(e) =>
+				e.dynamicParams &&
+				(Array.isArray(e.dynamicParams)
+					? e.dynamicParams
+					: Object.entries(e.dynamicParams)
+				).length > 0,
+		)[index];
+		if (!sitemapOptions || !sitemapOptions.dynamicParams) {
 			return [];
 		}
 
 		for (const equivalentRoute of equivalentRoutes) {
-			const options = normalizeDynamicParams(
-				sitemapOptions?.dynamicParams,
-			).find((e) => e.locale === equivalentRoute.route.locale);
+			const options = normalizeDynamicParams(sitemapOptions.dynamicParams).find(
+				(e) => e.locale === equivalentRoute.route.locale,
+			);
 
 			if (!options) {
 				// A dynamic route is not required to always have an equivalent in another language eg.
@@ -97,14 +96,17 @@ export function generateSitemap(
 
 				newPage = newPage.replace(`[${key}]`, value);
 			}
-			newPage = `${new URL(page).origin}${newPage}`;
+			newPage = handleTrailingSlash(
+				`${new URL(page).origin}${newPage}`,
+				config,
+			);
 			links.push({
 				lang: equivalentRoute.route.locale,
 				url: newPage,
 			});
 		}
 		return [...links].sort((a, b) =>
-			a.url.localeCompare(b.url, "en", { numeric: true }),
+			a.lang.localeCompare(b.lang, "en", { numeric: true }),
 		);
 	};
 

diff --git a/package/src/sitemap/integration.ts b/package/src/sitemap/integration.ts
@@ -10,7 +10,7 @@ import {
 import { AstroError } from "astro/errors";
 import { z } from "astro/zod";
 import { simpleSitemapAndIndex } from "sitemap";
-import { withTrailingSlash, withoutTrailingSlash } from "ufo";
+import { withoutTrailingSlash } from "ufo";
 import { normalizePath } from "vite";
 import type { Route as InternalRoute } from "../types.js";
 import { generateSitemap } from "./generate-sitemap.js";
@@ -20,7 +20,9 @@ import {
 	createImpossibleError,
 	formatConfigErrorMessage,
 	getPathnameFromRouteData,
+	handleTrailingSlash,
 	isStatusCodePage,
+	normalizeDynamicParams,
 } from "./utils.js";
 
 const OUTFILE = "sitemap-index.xml";
@@ -105,6 +107,23 @@ export const integration = defineIntegration({
 										);
 									}
 									route.sitemapOptions.push(response.data);
+									if (route.route) {
+										const { locale, injectedRoute } = route.route;
+										const params = normalizeDynamicParams(
+											response.data.dynamicParams,
+										)?.find((e) => e.locale === locale);
+										if (params) {
+											let page = injectedRoute.pattern;
+											for (const [key, value] of Object.entries(
+												params.params,
+											)) {
+												if (value) {
+													page = page.replace(`[${key}]`, value);
+												}
+											}
+											route.pages.push(page);
+										}
+									}
 								}
 							}
 						},
@@ -113,6 +132,12 @@ export const integration = defineIntegration({
 				"astro:build:done": async (params) => {
 					const { logger } = params;
 
+					for (const route of initialRoutes) {
+						if (route.pages.length === 0 && route.route) {
+							route.pages.push(route.route.injectedRoute.pattern);
+						}
+					}
+
 					for (const r of initialRoutes.filter((e) => !e.routeData)) {
 						const routeData = params.routes.find(
 							(e) =>
@@ -125,7 +150,7 @@ export const integration = defineIntegration({
 							);
 						}
 						r.routeData = routeData;
-						r.include = true;
+						r.include = routeData.type === "page";
 					}
 
 					const _routes = [
@@ -202,16 +227,7 @@ export const integration = defineIntegration({
 
 								const newUrl = new URL(fullPath, finalSiteUrl).href;
 
-								if (config.trailingSlash === "never") {
-									urls.push(newUrl);
-								} else if (
-									config.build.format === "directory" &&
-									!newUrl.endsWith("/")
-								) {
-									urls.push(`${newUrl}/`);
-								} else {
-									urls.push(newUrl);
-								}
+								urls.push(handleTrailingSlash(newUrl, config));
 							}
 
 							return urls;
@@ -242,21 +258,21 @@ export const integration = defineIntegration({
 						}
 
 						for (const route of _routes.filter((e) => e.include)) {
-							for (const rawPage of pageUrls) {
-								const page = normalizePath(
-									`/${relative(config.base, new URL(rawPage).pathname)}`,
-								);
-								// biome-ignore lint/style/noNonNullAssertion: <explanation>
-								if (route.routeData!.pattern.test(withTrailingSlash(page))) {
-									route.pages.push(rawPage);
-								}
-							}
+							route.pages = route.pages.map((page) =>
+								page.startsWith("/")
+									? handleTrailingSlash(
+											new URL(page, finalSiteUrl).href,
+											config,
+										)
+									: page,
+							);
 						}
 
 						const urlData = generateSitemap(
 							_routes.filter((e) => e.include),
 							finalSiteUrl.href,
 							options,
+							config,
 						);
 
 						const destDir = fileURLToPath(params.dir);

diff --git a/package/src/sitemap/utils.ts b/package/src/sitemap/utils.ts
@@ -1,6 +1,7 @@
-import type { RouteData } from "astro";
+import type { AstroConfig, RouteData } from "astro";
 import { AstroError } from "astro/errors";
 import type { ZodError } from "astro/zod";
+import type { Route } from "./integration.js";
 
 const STATUS_CODE_PAGES = new Set(["404", "500"]);
 
@@ -37,3 +38,30 @@ export const getPathnameFromRouteData = ({ segments }: RouteData) => {
 
 	return `/${pathname}`;
 };
+
+export const normalizeDynamicParams = (
+	_params: Route["sitemapOptions"][number]["dynamicParams"],
+) => {
+	if (!_params) {
+		return [];
+	}
+
+	if (Array.isArray(_params)) {
+		return _params;
+	}
+
+	return Object.entries(_params).map(([locale, params]) => ({
+		locale,
+		params,
+	}));
+};
+
+export const handleTrailingSlash = (url: string, config: AstroConfig) => {
+	if (config.trailingSlash === "never") {
+		return url;
+	}
+	if (config.build.format === "directory" && !url.endsWith("/")) {
+		return `${url}/`;
+	}
+	return url;
+};