Skip to content

[Docs] Fix 404 asset base path for previews#1066

Open
CodexRaunak wants to merge 1 commit into
layer5io:masterfrom
CodexRaunak:fix-404-preview-base-path
Open

[Docs] Fix 404 asset base path for previews#1066
CodexRaunak wants to merge 1 commit into
layer5io:masterfrom
CodexRaunak:fix-404-preview-base-path

Conversation

@CodexRaunak
Copy link
Copy Markdown
Contributor

Description

Fixes the Layer5 docs 404 page asset base path so missing nested routes render with styling in production while preserving relative behavior for GitHub Pages PR previews.

Root cause

The generated 404 page emits relative asset URLs like ./scss/... and ./js/.... On missing nested production paths such as /foo/ or /foo/bar, those relative URLs resolve against the missing route depth and can load incorrectly.

Changes

  • Add a 404-only runtime base path before relative assets load.
  • Use / for production docs paths.
  • Preserve /pr-preview/pr-N/ paths, including the GitHub Pages repository prefix, for PR previews.
  • Create and insert the element via DOM APIs instead of document.write().

Validation

  • Confirmed production and preview workflows use root-relative builds with GitHub Pages preview subpaths.
  • Built docs with Hugo using production-style --baseURL / into /private/tmp/layer5-docs-404-review-suggestion.
  • Verified generated 404.html includes the DOM-created base script before relative CSS and JS asset links, and does not use document.write for the 404 base path.

Signed-off-by: Raunak Madan <madanraunak24@gmail.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a script block to layouts/partials/head.html for 404 pages to dynamically set the <base> tag's href attribute when in a PR preview environment. The reviewer suggested updating the JavaScript variable declarations from var to const to align with modern standards and improve block-scoping.

Comment on lines +18 to +25
(function () {
var path = window.location.pathname;
var previewPath = path.match(/^((?:\/[^/]+)?\/pr-preview\/pr-[^/]+\/)/);
var basePath = previewPath ? previewPath[1] : "/";
var base = document.createElement("base");
base.href = basePath;
document.head.insertBefore(base, document.head.firstChild);
})();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using var for variable declarations is an outdated practice in modern JavaScript. It is highly recommended to use const (or let if re-assignment is needed) to ensure block-scoping, prevent accidental re-declarations, and improve overall code maintainability and readability.

Suggested change
(function () {
var path = window.location.pathname;
var previewPath = path.match(/^((?:\/[^/]+)?\/pr-preview\/pr-[^/]+\/)/);
var basePath = previewPath ? previewPath[1] : "/";
var base = document.createElement("base");
base.href = basePath;
document.head.insertBefore(base, document.head.firstChild);
})();
(function () {
const path = window.location.pathname;
const previewPath = path.match(/^((?:\/[^/]+)?\/pr-preview\/pr-[^/]+\/)/);
const basePath = previewPath ? previewPath[1] : "/";
const base = document.createElement("base");
base.href = basePath;
document.head.insertBefore(base, document.head.firstChild);
})();

@github-actions
Copy link
Copy Markdown
Contributor

🚀 Preview deployment: https://layer5io.github.io/docs/pr-preview/pr-1066/

Note: Preview may take a moment (GitHub Pages deployment in progress). Please wait and refresh. Track deployment here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant