chore(docs): fix base url paths#130
Conversation
- Changed internal links in documentation to use root-relative paths for consistency. - Added a new transform for site URLs to ensure correct linking in different environments. - Updated various files to reflect these changes, including configuration and layout files. - Added tests to verify that all internal links are correctly formatted. Signed-off-by: Cory Rylan <crylan@nvidia.com>
📝 WalkthroughWalkthroughThe PR systematically normalizes internal site URLs by introducing centralized URL utilities, an HTML transform for automatic rewriting during the build pipeline, and updating all code and documentation to use absolute root-relative paths instead of relative paths. ChangesURL Normalization Infrastructure and Implementation
Sequence Diagram(s)sequenceDiagram
participant Browser
participant EleventyBuild as Eleventy Build
participant HtmlBase as HtmlBasePlugin
participant Transform as siteUrlsTransform
participant Utils as URL Utils
participant Output as HTML Output
Browser->>EleventyBuild: trigger build
EleventyBuild->>HtmlBase: render templates
HtmlBase->>EleventyBuild: add <base href>
EleventyBuild->>Transform: pass generated HTML
Transform->>Utils: parse & resolve URLs
Utils->>Transform: absolute or base-relative paths
Transform->>Output: rewrite href/src attributes
EleventyBuild->>Browser: serve normalized URLs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| import { | ||
| DEPLOYED_SITE_URL, | ||
| ELEMENTS_SITE_ORIGIN, | ||
| getSitePath as getBaseFreeSitePath, | ||
| getSiteHref, | ||
| getSiteUrl | ||
| } from '../utils/site-url.js'; |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/site/src/_11ty/layouts/docs.11ty.js (1)
50-54: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider simplifying the baseTabUrl construction.
The
baseTabUrlremoves the leading/on line 51 (.replace('/', '')), then lines 91, 94, and 99 add it back (href="/${baseTabUrl}"). This could be simplified by keeping the leading/inbaseTabUrl:const baseTabUrl = `${data.page.url - .replace('/', '') .replace('/api/', '/') .replace('/examples/', '/') .replace(/(.*\/data-grid\/).+/, '$1')}`;Then use it directly:
-<a href="/${baseTabUrl}">Overview</a> +<a href="${baseTabUrl}">Overview</a>This would be clearer and avoid the remove-then-add pattern.
Also applies to: 91-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/site/src/_11ty/layouts/docs.11ty.js` around lines 50 - 54, The code builds baseTabUrl by stripping the leading '/' from data.page.url then later prepends '/' again when creating hrefs; to simplify, stop removing the leading slash: remove the initial .replace('/', '') from the baseTabUrl expression (leave data.page.url as-is when applying the subsequent .replace('/api/', '/'), .replace('/examples/', '/'), and .replace(/(.*\/data-grid\/).+/, '$1')), and update all call sites that currently do href="/${baseTabUrl}" to use href="${baseTabUrl}" (references: the baseTabUrl variable and the href usages that add '/').
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/site/package.json`:
- Around line 204-211: The current package.json test task watches all source
files via the "files" array which causes tests defined in the "command" (e.g.,
metadata.test.ts, links.test.ts, site-urls.test.ts, api.test.ts) to re-run on
any src change; update the "files" array to only match test files (for example
restrict to patterns like src/**/*.{test,spec}.ts or src/**/*.test.ts and
include matching .js/.md if you have test variants) so that vitest only reruns
when test files change, leaving the "command" unchanged.
In `@projects/site/src/_11ty/layouts/links.test.ts`:
- Around line 6-9: The four regex constants RELATIVE_INTERNAL_LINK_PATTERN,
BASE_PREFIXED_INTERNAL_LINK_PATTERN, JS_RELATIVE_INTERNAL_LINK_PATTERN, and
JS_BASE_PREFIXED_INTERNAL_LINK_PATTERN only account for "./" relative paths;
update each pattern to also match one or more "../" segments so "../docs/...",
"../examples/...", and "../starters/..." are caught too — modify the group that
currently matches (?:\.\/)? to allow sequences like (?:\.\.\/|\.\/)+ or a more
concise (?:\.\.\/)*\.\/? depending on desired strictness so both single and
multi-level parent traversals are rejected for both HTML and JS forms.
In `@projects/site/src/docs/metrics/testing-and-performance.11ty.js`:
- Around line 29-32: Remove the duplicate selected attribute from the wrong tab:
the first <nve-tabs-item> that links to "/docs/metrics/" currently has selected
but the page's correct active tab is the <nve-tabs-item> for "Testing &
Performance" (the one linking to "/docs/metrics/testing-and-performance/");
remove the selected attribute from the Metrics tab so only the Testing &
Performance <nve-tabs-item> remains selected to prevent conflicting active-state
behavior.
In `@projects/site/src/docs/metrics/wireit.11ty.js`:
- Line 59: Duplicate selected attribute: remove the extra selected on the
Metrics tab so only the Wireit Explorer tab remains selected. Locate the
<nve-tabs-item> element that renders the "Metrics" tab (the tag containing "<a
href=\"/docs/metrics/\">Metrics</a>") and delete the selected attribute from
that element, leaving the selected attribute only on the "Wireit Explorer"
<nve-tabs-item>.
---
Outside diff comments:
In `@projects/site/src/_11ty/layouts/docs.11ty.js`:
- Around line 50-54: The code builds baseTabUrl by stripping the leading '/'
from data.page.url then later prepends '/' again when creating hrefs; to
simplify, stop removing the leading slash: remove the initial .replace('/', '')
from the baseTabUrl expression (leave data.page.url as-is when applying the
subsequent .replace('/api/', '/'), .replace('/examples/', '/'), and
.replace(/(.*\/data-grid\/).+/, '$1')), and update all call sites that currently
do href="/${baseTabUrl}" to use href="${baseTabUrl}" (references: the baseTabUrl
variable and the href usages that add '/').
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ce99f9ec-a08b-4139-a9d2-571dfc979998
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (69)
projects/core/src/tabs/tabs.examples.tsprojects/site/eleventy.config.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/_11ty/layouts/docs.11ty.jsprojects/site/src/_11ty/layouts/links.test.tsprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/page.11ty.jsprojects/site/src/_11ty/plugins/llms-txt.jsprojects/site/src/_11ty/plugins/sitemap-xml.jsprojects/site/src/_11ty/shortcodes/index.jsprojects/site/src/_11ty/templates/api.jsprojects/site/src/_11ty/transforms/anchor-generator.jsprojects/site/src/_11ty/transforms/site-urls.jsprojects/site/src/_11ty/transforms/site-urls.test.tsprojects/site/src/_11ty/utils/site-url.jsprojects/site/src/_internal/metrics-carousel/metrics-carousel.tsprojects/site/src/docs/about/contributions.mdprojects/site/src/docs/about/migration.mdprojects/site/src/docs/about/support.mdprojects/site/src/docs/api-design/logs.mdprojects/site/src/docs/api-design/packaging.mdprojects/site/src/docs/changelog/changelog.11ty.jsprojects/site/src/docs/changelog/index.11ty.jsprojects/site/src/docs/elements/dialog.mdprojects/site/src/docs/elements/drawer.mdprojects/site/src/docs/elements/dropdown.mdprojects/site/src/docs/elements/forms/index.mdprojects/site/src/docs/elements/icon.mdprojects/site/src/docs/elements/notification.mdprojects/site/src/docs/elements/page.mdprojects/site/src/docs/elements/toast.mdprojects/site/src/docs/elements/toggletip.mdprojects/site/src/docs/elements/tooltip.mdprojects/site/src/docs/foundations/iconography.mdprojects/site/src/docs/foundations/index.mdprojects/site/src/docs/foundations/layout/grid.mdprojects/site/src/docs/foundations/layout/horizontal.mdprojects/site/src/docs/foundations/layout/index.mdprojects/site/src/docs/foundations/layout/responsive/container.mdprojects/site/src/docs/foundations/layout/responsive/index.mdprojects/site/src/docs/foundations/layout/responsive/viewport.mdprojects/site/src/docs/foundations/layout/vertical.mdprojects/site/src/docs/foundations/popovers.mdprojects/site/src/docs/foundations/themes/index.11ty.jsprojects/site/src/docs/foundations/typography.mdprojects/site/src/docs/foundations/view-transitions.mdprojects/site/src/docs/integrations/angular.mdprojects/site/src/docs/integrations/index.11ty.jsprojects/site/src/docs/integrations/lit.mdprojects/site/src/docs/integrations/mcp-apps.mdprojects/site/src/docs/integrations/shortcodes.jsprojects/site/src/docs/internal/guidelines/component-creation.mdprojects/site/src/docs/internal/guidelines/testing.mdprojects/site/src/docs/metrics/api-status.11ty.jsprojects/site/src/docs/metrics/bundle-explorer.11ty.jsprojects/site/src/docs/metrics/index.11ty.jsprojects/site/src/docs/metrics/metadata.11ty.jsprojects/site/src/docs/metrics/testing-and-performance.11ty.jsprojects/site/src/docs/metrics/wireit.11ty.jsprojects/site/src/docs/monaco/diff-editor.mdprojects/site/src/docs/monaco/editor.mdprojects/site/src/docs/patterns/editor.mdprojects/site/src/docs/patterns/index.mdprojects/site/src/docs/patterns/panel.mdprojects/site/src/docs/skills/index.mdprojects/site/src/examples/examples.11ty.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/index.md
| "command": "vitest run src/_11ty/layouts/metadata.test.ts src/_11ty/layouts/links.test.ts src/_11ty/transforms/site-urls.test.ts src/_11ty/shortcodes/api.test.ts", | ||
| "files": [ | ||
| "src/docs/**/*.md", | ||
| "src/_11ty/layouts/common.js", | ||
| "src/_11ty/layouts/metadata.js", | ||
| "src/_11ty/layouts/metadata.test.ts", | ||
| "src/_11ty/shortcodes/api.js", | ||
| "src/_11ty/shortcodes/api.test.ts", | ||
| "src/**/*.js", | ||
| "src/**/*.md", | ||
| "src/**/*.ts", | ||
| "vitest.config.ts" | ||
| ], | ||
| "output": [] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Consider narrowing the test watch patterns to test files only.
The files array now watches all source files (src/**/*.js, src/**/*.md, src/**/*.ts) rather than just test files. This means tests will re-run whenever any source file changes, not just when test files change. While this ensures tests stay fresh, it may cause unnecessary test runs during development.
If tests should only re-run when test files change, consider:
"files": [
- "src/**/*.js",
- "src/**/*.md",
- "src/**/*.ts",
+ "src/**/*.test.ts",
+ "src/**/*.test.js",
"vitest.config.ts"
],Otherwise, if the current behavior is intentional (to ensure tests run on any source change), this is fine.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "command": "vitest run src/_11ty/layouts/metadata.test.ts src/_11ty/layouts/links.test.ts src/_11ty/transforms/site-urls.test.ts src/_11ty/shortcodes/api.test.ts", | |
| "files": [ | |
| "src/docs/**/*.md", | |
| "src/_11ty/layouts/common.js", | |
| "src/_11ty/layouts/metadata.js", | |
| "src/_11ty/layouts/metadata.test.ts", | |
| "src/_11ty/shortcodes/api.js", | |
| "src/_11ty/shortcodes/api.test.ts", | |
| "src/**/*.js", | |
| "src/**/*.md", | |
| "src/**/*.ts", | |
| "vitest.config.ts" | |
| ], | |
| "output": [] | |
| "command": "vitest run src/_11ty/layouts/metadata.test.ts src/_11ty/layouts/links.test.ts src/_11ty/transforms/site-urls.test.ts src/_11ty/shortcodes/api.test.ts", | |
| "files": [ | |
| "src/**/*.test.ts", | |
| "src/**/*.test.js", | |
| "vitest.config.ts" | |
| ], | |
| "output": [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/package.json` around lines 204 - 211, The current package.json
test task watches all source files via the "files" array which causes tests
defined in the "command" (e.g., metadata.test.ts, links.test.ts,
site-urls.test.ts, api.test.ts) to re-run on any src change; update the "files"
array to only match test files (for example restrict to patterns like
src/**/*.{test,spec}.ts or src/**/*.test.ts and include matching .js/.md if you
have test variants) so that vitest only reruns when test files change, leaving
the "command" unchanged.
| const RELATIVE_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()(?:\.\/)?(?:docs|examples|starters)\//; | ||
| const BASE_PREFIXED_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()\/elements\/(?:docs|examples|starters)\//; | ||
| const JS_RELATIVE_INTERNAL_LINK_PATTERN = /\bhref:\s*['"](?:\.\/)?(?:docs|examples|starters)\//; | ||
| const JS_BASE_PREFIXED_INTERNAL_LINK_PATTERN = /\bhref:\s*['"]\/elements\/(?:docs|examples|starters)\//; |
There was a problem hiding this comment.
Expand invalid-link regexes to catch ../ relative paths.
Line 6 and Line 8 only reject ./... relative links. ../docs/..., ../examples/..., and ../starters/... are still relative and currently bypass this test.
Suggested patch
-const RELATIVE_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()(?:\.\/)?(?:docs|examples|starters)\//;
+const RELATIVE_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()(?:(?:\.\.\/)+|\.\/)?(?:docs|examples|starters)\//;
...
-const JS_RELATIVE_INTERNAL_LINK_PATTERN = /\bhref:\s*['"](?:\.\/)?(?:docs|examples|starters)\//;
+const JS_RELATIVE_INTERNAL_LINK_PATTERN = /\bhref:\s*['"](?:(?:\.\.\/)+|\.\/)?(?:docs|examples|starters)\//;As per coding guidelines, “**/*.test.ts: Follow unit testing patterns ... including ... patterns” and this test should fully enforce the intended invalid-link patterns.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const RELATIVE_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()(?:\.\/)?(?:docs|examples|starters)\//; | |
| const BASE_PREFIXED_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()\/elements\/(?:docs|examples|starters)\//; | |
| const JS_RELATIVE_INTERNAL_LINK_PATTERN = /\bhref:\s*['"](?:\.\/)?(?:docs|examples|starters)\//; | |
| const JS_BASE_PREFIXED_INTERNAL_LINK_PATTERN = /\bhref:\s*['"]\/elements\/(?:docs|examples|starters)\//; | |
| const RELATIVE_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()(?:(?:\.\.\/)+|\.\/)?(?:docs|examples|starters)\//; | |
| const BASE_PREFIXED_INTERNAL_LINK_PATTERN = /(?:href=["']|]\()\/elements\/(?:docs|examples|starters)\//; | |
| const JS_RELATIVE_INTERNAL_LINK_PATTERN = /\bhref:\s*['"](?:(?:\.\.\/)+|\.\/)?(?:docs|examples|starters)\//; | |
| const JS_BASE_PREFIXED_INTERNAL_LINK_PATTERN = /\bhref:\s*['"]\/elements\/(?:docs|examples|starters)\//; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/_11ty/layouts/links.test.ts` around lines 6 - 9, The four
regex constants RELATIVE_INTERNAL_LINK_PATTERN,
BASE_PREFIXED_INTERNAL_LINK_PATTERN, JS_RELATIVE_INTERNAL_LINK_PATTERN, and
JS_BASE_PREFIXED_INTERNAL_LINK_PATTERN only account for "./" relative paths;
update each pattern to also match one or more "../" segments so "../docs/...",
"../examples/...", and "../starters/..." are caught too — modify the group that
currently matches (?:\.\/)? to allow sequences like (?:\.\.\/|\.\/)+ or a more
concise (?:\.\.\/)*\.\/? depending on desired strictness so both single and
multi-level parent traversals are rejected for both HTML and JS forms.
Source: Coding guidelines
| <nve-tabs-item selected><a href="/docs/metrics/">Metrics</a></nve-tabs-item> | ||
| <nve-tabs-item><a href="/docs/metrics/api-status/">API Status</a></nve-tabs-item> | ||
| <nve-tabs-item selected><a href="/docs/metrics/testing-and-performance/">Testing & Performance</a></nve-tabs-item> | ||
| <nve-tabs-item><a href="/docs/metrics/wireit/">Wireit Explorer</a></nve-tabs-item> |
There was a problem hiding this comment.
Remove duplicate selected state in the metrics tab list.
At Line 29 and Line 31, two <nve-tabs-item> entries are marked selected. Keep only the current page tab selected (Testing & Performance) to avoid conflicting active-state behavior.
Suggested fix
- <nve-tabs-item selected><a href="/docs/metrics/">Metrics</a></nve-tabs-item>
+ <nve-tabs-item><a href="/docs/metrics/">Metrics</a></nve-tabs-item>
<nve-tabs-item><a href="/docs/metrics/api-status/">API Status</a></nve-tabs-item>
<nve-tabs-item selected><a href="/docs/metrics/testing-and-performance/">Testing & Performance</a></nve-tabs-item>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <nve-tabs-item selected><a href="/docs/metrics/">Metrics</a></nve-tabs-item> | |
| <nve-tabs-item><a href="/docs/metrics/api-status/">API Status</a></nve-tabs-item> | |
| <nve-tabs-item selected><a href="/docs/metrics/testing-and-performance/">Testing & Performance</a></nve-tabs-item> | |
| <nve-tabs-item><a href="/docs/metrics/wireit/">Wireit Explorer</a></nve-tabs-item> | |
| <nve-tabs-item><a href="/docs/metrics/">Metrics</a></nve-tabs-item> | |
| <nve-tabs-item><a href="/docs/metrics/api-status/">API Status</a></nve-tabs-item> | |
| <nve-tabs-item selected><a href="/docs/metrics/testing-and-performance/">Testing & Performance</a></nve-tabs-item> | |
| <nve-tabs-item><a href="/docs/metrics/wireit/">Wireit Explorer</a></nve-tabs-item> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/testing-and-performance.11ty.js` around lines
29 - 32, Remove the duplicate selected attribute from the wrong tab: the first
<nve-tabs-item> that links to "/docs/metrics/" currently has selected but the
page's correct active tab is the <nve-tabs-item> for "Testing & Performance"
(the one linking to "/docs/metrics/testing-and-performance/"); remove the
selected attribute from the Metrics tab so only the Testing & Performance
<nve-tabs-item> remains selected to prevent conflicting active-state behavior.
| <nve-tabs-item selected><a href="docs/metrics/wireit/">Wireit Explorer</a></nve-tabs-item> | ||
| <nve-tabs-item><a href="docs/metrics/bundle-explorer/">Bundle Explorer</a></nve-tabs-item> | ||
| <nve-tabs-item><a href="docs/metrics/metadata/">Raw Metadata</a></nve-tabs-item> | ||
| <nve-tabs-item selected><a href="/docs/metrics/">Metrics</a></nve-tabs-item> |
There was a problem hiding this comment.
Remove the duplicate selected attribute.
Two tabs are marked as selected (lines 59 and 62), but only the "Wireit Explorer" tab (line 62) should be selected on this page.
🐛 Proposed fix
- <nve-tabs-item selected><a href="/docs/metrics/">Metrics</a></nve-tabs-item>
+ <nve-tabs-item><a href="/docs/metrics/">Metrics</a></nve-tabs-item>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <nve-tabs-item selected><a href="/docs/metrics/">Metrics</a></nve-tabs-item> | |
| <nve-tabs-item><a href="/docs/metrics/">Metrics</a></nve-tabs-item> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/wireit.11ty.js` at line 59, Duplicate selected
attribute: remove the extra selected on the Metrics tab so only the Wireit
Explorer tab remains selected. Locate the <nve-tabs-item> element that renders
the "Metrics" tab (the tag containing "<a href=\"/docs/metrics/\">Metrics</a>")
and delete the selected attribute from that element, leaving the selected
attribute only on the "Wireit Explorer" <nve-tabs-item>.
Summary by CodeRabbit
Bug Fixes
New Features