Simplify getBreadcrumbs (github#26015)

* fix getBreadcrumbs

* don't go too deep

* remove commented out code

Author: peterbe

components/page-header/Breadcrumbs.tsx

@@ -6,7 +6,6 @@ import styles from './Breadcrumbs.module.scss'
 
 export type BreadcrumbT = {
   title: string
-  documentType?: string
   href?: string
 }
 

middleware/contextualizers/breadcrumbs.js

@@ -1,3 +1,5 @@
+import liquid from '../../lib/render-content/liquid.js'
+
 export default async function breadcrumbs(req, res, next) {
   if (!req.context.page) return next()
   if (req.context.page.hidden) return next()
@@ -9,95 +11,42 @@ export default async function breadcrumbs(req, res, next) {
     return next()
   }
 
-  const currentSiteTree =
-    req.context.siteTree[req.context.currentLanguage][req.context.currentVersion]
-  const fallbackSiteTree = req.context.siteTree.en[req.context.currentVersion]
-
-  req.context.breadcrumbs = await getBreadcrumbs(
-    // Array of child pages on the root, i.e., the product level.
-    currentSiteTree.childPages,
-    fallbackSiteTree.childPages,
-    req.context.currentPath.slice(3),
-    req.context.currentLanguage
-  )
+  req.context.breadcrumbs = await getBreadcrumbs(req)
 
   return next()
 }
 
-async function getBreadcrumbs(
-  pageArray,
-  fallbackPageArray,
-  currentPathWithoutLanguage,
-  intendedLanguage
-) {
-  // Find the page that starts with the requested path
-  let childPage = findPageWithPath(currentPathWithoutLanguage, pageArray)
-
-  // Find the page in the fallback page array (likely the English sub-tree)
-  const fallbackChildPage =
-    findPageWithPath(currentPathWithoutLanguage, fallbackPageArray || []) || childPage
-
-  // No matches, we bail
-  if (!childPage && !fallbackChildPage) {
-    return []
-  }
-
-  // Didn't find the intended page, but found the fallback
-  if (!childPage) {
-    childPage = fallbackChildPage
+async function getBreadcrumbs(req) {
+  const crumbs = []
+  const { currentPath, currentVersion } = req.context
+  const split = currentPath.split('/')
+  while (split.length > 2 && split[split.length - 1] !== currentVersion) {
+    const href = split.join('/')
+    const page = req.context.pages[href]
+    crumbs.push({
+      href,
+      title: await getShortTitle(page, req.context),
+    })
+    split.pop()
   }
+  crumbs.reverse()
 
-  const breadcrumb = {
-    documentType: childPage.page.documentType,
-    // give the breadcrumb the intendedLanguage, so nav through breadcrumbs doesn't inadvertantly change the user's selected language
-    href: `/${intendedLanguage}/${childPage.href.slice(4)}`,
-    title: childPage.renderedShortTitle || childPage.renderedFullTitle,
-  }
-
-  // Recursively loop through the childPages and create each breadcrumb, until we reach the
-  // point where the current siteTree page is the same as the requested page. Then stop.
-  if (childPage.childPages && currentPathWithoutLanguage !== childPage.href.slice(3)) {
-    return [
-      breadcrumb,
-      ...(await getBreadcrumbs(
-        childPage.childPages,
-        fallbackChildPage.childPages,
-        currentPathWithoutLanguage,
-        intendedLanguage
-      )),
-    ]
-  } else {
-    return [breadcrumb]
-  }
+  return crumbs
 }
 
-// Finds the page that starts with or equals the requested path in the array of
-// pages e.g. if the current page is /actions/learn-github-actions/understanding-github-actions,
-// depending on the pages in the pageArray agrument, would find:
-//
-// * /actions
-// * /actions/learn-github-actions
-// * /actions/learn-github-actions/understanding-github-actions
-function findPageWithPath(pageToFind, pageArray) {
-  return pageArray.find((page) => {
-    const pageWithoutLanguage = page.href.slice(3)
-    const numPathSegments = pageWithoutLanguage.split('/').length
-    const pageToFindNumPathSegments = pageToFind.split('/').length
-
-    if (pageToFindNumPathSegments > numPathSegments) {
-      // if the current page to find has more path segments, add a trailing
-      // slash to the page comparison to avoid an overlap like:
-      //
-      // * /github-cli/github-cli/about-github-cli with /github
-      return pageToFind.startsWith(`${pageWithoutLanguage}/`)
-    } else if (pageToFindNumPathSegments === numPathSegments) {
-      // if the current page has the same number of path segments, only match
-      // if the paths are the same to avoid an overlap like:
-      //
-      // * /get-started/using-github with /get-started/using-git
-      return pageToFind === pageWithoutLanguage
-    } else {
-      return false
+async function getShortTitle(page, context) {
+  if (page.rawShortTitle) {
+    if (page.rawShortTitle.includes('{')) {
+      // Can't easily cache this because the `page` is reused for multiple
+      // permalinks. We could do what the `Page.render()` method does which
+      // specifically caches based on the `context.currentPath` but at
+      // this point it's probably not worth it.
+      return await liquid.parseAndRender(page.rawShortTitle, context)
     }
-  })
+    return page.shortTitle
+  }
+  if (page.rawTitle.includes('{')) {
+    return await liquid.parseAndRender(page.rawTitle, context)
+  }
+  return page.title
 }

tests/rendering/breadcrumbs.js

@@ -120,7 +120,6 @@ describe('breadcrumbs', () => {
       const breadcrumbs = await getJSON('/en/github?json=breadcrumbs')
       const expected = [
         {
-          documentType: 'product',
           href: '/en/github',
           title: 'GitHub',
         },
@@ -134,17 +133,14 @@ describe('breadcrumbs', () => {
       )
       const expected = [
         {
-          documentType: 'product',
           href: '/en/issues',
           title: 'GitHub Issues',
         },
         {
-          documentType: 'category',
           href: '/en/issues/tracking-your-work-with-issues',
           title: 'Issues',
         },
         {
-          documentType: 'article',
           href: '/en/issues/tracking-your-work-with-issues/quickstart',
           title: 'Quickstart for GitHub Issues',
         },
@@ -158,17 +154,14 @@ describe('breadcrumbs', () => {
       )
       const expected = [
         {
-          documentType: 'product',
           href: '/en/account-and-profile',
           title: 'Account and profile',
         },
         {
-          documentType: 'category',
           href: '/en/account-and-profile/setting-up-and-managing-your-github-user-account',
           title: 'User accounts',
         },
         {
-          documentType: 'mapTopic',
           href: '/en/account-and-profile/setting-up-and-managing-your-github-user-account/managing-user-account-settings',
           title: 'User account settings',
         },
@@ -182,22 +175,18 @@ describe('breadcrumbs', () => {
       )
       const expected = [
         {
-          documentType: 'product',
           href: '/en/account-and-profile',
           title: 'Account and profile',
         },
         {
-          documentType: 'category',
           href: '/en/account-and-profile/setting-up-and-managing-your-github-user-account',
           title: 'User accounts',
         },
         {
-          documentType: 'mapTopic',
           href: '/en/account-and-profile/setting-up-and-managing-your-github-user-account/managing-user-account-settings',
           title: 'User account settings',
         },
         {
-          documentType: 'article',
           href: '/en/account-and-profile/setting-up-and-managing-your-github-user-account/managing-user-account-settings/about-your-personal-dashboard',
           title: 'Your personal dashboard',
         },
@@ -211,17 +200,14 @@ describe('breadcrumbs', () => {
       )
       const expected = [
         {
-          documentType: 'product',
           href: '/en/github',
           title: 'GitHub',
         },
         {
-          documentType: 'category',
           href: '/en/github/site-policy',
           title: 'Site policy',
         },
         {
-          documentType: 'article',
           href: '/en/github/site-policy/github-privacy-statement',
           title: 'GitHub Privacy Statement',
         },