Skip to content

Commit c639d86

Browse files
sarahssarahs
authored
Merge pull request #16947 from github/optimize-sitetree
sitetree optimizations
2 parents b981743 + 5fa3dd8 commit c639d86

11 files changed

Lines changed: 70 additions & 92 deletions

‎includes/breadcrumbs.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
{% if breadcrumb[1].href == '' %}
44
<span>{{breadcrumb[1].title}}</span>
55
{% else %}
6-
<a title="{{ breadcrumb[0]}}: {{breadcrumb[1].title}}" href="/{{currentLanguage}}{{breadcrumb[1].href}}" class="d-inline-block {% if breadcrumb[1].href == currentPathWithoutLanguage %}text-gray-light{% endif %}">
6+
<a title="{{ breadcrumb[0]}}: {{breadcrumb[1].title}}" href="{{{breadcrumb[1].href}}" class="d-inline-block {% if breadcrumb[1].href == currentPath %}text-gray-light{% endif %}">
77
{{breadcrumb[1].title}}</a>
88
{% endif %}
99
{% endfor %}

‎includes/sidebar-guides.html‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
<ul class="sidebar-categories">
22
{% for category in product[1].categories %}
33
<li class="sidebar-category {% if breadcrumbs.category.href == category[1].href %}active{% endif %}">
4-
<a href="/{{currentLanguage}}{{category[1].href}}">{{ category[1].title }}</a>
4+
<a href="{{category[1].href}}">{{ category[1].title }}</a>
55
<!-- some categories have topics with child articles -->
66
{% if category[1].maptopics %}
77
<ul class="sidebar-maptopics">
88
{% for maptopic in category[1].maptopics %}
99
<li class="sidebar-maptopic {% if breadcrumbs.maptopic.href == maptopic[1].href %}active{% endif %}">
10-
<a href="/{{currentLanguage}}{{maptopic[1].href}}">{{ maptopic[1].title }}</a>
10+
<a href="{{maptopic[1].href}}">{{ maptopic[1].title }}</a>
1111
<ul class="sidebar-articles">
1212
{% for article in maptopic[1].articles %}
1313
<li class="sidebar-article {% if currentPath == article[1].href %}active{% endif %}">
14-
<a href="/{{currentLanguage}}{{article[1].href}}">{{ article[1].title }}</a>
14+
<a href="{{article[1].href}}">{{ article[1].title }}</a>
1515
</li>
1616
{% endfor %}
1717
</ul>
@@ -23,7 +23,7 @@
2323
<ul class="sidebar-articles">
2424
{% for article in category[1].articles %}
2525
<li class="sidebar-article{% if currentPath == article[1].href %} active{% endif %}">
26-
<a href="/{{currentLanguage}}{{article[1].href}}">{{ article[1].title }}</a>
26+
<a href="{{article[1].href}}">{{ article[1].title }}</a>
2727
</li>
2828
{% endfor %}
2929
</ul>

‎includes/sidebar-specific-product.html‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@
1010
{% include all-products-link %}
1111
<li title="{{product.title}}" class="sidebar-product mb-2">
1212
{% unless page.hidden %}
13-
<a href="/{{currentLanguage}}{{product.href}}" class="pl-4 pr-5 pb-1 f4">{{ product.title }}</a>
13+
<a href="{{product.href}}" class="pl-4 pr-5 pb-1 f4">{{ product.title }}</a>
1414
{% endunless %}
1515
</li>
1616
<ul class="sidebar-categories list-style-none">
1717
{% for category in product.categories %}
18-
{% capture fullPathToCategory %}/{{currentLanguage}}{{category[1].href}}{% endcapture %}
18+
{% capture fullPathToCategory %}{{category[1].href}}{% endcapture %}
1919

2020
<li class="sidebar-category py-1 {% if breadcrumbs.category.href == category[1].href %}active {% if currentPath == fullPathToCategory %}is-current-page {% endif %}{% endif %}{% if category[1].standalone %}standalone-category{% endif %}">
2121
{% if category[1].standalone %}
@@ -37,13 +37,13 @@
3737
<ul class="sidebar-topics list-style-none position-relative">
3838
{% for maptopic in category[1].maptopics %}
3939
{% unless maptopic[1].hidden %}
40-
{% capture fullPathToMaptopic %}/{{currentLanguage}}{{maptopic[1].href}}{% endcapture %}
40+
{% capture fullPathToMaptopic %}{{maptopic[1].href}}{% endcapture %}
4141

4242
<li class="sidebar-maptopic {% if breadcrumbs.maptopic.href == maptopic[1].href %}active {% if currentPath == fullPathToMaptopic %}is-current-page{% endif %}{% endif %}">
4343
<a href="{{fullPathToMaptopic}}" class="pl-4 pr-5 py-2">{{ maptopic[1].title }}</a>
4444
<ul class="sidebar-articles my-2">
4545
{% for article in maptopic[1].articles %}
46-
{% capture fullPathToArticle %}/{{currentLanguage}}{{article[1].href}}{% endcapture %}
46+
{% capture fullPathToArticle %}{{article[1].href}}{% endcapture %}
4747

4848
<li class="sidebar-article {% if breadcrumbs.article.href == article[1].href %}active {% if currentPath == fullPathToArticle %}is-current-page{% endif %}{% endif %}">
4949
<a href="{{fullPathToArticle}}" class="pl-6 pr-5 py-1{% if forloop.last %} pb-2{% endif %}">{{ article[1].title }}</a>
@@ -58,7 +58,7 @@
5858
{% else %}
5959
<ul class="sidebar-articles list-style-none">
6060
{% for article in category[1].articles %}
61-
{% capture fullPathToArticle %}/{{currentLanguage}}{{article[1].href}}{% endcapture %}
61+
{% capture fullPathToArticle %}{{article[1].href}}{% endcapture %}
6262
<li class="sidebar-article {% if breadcrumbs.article.href == article[1].href %}active {% if currentPath == fullPathToArticle %}is-current-page{% endif %}{% endif %}">
6363
<a href="{{fullPathToArticle}}" class="pl-4 pr-5 py-1{% if forloop.last %} pb-2{% endif %}">{{ article[1].title }}</a>
6464
</li>

‎lib/find-page.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ module.exports = function findPage (href, pageMap, redirects = {}, languageCode
88
// remove trailing slash
99
href = slash(href).replace(patterns.trailingSlash, '$1')
1010

11+
// do an initial lookup on the path as-is
12+
let page = pageMap[removeFragment(href)]
13+
if (page) return page
14+
1115
// check all potential versions
1216
const versionedPathsToCheck = [...new Set(allVersions.map(version => {
1317
return getVersionedPathWithLanguage(href, version, languageCode)
@@ -22,8 +26,8 @@ module.exports = function findPage (href, pageMap, redirects = {}, languageCode
2226
// need to account for redirects again
2327
pathToPage = redirects[pathToPage] || pathToPage
2428

25-
// find the page
26-
const page = pageMap[removeFragment(pathToPage)]
29+
// try finding the page again
30+
page = pageMap[removeFragment(pathToPage)]
2731

2832
if (page) return page
2933

‎lib/get-map-topic-content.js‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,12 @@
1-
const findPage = require('./find-page')
1+
// get the childArticles set on map topics in lib/site-tree.js
2+
module.exports = function getMapTopicContent (parentProductId, breadcrumbs, siteTree) {
3+
const childArticles = siteTree.products[parentProductId].categories[breadcrumbs.category.href].maptopics[breadcrumbs.maptopic.href].childArticles
24

3-
// get the page.childArticles set on english map topics in lib/site-tree.js
4-
module.exports = function getMapTopicContent (page, pageMap, redirects) {
5-
const englishPage = page.languageCode !== 'en'
6-
? findPage(`/${page.relativePath.replace(/.md$/, '')}`, pageMap, redirects, 'en')
7-
: page
8-
9-
if (!englishPage) {
10-
console.error(`cannot find english page: ${page.fullPath}`)
11-
return
12-
}
13-
14-
if (!englishPage.childArticles) {
15-
console.error(`error getting child articles on map topic: ${page.fullPath}`)
16-
return
5+
if (!childArticles) {
6+
console.error(`can't find child articles from siteTree for map topic '${breadcrumbs.maptopic.href}'`)
177
}
188

19-
return englishPage.childArticles
9+
return childArticles
2010
.map(article => `{% link_with_intro /${article.href} %}`)
2111
.join('\n\n')
2212
}

‎lib/page.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ class Page {
148148
this.shortTitle = await renderContent(this.shortTitle, context, { textOnly: true, encodeEntities: true })
149149

150150
let markdown = this.mapTopic
151-
? getMapTopicContent(this, context.pages, context.redirects)
151+
// get the map topic child articles from the siteTree
152+
? getMapTopicContent(this.parentProduct.id, context.breadcrumbs, context.siteTree[context.currentLanguage][context.currentVersion])
152153
: this.markdown
153154

154155
// If the article is interactive parse the React!

‎lib/site-tree.js‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const path = require('path')
22
const findPageInVersion = require('./find-page-in-version')
33
const products = Object.values(require('../lib/all-products'))
4-
const { getVersionedPathWithoutLanguage } = require('./path-utils')
4+
const { getVersionedPathWithLanguage, getPathWithLanguage } = require('./path-utils')
55
const languageCodes = Object.keys(require('./languages'))
66
const addTitlesToTree = require('./site-tree-titles')
77
const allVersions = Object.keys(require('./all-versions'))
@@ -35,15 +35,16 @@ module.exports = async function buildSiteTree (pageMap, site, redirects) {
3535
return
3636
}
3737

38-
product.href = item.href
38+
// we don't want versioned product links because these links already have a default version in them
39+
product.href = getPathWithLanguage(item.href, languageCode)
3940

4041
// find the product TOC page and get TOC items
4142
const page = findPageInVersion(item.href, pageMap, redirects, languageCode, version)
4243

4344
// skip if page can't be found in this version
4445
if (!page) return
4546

46-
product.categories = buildCategoriesTree(page.tocItems, item.href, pageMap, redirects, version, languageCode)
47+
product.categories = buildCategoriesTree(page.tocItems, product.href, pageMap, redirects, version, languageCode)
4748

4849
productTree[item.id] = product
4950
return null
@@ -67,11 +68,12 @@ function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version
6768

6869
const categoryHref = path.join(productHref, item.href)
6970

70-
const versionedCategoryHref = getVersionedPathWithoutLanguage(categoryHref, version)
71+
// we DO want versioned category links
72+
const versionedCategoryHref = getVersionedPathWithLanguage(categoryHref, version, languageCode)
7173
category.href = versionedCategoryHref
7274

7375
// find the category TOC page and get its TOC items
74-
const page = findPageInVersion(categoryHref, pageMap, redirects, languageCode, version)
76+
const page = findPageInVersion(versionedCategoryHref, pageMap, redirects, languageCode, version)
7577

7678
// skip if page can't be found in this version
7779
if (!page) return
@@ -90,9 +92,9 @@ function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version
9092
// if TOC contains maptopics, build a maptopics tree
9193
// otherwise build an articles tree
9294
if (hasMaptopics) {
93-
category.maptopics = buildMaptopicsTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
95+
category.maptopics = buildMaptopicsTree(page.tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode)
9496
} else {
95-
category.articles = buildArticlesTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
97+
category.articles = buildArticlesTree(page.tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode)
9698
}
9799
}
98100

@@ -102,7 +104,7 @@ function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version
102104
return categoryTree
103105
}
104106

105-
function buildMaptopicsTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
107+
function buildMaptopicsTree (tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode) {
106108
const maptopicTree = {}
107109

108110
// for every maptopic in a category TOC...
@@ -111,38 +113,32 @@ function buildMaptopicsTree (tocItems, categoryHref, pageMap, redirects, version
111113
.forEach(item => {
112114
const maptopic = {}
113115

114-
const maptopicHref = path.join(categoryHref, item.href)
115-
116-
const versionedMaptopicHref = getVersionedPathWithoutLanguage(maptopicHref, version)
116+
const versionedMaptopicHref = path.join(versionedCategoryHref, item.href)
117117
maptopic.href = versionedMaptopicHref
118118

119-
// we already have access to the child articles via the category TOC items
120-
// but we still need the page to get the available versions
121-
const page = findPageInVersion(maptopicHref, pageMap, redirects, languageCode, version)
119+
// find the category TOC page and get its TOC items
120+
const page = findPageInVersion(versionedMaptopicHref, pageMap, redirects, languageCode, version)
122121

123122
// skip if page can't be found in this version
124123
if (!page) return
125124

126125
// if this is not a maptopic, return early
127126
if (!page.mapTopic) return
128127

129-
const childArticles = getChildArticles(tocItems, item.href)
130-
131128
maptopic.title = page.title
132129
maptopic.shortTitle = page.shortTitle
133130
maptopic.hidden = page.hidden
134-
135131
// make the child articles accessible to the page object for maptopic rendering
136-
if (!page.childArticles) page.childArticles = childArticles
132+
maptopic.childArticles = getChildArticles(tocItems, item.href)
133+
maptopic.articles = buildArticlesTree(maptopic.childArticles, versionedCategoryHref, pageMap, redirects, version, languageCode)
137134

138-
maptopic.articles = buildArticlesTree(childArticles, categoryHref, pageMap, redirects, version, languageCode)
139135
maptopicTree[versionedMaptopicHref] = maptopic
140136
})
141137

142138
return maptopicTree
143139
}
144140

145-
function buildArticlesTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
141+
function buildArticlesTree (tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode) {
146142
const articleTree = {}
147143

148144
// REST categories may not have TOC items
@@ -152,12 +148,11 @@ function buildArticlesTree (tocItems, categoryHref, pageMap, redirects, version,
152148
tocItems.forEach(item => {
153149
const article = {}
154150

155-
const articleHref = path.join(categoryHref, item.href)
156-
157-
const versionedArticleHref = getVersionedPathWithoutLanguage(articleHref, version)
151+
const versionedArticleHref = path.join(versionedCategoryHref, item.href)
158152
article.href = versionedArticleHref
159153

160-
const page = findPageInVersion(articleHref, pageMap, redirects, languageCode, version)
154+
// find the category TOC page and get its TOC items
155+
const page = findPageInVersion(versionedArticleHref, pageMap, redirects, languageCode, version)
161156

162157
// skip if page can't be found in this version
163158
if (!page) return

‎middleware/breadcrumbs.js‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ module.exports = async (req, res, next) => {
2323
const product = req.context.siteTree[req.language][req.context.currentVersion].products[req.context.currentProduct]
2424

2525
req.context.breadcrumbs.product = {
26-
href: path.posix.join('/', req.context.currentVersion, productPath),
26+
href: path.posix.join('/', req.context.currentLanguage, req.context.currentVersion, productPath),
2727
title: product.title
2828
}
2929

@@ -32,7 +32,7 @@ module.exports = async (req, res, next) => {
3232
// get category path
3333
// e.g., `getting-started-with-github` in /free-pro-team@latest/github/getting-started-with-github
3434
// or /enterprise-server@2.21/github/getting-started-with-github
35-
const categoryPath = path.posix.join('/', req.context.currentVersion, productPath, pathParts[1])
35+
const categoryPath = path.posix.join('/', req.context.currentLanguage, req.context.currentVersion, productPath, pathParts[1])
3636

3737
const category = product.categories[categoryPath]
3838

@@ -49,7 +49,7 @@ module.exports = async (req, res, next) => {
4949
// e.g., /github/getting-started-with-github/learning-about-github
5050
let maptopic
5151
if (req.context.page.mapTopic) {
52-
const maptopicPath = path.posix.join(categoryPath, pathParts[2])
52+
const maptopicPath = req.path
5353

5454
maptopic = category.maptopics[maptopicPath]
5555

@@ -60,9 +60,7 @@ module.exports = async (req, res, next) => {
6060
title: maptopic.shortTitle || maptopic.title
6161
}
6262
} else {
63-
// get article path
64-
// e.g., /github/getting-started-with-github/githubs-products
65-
const articlePath = path.posix.join(categoryPath, pathParts[2])
63+
const articlePath = req.path
6664

6765
// find parent maptopic if one exists
6866
// some categories don't have maptopics, e.g. site-policy
@@ -77,17 +75,7 @@ module.exports = async (req, res, next) => {
7775
}
7876
}
7977

80-
let articleKey = '/' + req.language + articlePath
81-
let articlePage = req.context.pages[articleKey]
82-
83-
// fall back to English if localized article does not exist
84-
if (!articlePage && req.language !== 'en') {
85-
articleKey = '/en' + articlePath
86-
articlePage = req.context.pages[articleKey]
87-
}
88-
89-
if (!articlePage) return next()
90-
78+
const articlePage = req.context.page
9179
const articleTitle = await articlePage.renderProp('shortTitle', req.context, { textOnly: true, encodeEntities: true })
9280

9381
req.context.breadcrumbs.article = {

‎middleware/early-access-breadcrumbs.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ module.exports = async (req, res, next) => {
4040

4141
// get Early Access category path
4242
// e.g., `enforcing-best-practices-with-github-policies` in /free-pro-team@latest/early-access/github/enforcing-best-practices-with-github-policies
43-
const categoryPath = path.posix.join('/', req.context.currentVersion, 'early-access', pathParts[0], pathParts[1])
44-
const category = req.context.pages[path.posix.join('/en', categoryPath)]
43+
const categoryPath = path.posix.join('/', 'en', req.context.currentVersion, 'early-access', pathParts[0], pathParts[1])
44+
const category = req.context.pages[categoryPath]
4545

4646
if (!category) return next()
4747

@@ -54,7 +54,7 @@ module.exports = async (req, res, next) => {
5454

5555
// for Early Access purposes, we don't need to differentiate between map topics and articles breadcrumbs
5656
const mapTopicOrArticlePath = path.posix.join(categoryPath, pathParts[2])
57-
const mapTopicOrArticle = req.context.pages[path.posix.join('/en', mapTopicOrArticlePath)]
57+
const mapTopicOrArticle = req.context.pages[mapTopicOrArticlePath]
5858

5959
if (!mapTopicOrArticle) return next()
6060

‎tests/content/site-tree.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,23 @@ describe('siteTree', () => {
2424
test('object order', () => {
2525
expect(Object.keys(siteTree)[0]).toBe('en')
2626
expect(Object.keys(siteTree.en)[0]).toBe(nonEnterpriseDefaultVersion)
27-
expect(Object.keys(siteTree.en[nonEnterpriseDefaultVersion].products.github.categories)[0]).toBe(`/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
27+
expect(Object.keys(siteTree.en[nonEnterpriseDefaultVersion].products.github.categories)[0]).toBe(`/en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
2828
})
2929

3030
test('object structure', () => {
3131
expect(nonEnterpriseDefaultVersion in siteTree.en).toBe(true)
3232
expect(`enterprise-server@${latestEnterpriseRelease}` in siteTree.en).toBe(true)
33-
expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.href`]).toBe(`/${nonEnterpriseDefaultVersion}/github`)
34-
expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./${nonEnterpriseDefaultVersion}/github/getting-started-with-github.href`]).toBe(`/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
33+
expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.href`]).toBe(`/en/${nonEnterpriseDefaultVersion}/github`)
34+
expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github.href`]).toBe(`/en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
3535
})
3636

3737
describe('localized titles', () => {
3838
test('titles for categories', () => {
39-
const japaneseTitle = flatTree[`ja.${nonEnterpriseDefaultVersion}.products.github.categories./${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
39+
const japaneseTitle = flatTree[`ja.${nonEnterpriseDefaultVersion}.products.github.categories./ja/${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
4040
expect(typeof japaneseTitle).toBe('string')
4141
expect(japaneseCharacters.presentIn(japaneseTitle)).toBe(true)
4242

43-
const englishTitle = flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
43+
const englishTitle = flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
4444
expect(typeof englishTitle).toBe('string')
4545
expect(japaneseCharacters.presentIn(englishTitle)).toBe(false)
4646
})
@@ -52,7 +52,7 @@ describe('siteTree', () => {
5252
test('articles that include site data in liquid templating', () => {
5353
const pageWithDynamicTitle = siteTree.en[`enterprise-server@${latestEnterpriseRelease}`]
5454
.products.admin
55-
.categories[`/enterprise-server@${latestEnterpriseRelease}/admin/enterprise-support`]
55+
.categories[`/en/enterprise-server@${latestEnterpriseRelease}/admin/enterprise-support`]
5656
// Source frontmatter from article:
5757
// title: 'Working with {{ site.data.variables.contact.github_support }}'
5858
expect(pageWithDynamicTitle.title).toEqual('Working with GitHub Support')

0 commit comments

Comments
 (0)