Add configurable base path for static export and subpath deploys#3
Merged
Conversation
The static export is deployed to https://hsiangnianian.github.io/cortex/, but all asset/JS/CSS paths resolved from root `/`, causing 404s and a blank page. Configure `basePath` via `BASE_PATH` env var (defaults to empty for root deployments) and prefix all raw absolute paths that Next.js doesn't auto-handle: - fetch("/api/...") calls in useTestState, stats-client, site-goal - service worker registration path - share URL construction - Notification icon path - manifest.json start_url and icon src - sitemap/robots URLs - canonical/alternate URLs in locale layout - lang-init inline script (strip basePath before reading locale segment) `<Link>`, `next/image`, and metadata `icons`/`openGraph.images` are auto-prefixed by Next.js and left untouched.
The `.env*` glob in .gitignore also excluded `.env.example`, which documents required env vars for new contributors. Add a negation pattern so the template is tracked while real `.env` files remain ignored.
Reviewer's Guide添加一个可配置的 File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience打开你的 dashboard 来:
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds a configurable basePath for subpath deployments (e.g. GitHub Pages) and systematically applies it across fetch calls, URLs, PWA manifest, robots/sitemap, service worker registration, and client-side language detection, while cleaning up minor formatting inconsistencies. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我发现了 4 个问题,并给出了一些整体反馈:
- 建议对 BASE_PATH 做规范化处理(前面强制有
/、末尾不带/),以及/或者更新 withBasePath 的实现,避免在组合不同环境变量值和根绝对路径时出现双斜杠。 - 运行时代码使用的是 NEXT_PUBLIC_BASE_PATH,而 next.config.ts 读取的是 BASE_PATH,这两者可能会产生偏差;在 basePath 上统一使用一个环境变量,可以减少构建时与前后端代码之间的配置错误。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- Consider normalizing BASE_PATH (leading slash, no trailing slash) and/or updating withBasePath to avoid double slashes when combining different env values and root-absolute paths.
- Runtime code uses NEXT_PUBLIC_BASE_PATH while next.config.ts reads BASE_PATH, which can diverge; aligning on a single env variable for basePath would reduce configuration mistakes between build-time and client/server code.
## Individual Comments
### Comment 1
<location path="next.config.ts" line_range="7-11" />
<code_context>
const withNextIntl = createNextIntlPlugin();
+// Deployed to https://hsiangnianian.github.io/cortex/ — override via env for other deployments.
+const BASE_PATH = process.env.BASE_PATH ?? "";
+
const nextConfig: NextConfig = {
output: "export",
+ basePath: BASE_PATH || undefined,
+ // trailingSlash helps GitHub Pages resolve directory-style URLs
+ trailingSlash: true,
</code_context>
<issue_to_address>
**issue (bug_risk):** Base path env var is inconsistent with runtime BASE_PATH, which can desync URLs and routing.
`next.config.ts` reads `process.env.BASE_PATH`, while `src/lib/site-config.ts` uses `process.env.NEXT_PUBLIC_BASE_PATH`. In multi-env deployments this can cause `basePath` and code using `withBasePath` / `BASE_PATH` to diverge, breaking navigation, fetches, or lang init. Please standardize on a single env var (e.g. `NEXT_PUBLIC_BASE_PATH`) and reuse it in both places.
</issue_to_address>
### Comment 2
<location path="src/lib/site-config.ts" line_range="39-44" />
<code_context>
+ * Raw `fetch("/api/...")` calls must be prefixed with this, unlike
+ * `<Link>` / `next/image` which auto-apply basePath.
+ */
+export const BASE_PATH = process.env.NEXT_PUBLIC_BASE_PATH ?? "";
+
+/** Prefix a root-absolute path with the configured basePath. */
+export function withBasePath(path: string): string {
+ if (!BASE_PATH) return path;
+ return `${BASE_PATH}${path}`;
+}
</code_context>
<issue_to_address>
**suggestion:** withBasePath assumes a normalized BASE_PATH and may produce double slashes or mis-handle non-root-relative paths.
Right now it just concatenates `${BASE_PATH}${path}`. If `BASE_PATH` has a trailing slash (e.g. `/cortex/`) and callers pass `/api/stats`, you get `/cortex//api/stats`. If callers ever pass `stats` (no leading slash), the URL shape changes again. Consider normalizing `BASE_PATH` (e.g. strip trailing slash) and enforcing/normalizing `path` to start with `/` inside `withBasePath`.
</issue_to_address>
### Comment 3
<location path="src/app/[locale]/layout.tsx" line_range="60-62" />
<code_context>
{ rel: "apple-touch-icon", url: "/favicon-180.png" },
],
- metadataBase: new URL(BASE_URL),
+ metadataBase: new URL(SITE_URL),
alternates: {
canonical: locale === "zh-CN" ? BASE_URL : `${BASE_URL}/${locale}`,
languages: {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using SITE_URL for metadataBase while BASE_URL includes BASE_PATH can lead to inconsistent canonical/OG URLs.
Here `BASE_URL` is `${SITE_URL}${BASE_PATH}`, but `metadataBase` now uses `new URL(SITE_URL)`. That means relative metadata URLs (OG images, canonical links, etc.) resolve without `BASE_PATH`, while `alternates.canonical` still includes it. To avoid inconsistent URLs, either use `new URL(BASE_URL)` so everything includes the basePath, or build all paths relative to `SITE_URL` and keep `alternates.canonical` consistent with that choice.
</issue_to_address>
### Comment 4
<location path="src/app/layout.tsx" line_range="9-12" />
<code_context>
import { routing } from "@/i18n/routing";
const LOCALES_JSON = JSON.stringify(routing.locales);
+const BASE_PATH_JS = JSON.stringify(BASE_PATH);
export const metadata: Metadata = {
</code_context>
<issue_to_address>
**issue (bug_risk):** Lang-init script relies on BASE_PATH matching the actual deployed basePath; divergence will break locale detection.
Because the script strips `bp` from `window.location.pathname` before reading the locale, any mismatch between `BASE_PATH` from `site-config` and `next.config.ts`’s `basePath` (including slashes) means `p.StartsWith(bp)` fails and locale detection uses the wrong path for subpath deployments. Please ensure both configs use the same env source and a normalized `basePath` format (e.g., no trailing slash) so this logic remains reliable.
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的 Review。
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- Consider normalizing BASE_PATH (leading slash, no trailing slash) and/or updating withBasePath to avoid double slashes when combining different env values and root-absolute paths.
- Runtime code uses NEXT_PUBLIC_BASE_PATH while next.config.ts reads BASE_PATH, which can diverge; aligning on a single env variable for basePath would reduce configuration mistakes between build-time and client/server code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider normalizing BASE_PATH (leading slash, no trailing slash) and/or updating withBasePath to avoid double slashes when combining different env values and root-absolute paths.
- Runtime code uses NEXT_PUBLIC_BASE_PATH while next.config.ts reads BASE_PATH, which can diverge; aligning on a single env variable for basePath would reduce configuration mistakes between build-time and client/server code.
## Individual Comments
### Comment 1
<location path="next.config.ts" line_range="7-11" />
<code_context>
const withNextIntl = createNextIntlPlugin();
+// Deployed to https://hsiangnianian.github.io/cortex/ — override via env for other deployments.
+const BASE_PATH = process.env.BASE_PATH ?? "";
+
const nextConfig: NextConfig = {
output: "export",
+ basePath: BASE_PATH || undefined,
+ // trailingSlash helps GitHub Pages resolve directory-style URLs
+ trailingSlash: true,
</code_context>
<issue_to_address>
**issue (bug_risk):** Base path env var is inconsistent with runtime BASE_PATH, which can desync URLs and routing.
`next.config.ts` reads `process.env.BASE_PATH`, while `src/lib/site-config.ts` uses `process.env.NEXT_PUBLIC_BASE_PATH`. In multi-env deployments this can cause `basePath` and code using `withBasePath` / `BASE_PATH` to diverge, breaking navigation, fetches, or lang init. Please standardize on a single env var (e.g. `NEXT_PUBLIC_BASE_PATH`) and reuse it in both places.
</issue_to_address>
### Comment 2
<location path="src/lib/site-config.ts" line_range="39-44" />
<code_context>
+ * Raw `fetch("/api/...")` calls must be prefixed with this, unlike
+ * `<Link>` / `next/image` which auto-apply basePath.
+ */
+export const BASE_PATH = process.env.NEXT_PUBLIC_BASE_PATH ?? "";
+
+/** Prefix a root-absolute path with the configured basePath. */
+export function withBasePath(path: string): string {
+ if (!BASE_PATH) return path;
+ return `${BASE_PATH}${path}`;
+}
</code_context>
<issue_to_address>
**suggestion:** withBasePath assumes a normalized BASE_PATH and may produce double slashes or mis-handle non-root-relative paths.
Right now it just concatenates `${BASE_PATH}${path}`. If `BASE_PATH` has a trailing slash (e.g. `/cortex/`) and callers pass `/api/stats`, you get `/cortex//api/stats`. If callers ever pass `stats` (no leading slash), the URL shape changes again. Consider normalizing `BASE_PATH` (e.g. strip trailing slash) and enforcing/normalizing `path` to start with `/` inside `withBasePath`.
</issue_to_address>
### Comment 3
<location path="src/app/[locale]/layout.tsx" line_range="60-62" />
<code_context>
{ rel: "apple-touch-icon", url: "/favicon-180.png" },
],
- metadataBase: new URL(BASE_URL),
+ metadataBase: new URL(SITE_URL),
alternates: {
canonical: locale === "zh-CN" ? BASE_URL : `${BASE_URL}/${locale}`,
languages: {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using SITE_URL for metadataBase while BASE_URL includes BASE_PATH can lead to inconsistent canonical/OG URLs.
Here `BASE_URL` is `${SITE_URL}${BASE_PATH}`, but `metadataBase` now uses `new URL(SITE_URL)`. That means relative metadata URLs (OG images, canonical links, etc.) resolve without `BASE_PATH`, while `alternates.canonical` still includes it. To avoid inconsistent URLs, either use `new URL(BASE_URL)` so everything includes the basePath, or build all paths relative to `SITE_URL` and keep `alternates.canonical` consistent with that choice.
</issue_to_address>
### Comment 4
<location path="src/app/layout.tsx" line_range="9-12" />
<code_context>
import { routing } from "@/i18n/routing";
const LOCALES_JSON = JSON.stringify(routing.locales);
+const BASE_PATH_JS = JSON.stringify(BASE_PATH);
export const metadata: Metadata = {
</code_context>
<issue_to_address>
**issue (bug_risk):** Lang-init script relies on BASE_PATH matching the actual deployed basePath; divergence will break locale detection.
Because the script strips `bp` from `window.location.pathname` before reading the locale, any mismatch between `BASE_PATH` from `site-config` and `next.config.ts`’s `basePath` (including slashes) means `p.startsWith(bp)` fails and locale detection uses the wrong path for subpath deployments. Please ensure both configs use the same env source and a normalized `basePath` format (e.g., no trailing slash) so this logic remains reliable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Three issues identified during review: 1. next.config.ts read `process.env.BASE_PATH` while site-config.ts read `NEXT_PUBLIC_BASE_PATH` — a mismatch that could cause the two to drift under different deployment environments. Both now read `NEXT_PUBLIC_BASE_PATH` as the single source of truth. 2. `withBasePath()` blindly concatenated `BASE_PATH + path`, producing double slashes (`/cortex//api/stats`) when BASE_PATH had a trailing slash or path lacked a leading slash. Now normalizes BASE_PATH (strip trailing slash, ensure leading slash) and forces path to start with `/`. 3. `metadataBase` used `SITE_URL` (without basePath) while `alternates.canonical` used `BASE_URL` (with basePath), causing relative metadata URLs (OG images, canonical) to resolve inconsistently. Both now use `BASE_URL`. Also drop the redundant `BASE_PATH` env var (without `NEXT_PUBLIC_` prefix) from .env.example.
Owner
|
@SourceryAI title |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
为静态导出部署添加可配置的基础路径支持,并在整个应用中更新 URL 和数据获取调用以遵循该设置。
Enhancements:
BASE_PATH和withBasePath辅助方法,以支持像 GitHub Pages 这样的子路径部署。basePath和trailingSlash设置,以兼容静态托管。Build:
basePath和trailingSlash选项,用于静态导出。Original summary in English
Summary by Sourcery
Add configurable base path support for static export deployments and update URLs and fetch calls to respect it across the app.
Enhancements:
Build: