refactor: add changes after review

Author: joyenjoyer

.github/workflows/deploy.yml

@@ -30,18 +30,24 @@ jobs:
           case "${{ github.ref }}" in
             "refs/heads/master")
               echo "::set-output name=deploy_dir::./"
+              echo "::set-output name=public_path::/"
               ;;
             "refs/heads/v8_maintenance")
               echo "::set-output name=deploy_dir::v8"
+              echo "::set-output name=public_path::/v8/"
               ;;
             "refs/heads/v9_maintenance")
               echo "::set-output name=deploy_dir::v9"
+              echo "::set-output name=public_path::/v9/"
               ;;
             "refs/heads/v10_maintenance")
               echo "::set-output name=deploy_dir::v10"
+              echo "::set-output name=public_path::/v10/"
               ;;
           esac
       - name: Build docs-app
+        env:
+          PUBLIC_PATH: ${{ steps.set-build-dir.outputs.public_path }}
         run: pnpm run build:docs
       - name: Copy index.html to 404.html for GitHub Pages
         run: cp ./packages/__docs__/__build__/index.html ./packages/__docs__/__build__/404.html

.github/workflows/preview.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     env:
       GITHUB_PULL_REQUEST_PREVIEW: 'true'
-      PR_NUMBER: ${{ github.event.pull_request.number }}
+      PUBLIC_PATH: /pr-preview/pr-${{ github.event.pull_request.number }}/
     steps:
       - uses: actions/checkout@v4
       - uses: pnpm/action-setup@v4

packages/__docs__/src/App/index.tsx

@@ -70,7 +70,7 @@ import {
 import {
   parseCurrentUrl,
   navigateToVersion,
-  getAssetBasePath,
+  getDeployBase,
   MINOR_VERSION_REGEX
 } from '../navigationUtils'
 
@@ -136,7 +136,7 @@ class App extends Component<AppProps, AppState> {
   }
 
   getDocsBasePath = () => {
-    const base = getAssetBasePath()
+    const base = getDeployBase()
     const { selectedMinorVersion } = this.state
     if (selectedMinorVersion) {
       return `${base}/docs/${selectedMinorVersion}/`
@@ -212,13 +212,13 @@ class App extends Component<AppProps, AppState> {
     navigateToVersion(newVersion)
 
     this.fetchMainDocsData(
-      `${getAssetBasePath()}/docs/${newVersion}/markdown-and-sources-data.json`,
+      `${getDeployBase()}/docs/${newVersion}/markdown-and-sources-data.json`,
       signal
     ).catch(errorHandler)
 
     // Icons are not version-specific; only re-fetch if not already loaded
     if (!this.state.legacyIconsData) {
-      fetch(`${getAssetBasePath()}/legacy-icons-data.json`, { signal })
+      fetch(`${getDeployBase()}/legacy-icons-data.json`, { signal })
         .then((response) => response.json())
         .then((legacyIconsData) => {
           this.setState({ legacyIconsData })
@@ -282,7 +282,7 @@ class App extends Component<AppProps, AppState> {
     this.fetchVersionData(signal).catch(errorHandler)
     document.addEventListener('keydown', this.handleTabKey)
 
-    fetch(`${getAssetBasePath()}/legacy-icons-data.json`, { signal })
+    fetch(`${getDeployBase()}/legacy-icons-data.json`, { signal })
       .then((response) => response.json())
       .then((iconsData) => this.setState({ legacyIconsData: iconsData }))
       .catch(errorHandler)
@@ -304,13 +304,13 @@ class App extends Component<AppProps, AppState> {
             selectedMinorVersion
           })
           return this.fetchMainDocsData(
-            `${getAssetBasePath()}/docs/${selectedMinorVersion}/markdown-and-sources-data.json`,
+            `${getDeployBase()}/docs/${selectedMinorVersion}/markdown-and-sources-data.json`,
             signal
           )
         }
         // No minor versions available, fetch from root path
         return this.fetchMainDocsData(
-          `${getAssetBasePath()}/markdown-and-sources-data.json`,
+          `${getDeployBase()}/markdown-and-sources-data.json`,
           signal
         )
       })
@@ -360,23 +360,21 @@ class App extends Component<AppProps, AppState> {
   getPathInfo = () => {
     const { hash, pathname } = window.location
 
-    const cleanPath = pathname.replace(/^\/+|\/+$/g, '')
-    const segments = cleanPath.split('/').filter(Boolean)
-
-    // Skip PR preview prefix
-    let idx = 0
+    // Strip the deploy base (e.g. '/latest', '/pr-preview/pr-123',
+    // '/<repo>/pr-preview/pr-N') before parsing app segments.
+    const deployBase = getDeployBase()
+    let rest = pathname
     if (
-      segments.length >= 2 &&
-      segments[0] === 'pr-preview' &&
-      segments[1].startsWith('pr-')
+      deployBase &&
+      (rest === deployBase || rest.startsWith(deployBase + '/'))
     ) {
-      idx = 2
+      rest = rest.slice(deployBase.length)
     }
 
-    // Skip /latest/ prefix
-    if (idx === 0 && segments[idx] === 'latest') {
-      idx++
-    }
+    const cleanPath = rest.replace(/^\/+|\/+$/g, '')
+    const segments = cleanPath.split('/').filter(Boolean)
+
+    let idx = 0
 
     // Skip minor version prefix (e.g. v11_7)
     if (idx < segments.length && MINOR_VERSION_REGEX.test(segments[idx])) {

packages/__docs__/src/index.html

@@ -6,14 +6,15 @@
          deploy base -->
     <script>
       ;(function () {
-        var base = '<%= PUBLIC_PATH %>'
-        var l = window.location
-        if (l.pathname === base) return
-        if (l.pathname + '/' === base) return
-        if (l.pathname.indexOf(base) !== 0) return
-        var rest = l.pathname.slice(base.length)
-        l.replace(
-          base + '?__spa_route=' + encodeURIComponent('/' + rest + l.hash)
+        const base = '<%= PUBLIC_PATH %>'
+        const { pathname, search, hash } = window.location
+        if (pathname === base || pathname + '/' === base) return
+        if (!pathname.startsWith(base)) return
+        const rest = pathname.slice(base.length)
+        window.location.replace(
+          base +
+            '?__spa_route=' +
+            encodeURIComponent('/' + rest + search + hash)
         )
       })()
     </script>

packages/__docs__/src/navigationUtils.ts

@@ -29,16 +29,14 @@ declare const __webpack_public_path__: string
 const MINOR_VERSION_REGEX = /^v\d+_\d+$/
 
 type ParsedUrl = {
-  basePrefix: string
   minorVersion: string | null
   page: string
   sectionId: string | undefined
 }
 
 /**
- * Returns '' for a root deploy, '/latest' on instructure.design,
- * '/pr-preview/pr-<n>' for PR previews. For sub-host deployments it can be
- * '/<repo>/pr-preview/pr-<n>'.
+ * Returns webpack's `output.publicPath` with the trailing slash stripped.
+ * Always begins with `/`, or is empty for a root deploy.
  */
 function getDeployBase(): string {
   if (typeof __webpack_public_path__ === 'string' && __webpack_public_path__) {
@@ -52,21 +50,18 @@ function parseCurrentUrl(): ParsedUrl {
 
   const deployBase = getDeployBase()
   let rest = pathname
-  if (deployBase && rest.startsWith(deployBase)) {
+  if (
+    deployBase &&
+    (rest === deployBase || rest.startsWith(deployBase + '/'))
+  ) {
     rest = rest.slice(deployBase.length)
   }
 
   const cleanPath = rest.replace(/^\/+|\/+$/g, '')
   const segments = cleanPath.split('/').filter(Boolean)
 
-  let appPrefix = ''
   let idx = 0
 
-  if (segments[idx] === 'latest') {
-    appPrefix = '/latest'
-    idx++
-  }
-
   // Detect minor version prefix: /v11_7
   let minorVersion: string | null = null
   if (idx < segments.length && MINOR_VERSION_REGEX.test(segments[idx])) {
@@ -83,7 +78,7 @@ function parseCurrentUrl(): ParsedUrl {
     sectionId = decodeURI(hash.replace(/^#+/, ''))
   }
 
-  return { basePrefix: deployBase + appPrefix, minorVersion, page, sectionId }
+  return { minorVersion, page, sectionId }
 }
 
 type BuildUrlOptions = {
@@ -98,7 +93,7 @@ function buildUrl(targetPage: string, options?: BuildUrlOptions): string {
       ? options.minorVersion
       : parsed.minorVersion
 
-  let url = parsed.basePrefix
+  let url = getDeployBase()
 
   if (minorVersion) {
     url += `/${minorVersion}`
@@ -130,20 +125,12 @@ function navigateToVersion(version: string | null): void {
   window.dispatchEvent(new PopStateEvent('popstate'))
 }
 
-/**
- * Returns the base path prefix for fetching static assets.
- * On PR previews this is e.g. `/pr-preview/pr-2425`, otherwise empty string.
- */
-function getAssetBasePath(): string {
-  return getDeployBase()
-}
-
 export {
   parseCurrentUrl,
   buildUrl,
   navigateTo,
   navigateToVersion,
-  getAssetBasePath,
+  getDeployBase,
   MINOR_VERSION_REGEX
 }
 export type { ParsedUrl }

packages/__docs__/src/versionData.ts

@@ -55,7 +55,7 @@ const fetchVersionData = async (signal: AbortSignal) => {
 }
 
 import type { MinorVersionData } from '../buildScripts/DataTypes.mts'
-import { getAssetBasePath } from './navigationUtils'
+import { getDeployBase } from './navigationUtils'
 
 /**
  * Fetches minor version data from docs-versions.json.
@@ -66,7 +66,9 @@ const fetchMinorVersionData = async (
   signal: AbortSignal
 ): Promise<MinorVersionData | undefined> => {
   try {
-    const result = await fetch(`${getAssetBasePath()}/docs-versions.json`, { signal })
+    const result = await fetch(`${getDeployBase()}/docs-versions.json`, {
+      signal
+    })
     if (!result.ok) {
       return undefined
     }

packages/__docs__/webpack.config.mjs

@@ -31,15 +31,19 @@ import webpack from 'webpack'
 const ENV = process.env.NODE_ENV || 'production'
 const DEBUG = process.env.DEBUG || ENV === 'development'
 const GITHUB_PULL_REQUEST_PREVIEW = process.env.GITHUB_PULL_REQUEST_PREVIEW || 'false'
-const PR_NUMBER = process.env.PR_NUMBER
-// The URL prefix this build is deployed under. Must end with a trailing
+// The URL prefix this build is deployed under. Must begin and end with a
 // slash. Fed into output.publicPath (which webpack exposes at runtime as
 // __webpack_public_path__) and into the HTML template's <base href> and
-// 404 SPA-redirect script. Keeping a single source here means a new deploy
-// target only needs to set this one env var.
-const PUBLIC_PATH =
-  process.env.PUBLIC_PATH ||
-  (PR_NUMBER ? `/pr-preview/pr-${PR_NUMBER}/` : '/')
+// 404 SPA-redirect script. Every CI workflow sets this explicitly; the
+// fallback to '/' only exists for local dev (`pnpm dev`).
+const PUBLIC_PATH = process.env.PUBLIC_PATH || '/'
+
+if (!/^\/([^?#]*\/)?$/.test(PUBLIC_PATH)) {
+  throw new Error(
+    `PUBLIC_PATH must begin and end with a slash and contain no query or fragment (got: "${PUBLIC_PATH}"). ` +
+    `Examples: "/", "/latest/", "/pr-preview/pr-123/", "/repo/pr-preview/pr-123/"`
+  )
+}
 
 const outputPath = resolvePath(import.meta.dirname, '__build__')