Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue #347]: Add next sitemap #354

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ Optionally, configure your code editor to auto run these tools on file save. Mos

</details>

## Sitemap

A dynamically generated sitemap is available for the application at http://localhost:3001/sitemap.xml. The sitemap helps search engines index the content of your site more efficiently and can also be used for tools such as accessibility scans. This sitemap is automatically updated to reflect the current application routes.

## Other topics

- [Internationalization](../docs/app/internationalization.md)
Expand Down
9 changes: 8 additions & 1 deletion app/src/app/[locale]/health/page.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
export default function Page() {
return <>healthy</>;
return (
<>
<head>
<title>Health Check</title>
</head>
<div>healthy</div>
</>
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little bit of a separate issue but this fixes an actual a11y issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a localization library in this repo - is there a reason not to use it here?

Copy link
Contributor Author

@rylew1 rylew1 Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Just added it .

I assume because we're not really refining tickets here we're not splitting them up as we might on our normal projects - first inclination is to actually break a task like that out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to say that you think including content strings using the localization library should be a separate task ticketed separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so because it's getting a bit off track of the intent of this PR

}
17 changes: 17 additions & 0 deletions app/src/app/sitemap.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { MetadataRoute } from "next";

import { getNextRoutes } from "../utils/getRoutes";

export default function sitemap(): MetadataRoute.Sitemap {
const routes = getNextRoutes("./src/app");

const baseUrl = process.env.NEXT_PUBLIC_BASE_URL || "http://localhost:3000";
const sitemap: MetadataRoute.Sitemap = routes.map((route) => ({
url: `${baseUrl}${route || ""}`,
lastModified: new Date().toISOString(),
changeFrequency: "weekly",
priority: 0.5,
}));

return sitemap;
}
2 changes: 1 addition & 1 deletion app/src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { defaultLocale, locales } from "./i18n/config";

// Don't run middleware on API routes or Next.js build output
export const config = {
matcher: ["/((?!api|_next|.*\\..*).*)"],
matcher: ["/((?!api|_next|sitemap|.*\\..*).*)"],
};

/**
Expand Down
41 changes: 41 additions & 0 deletions app/src/utils/getRoutes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import fs from "node:fs";
import path from "node:path";

// Helper function to list all paths in a directory recursively
export function listPaths(dir: string): string[] {
let fileList: string[] = [];
const files = fs.readdirSync(dir);
files.forEach((file) => {
const filePath = path.join(dir, file);
if (fs.statSync(filePath).isDirectory()) {
fileList = fileList.concat(listPaths(filePath));
} else {
fileList.push(filePath);
}
});
return fileList;
}

// Function to get the Next.js routes
export function getNextRoutes(src: string): string[] {
// Get all paths from the `app` directory
const appPaths = listPaths(src).filter((file) => file.endsWith("page.tsx"));

// Extract the route name for each `page.tsx` file
// Basically anything between [locale] and /page.tsx is extracted,
// which lets us get nested routes such as /newsletter/unsubscribe
const appRoutes = appPaths.map((filePath) => {
const relativePath = path.relative(src, filePath);
const route = relativePath
? "/" +
relativePath
// for any additional overrides add more replace here...
.replace("/page.tsx", "")
.replace(/\[locale\]/g, "")
.replace(/\\/g, "/")
: "/";
return route.replace(/\/\//g, "/");
});

return appRoutes;
}
Loading