Unravel pages (the array + map object) (github#16708)

* Revise the 'pages' module to export two methods: 'loadPages' and 'loadPageMap'

Update all existing references to use 'loadPages' for now

* Remove explicit Promise resolutions on loadPage* methods

* Condense reduction method into its now-singular usage spot

* Opt for for-of instead of forEach

* Make require of pages in warm-server more explicit

* Be more explicit about find-page using a pageMap

* Be more explicit about find-page-in-version using a pageMap

* Be more explicit about site-tree using a pageMap

* Extract the map creation from loadPageMap

* Be more explicit about using a pageMap

* Update redirects precompile to take two arguments: pageList, pageMap

* Rename internal loadPages method to loadPageList

* Clarify pageMap is what is stored in context.pages

* Use loadPageMap in tests and stuff

Author: JamesMGreene

lib/algolia/find-indexable-pages.js

@@ -1,4 +1,4 @@
-const loadPages = require('../pages')
+const { loadPages } = require('../pages')
 
 module.exports = async function findIndexablePages () {
   const allPages = await loadPages()

lib/find-page-in-version.js

@@ -1,9 +1,9 @@
 const findPage = require('./find-page')
 const getApplicableVersions = require('./get-applicable-versions')
 
-module.exports = function findPageInVersion (href, pages, redirects, languageCode, version, isDotcomOnly = false) {
+module.exports = function findPageInVersion (href, pageMap, redirects, languageCode, version, isDotcomOnly = false) {
   // findPage() will throw an error if an English page can't be found
-  const page = findPage(href, pages, redirects, languageCode)
+  const page = findPage(href, pageMap, redirects, languageCode)
   if (!page) return null
 
   // if the link is on the homepage, return the page as soon as it's found

lib/find-page.js

@@ -3,7 +3,7 @@ const patterns = require('./patterns')
 const allVersions = Object.keys(require('./all-versions'))
 const { getVersionedPathWithLanguage } = require('./path-utils')
 
-module.exports = function findPage (href, pages, redirects = {}, languageCode = 'en', sourceLanguage = null) {
+module.exports = function findPage (href, pageMap, redirects = {}, languageCode = 'en', sourceLanguage = null) {
   // Convert Windows backslashes to forward slashes
   // remove trailing slash
   href = slash(href).replace(patterns.trailingSlash, '$1')
@@ -16,14 +16,14 @@ module.exports = function findPage (href, pages, redirects = {}, languageCode =
   // get the first found path of the page (account for redirects)
   let pathToPage = versionedPathsToCheck.find(path => {
     path = redirects[path] || path
-    return pages[removeFragment(path)]
+    return pageMap[removeFragment(path)]
   })
 
   // need to account for redirects again
   pathToPage = redirects[pathToPage] || pathToPage
 
   // find the page
-  const page = pages[removeFragment(pathToPage)]
+  const page = pageMap[removeFragment(pathToPage)]
 
   if (page) return page
 
@@ -52,7 +52,7 @@ module.exports = function findPage (href, pages, redirects = {}, languageCode =
   // because localized content is not yet synced
   if (languageCode !== 'en') {
     // pass the source language so we can surface it in error messages
-    return findPage(href, pages, redirects, 'en', languageCode)
+    return findPage(href, pageMap, redirects, 'en', languageCode)
   }
 }
 

lib/find-unused-assets.js

@@ -6,7 +6,7 @@ const walk = require('walk-sync')
 const { execSync } = require('child_process')
 const assert = require('assert')
 const loadSiteData = require('./site-data')
-const loadPages = require('./pages')
+const { loadPages } = require('./pages')
 const patterns = require('./patterns')
 const getDataReferences = require('./get-liquid-data-references')
 const imagesPath = '/assets/images'

lib/get-english-headings.js

@@ -2,13 +2,13 @@
 const headingRegex = /^###?#? (.*?)$/gm
 
 // for any translated page, first get corresponding English markdown
-// then get the headings on both the translated and English pages
+// then get the headings on both the translated and English pageMap
 // finally, create a map of translation:English for all headings on the page
-module.exports = function getEnglishHeadings (page, pages) {
+module.exports = function getEnglishHeadings (page, pageMap) {
   const translatedHeadings = page.markdown.match(headingRegex)
   if (!translatedHeadings) return
 
-  const englishPage = pages[`/en/${page.relativePath.replace(/.md$/, '')}`]
+  const englishPage = pageMap[`/en/${page.relativePath.replace(/.md$/, '')}`]
   if (!englishPage) return
 
   // FIX  there may be bugs if English headings are updated before Crowdin syncs up :/

lib/get-map-topic-content.js

@@ -1,9 +1,9 @@
 const findPage = require('./find-page')
 
 // get the page.childArticles set on english map topics in lib/site-tree.js
-module.exports = function getMapTopicContent (page, pages, redirects) {
+module.exports = function getMapTopicContent (page, pageMap, redirects) {
   const englishPage = page.languageCode !== 'en'
-    ? findPage(`/${page.relativePath.replace(/.md$/, '')}`, pages, redirects, 'en')
+    ? findPage(`/${page.relativePath.replace(/.md$/, '')}`, pageMap, redirects, 'en')
     : page
 
   if (!englishPage) {

lib/pages.js

@@ -4,8 +4,8 @@ const Page = require('./page')
 const languages = require('./languages')
 const fs = require('fs')
 
-module.exports = async function () {
-  const pages = []
+async function loadPageList () {
+  const pageList = []
 
   // load english pages
   const englishPath = path.join(__dirname, '..', languages.en.dir, 'content')
@@ -15,7 +15,8 @@ module.exports = async function () {
       !relativePath.includes('README')
     })
     .map(fileData => new Page({ ...fileData, languageCode: languages.en.code }))
-  pages.push(...englishPages)
+
+  pageList.push(...englishPages)
 
   // load matching pages in other languages
   for (const page of englishPages) {
@@ -29,20 +30,40 @@ module.exports = async function () {
       } catch (_) {
         continue
       }
-      pages.push(new Page({
+
+      pageList.push(new Page({
         relativePath: page.relativePath,
         basePath,
         languageCode: language.code
       }))
     }
   }
 
-  // add named keys to the array for fast object-like reference
-  for (const page of pages) {
-    page.permalinks.forEach(permalink => {
-      pages[permalink.href] = page
-    })
-  }
+  return pageList
+}
+
+function createMapFromArray (pageList) {
+  // add keys to the object for each of the page's permalinks for fast lookup
+  const pageMap =
+    pageList.reduce(
+      (pageMap, page) => {
+        for (const permalink of page.permalinks) {
+          pageMap[permalink.href] = page
+        }
+        return pageMap
+      },
+      {}
+    )
+
+  return pageMap
+}
+
+async function loadPageMap (pageList) {
+  const pages = pageList || await loadPageList()
+  return createMapFromArray(pages)
+}
 
-  return Promise.resolve(pages)
+module.exports = {
+  loadPages: loadPageList,
+  loadPageMap
 }

lib/redirects/get-docs-path-from-developer-path.js

@@ -4,7 +4,7 @@ const { getVersionedPathWithLanguage } = require('../path-utils')
 
 // This function takes a known pre-migration path from developer.github.com and
 // infers and returns a current, correct docs.github.com path.
-module.exports = function getDocsPathFromDeveloperPath (oldDeveloperPath, allRedirects, pages) {
+module.exports = function getDocsPathFromDeveloperPath (oldDeveloperPath, allRedirects, pageMap) {
   let newPath = oldDeveloperPath
 
   // Look up the old path in the redirects object BEFORE modifying /v3 and /v4 paths
@@ -59,7 +59,7 @@ module.exports = function getDocsPathFromDeveloperPath (oldDeveloperPath, allRed
 
   // show an error if the page to be redirected to doesn't exist
   // but only if the current collection of pages includes REST and GraphQL reference pages
-  if (process.env.NODE_ENV !== 'test' && !pages[newPath] && pages['/en/rest/reference'] && pages['/en/graphql/reference']) {
+  if (process.env.NODE_ENV !== 'test' && !pageMap[newPath] && pageMap['/en/rest/reference'] && pageMap['/en/graphql/reference']) {
     console.error(`can't find ${newPath}! based on ${oldDeveloperPath}`)
   }
 

lib/redirects/precompile.js

@@ -10,12 +10,12 @@ const nonEnterpriseDefaultVersion = require('../non-enterprise-default-version')
 
 // This function runs at server warmup and precompiles possible redirect routes.
 // It outputs them in key-value pairs within a neat Javascript object: { oldPath: newPath }
-module.exports = async function precompileRedirects (pages) {
+module.exports = async function precompileRedirects (pageList, pageMap) {
   const allRedirects = {}
 
   // 1. CURRENT PAGES PERMALINKS AND FRONTMATTER
   // These redirects are created in lib/page.js via lib/redirects/permalinks.js
-  pages.forEach(page => Object.assign(allRedirects, page.redirects))
+  pageList.forEach(page => Object.assign(allRedirects, page.redirects))
 
   // 2. REDIRECTS FOR ARCHIVED ROUTES FROM 2.13 TO 2.17
   // Starting with 2.18, we updated the archival script to create stubbed HTML redirect files,
@@ -41,7 +41,7 @@ module.exports = async function precompileRedirects (pages) {
   // Note that the list only includes supported enterprise paths up to 2.21, which is
   // the last version that was available on developer.github.com before the migration.
   DEVELOPER_ROUTES.forEach(developerRoute => {
-    const newPath = getDocsPathFromDevPath(developerRoute, allRedirects, pages)
+    const newPath = getDocsPathFromDevPath(developerRoute, allRedirects, pageMap)
 
     // add the redirect to the object
     allRedirects[developerRoute] = newPath

lib/site-tree.js

@@ -14,7 +14,7 @@ const allVersions = Object.keys(require('./all-versions'))
 // or if a category has direct child articles:
 // siteTree[languageCode][version].products[productHref].categories[categoryHref].articles
 
-module.exports = async function buildSiteTree (pages, site, redirects) {
+module.exports = async function buildSiteTree (pageMap, site, redirects) {
   const siteTree = {}
 
   languageCodes.forEach(languageCode => {
@@ -38,12 +38,12 @@ module.exports = async function buildSiteTree (pages, site, redirects) {
         product.href = item.href
 
         // find the product TOC page and get TOC items
-        const page = findPageInVersion(item.href, pages, redirects, languageCode, version)
+        const page = findPageInVersion(item.href, pageMap, redirects, languageCode, version)
 
         // skip if page can't be found in this version
         if (!page) return
 
-        product.categories = buildCategoriesTree(page.tocItems, item.href, pages, redirects, version, languageCode)
+        product.categories = buildCategoriesTree(page.tocItems, item.href, pageMap, redirects, version, languageCode)
 
         productTree[item.id] = product
         return null
@@ -58,7 +58,7 @@ module.exports = async function buildSiteTree (pages, site, redirects) {
   return siteTree
 }
 
-function buildCategoriesTree (tocItems, productHref, pages, redirects, version, languageCode) {
+function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version, languageCode) {
   const categoryTree = {}
 
   // for every category in a product TOC...
@@ -71,7 +71,7 @@ function buildCategoriesTree (tocItems, productHref, pages, redirects, version,
     category.href = versionedCategoryHref
 
     // find the category TOC page and get its TOC items
-    const page = findPageInVersion(categoryHref, pages, redirects, languageCode, version)
+    const page = findPageInVersion(categoryHref, pageMap, redirects, languageCode, version)
 
     // skip if page can't be found in this version
     if (!page) return
@@ -90,9 +90,9 @@ function buildCategoriesTree (tocItems, productHref, pages, redirects, version,
       // if TOC contains maptopics, build a maptopics tree
       // otherwise build an articles tree
       if (hasMaptopics) {
-        category.maptopics = buildMaptopicsTree(page.tocItems, categoryHref, pages, redirects, version, languageCode)
+        category.maptopics = buildMaptopicsTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
       } else {
-        category.articles = buildArticlesTree(page.tocItems, categoryHref, pages, redirects, version, languageCode)
+        category.articles = buildArticlesTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
       }
     }
 
@@ -102,7 +102,7 @@ function buildCategoriesTree (tocItems, productHref, pages, redirects, version,
   return categoryTree
 }
 
-function buildMaptopicsTree (tocItems, categoryHref, pages, redirects, version, languageCode) {
+function buildMaptopicsTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
   const maptopicTree = {}
 
   // for every maptopic in a category TOC...
@@ -118,7 +118,7 @@ function buildMaptopicsTree (tocItems, categoryHref, pages, redirects, version,
 
       // we already have access to the child articles via the category TOC items
       // but we still need the page to get the available versions
-      const page = findPageInVersion(maptopicHref, pages, redirects, languageCode, version)
+      const page = findPageInVersion(maptopicHref, pageMap, redirects, languageCode, version)
 
       // skip if page can't be found in this version
       if (!page) return
@@ -135,14 +135,14 @@ function buildMaptopicsTree (tocItems, categoryHref, pages, redirects, version,
       // make the child articles accessible to the page object for maptopic rendering
       if (!page.childArticles) page.childArticles = childArticles
 
-      maptopic.articles = buildArticlesTree(childArticles, categoryHref, pages, redirects, version, languageCode)
+      maptopic.articles = buildArticlesTree(childArticles, categoryHref, pageMap, redirects, version, languageCode)
       maptopicTree[versionedMaptopicHref] = maptopic
     })
 
   return maptopicTree
 }
 
-function buildArticlesTree (tocItems, categoryHref, pages, redirects, version, languageCode) {
+function buildArticlesTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
   const articleTree = {}
 
   // REST categories may not have TOC items
@@ -157,7 +157,7 @@ function buildArticlesTree (tocItems, categoryHref, pages, redirects, version, l
     const versionedArticleHref = getVersionedPathWithoutLanguage(articleHref, version)
     article.href = versionedArticleHref
 
-    const page = findPageInVersion(articleHref, pages, redirects, languageCode, version)
+    const page = findPageInVersion(articleHref, pageMap, redirects, languageCode, version)
 
     // skip if page can't be found in this version
     if (!page) return

lib/warm-server.js

@@ -1,19 +1,21 @@
 const statsd = require('./statsd')
-const loadPages = require('./pages')
+const { loadPages, loadPageMap } = require('./pages')
 const loadRedirects = require('./redirects/precompile')
 const loadSiteData = require('./site-data')
 const loadSiteTree = require('./site-tree')
 
 // For local caching
-let pages, site, redirects, siteTree
+let pageList, pageMap, site, redirects, siteTree
 
 function isFullyWarmed () {
-  return Boolean(pages && site && redirects && siteTree)
+  // NOTE: Yes, `pageList` is specifically excluded here as it is transient data
+  const fullyWarmed = !!(pageMap && site && redirects && siteTree)
+  return fullyWarmed
 }
 
 function getWarmedCache () {
   return {
-    pages,
+    pages: pageMap,
     site,
     redirects,
     siteTree
@@ -27,20 +29,24 @@ async function warmServer () {
     console.log('Priming context information...')
   }
 
-  if (!pages || !site) {
+  if (!pageList || !site) {
     // Promise.all is used to load multiple things in parallel
-    [pages, site] = await Promise.all([
-      pages || loadPages(),
+    [pageList, site] = await Promise.all([
+      pageList || loadPages(),
       site || loadSiteData()
     ])
   }
 
+  if (!pageMap) {
+    pageMap = await loadPageMap(pageList)
+  }
+
   if (!redirects) {
-    redirects = await loadRedirects(pages)
+    redirects = await loadRedirects(pageList, pageMap)
   }
 
   if (!siteTree) {
-    siteTree = await loadSiteTree(pages, site, redirects)
+    siteTree = await loadSiteTree(pageMap, site, redirects)
   }
 
   if (process.env.NODE_ENV !== 'test') {

middleware/context.js

@@ -12,7 +12,8 @@ const featureFlags = Object.keys(require('../feature-flags'))
 // Note that additional middleware in middleware/index.js adds to this context object
 module.exports = async function contextualize (req, res, next) {
   // Ensure that we load some data only once on first request
-  const { site, redirects, pages, siteTree } = await warmServer()
+  const { site, redirects, siteTree, pages: pageMap } = await warmServer()
+
   req.context = {}
 
   // make feature flag environment variables accessible in layouts
@@ -39,7 +40,7 @@ module.exports = async function contextualize (req, res, next) {
   req.context.redirects = redirects
   req.context.site = site[req.language].site
   req.context.siteTree = siteTree
-  req.context.pages = pages
+  req.context.pages = pageMap
 
   return next()
 }

script/check-s3-images.js

@@ -2,7 +2,7 @@
 
 const path = require('path')
 const { chain, difference } = require('lodash')
-const loadPages = require('../lib/pages')
+const { loadPages } = require('../lib/pages')
 const loadSiteData = require('../lib/site-data')
 const renderContent = require('../lib/render-content')
 const allVersions = require('../lib/all-versions')