Skip to content

Commit bd63f47

Browse files
authored
Remove cheerio from Page#render (github#17566)
* Write our plugin * Include it * Move the RegEx * Don't rewriteLocalLinks with cheerio anymore * Process after HTML ast is generated * Use the same logic as before, just to see if it'll pass * Don't require languageCode/version * Only work on local links * Needs an href * Only update href if there's a new one to use * Check for node.properties * Some links are just mean * Move use-english-headings to be a plugin * Bail if no englishHeadings were passed * Install rehype-wrap * Wrap ol > li img in div.procedural-image-wrapper * Test for platform without cheerio * Use a plugin for rewriteAssetPathsToS3 * Remove cheerio from page.render * Fix require paths * SImplify * Fix some 🐛s * Use our own rehype-wrap * Move rewriteAssetPathsToS3 after HTML AST * Remove some console logs * Fix check for includesPlatformSpecificContent * Rename ast => tree
1 parent 1d348ee commit bd63f47

File tree

8 files changed

+186
-33
lines changed

8 files changed

+186
-33
lines changed

lib/page.js

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@ const path = require('path')
44
const cheerio = require('cheerio')
55
const patterns = require('./patterns')
66
const getMapTopicContent = require('./get-map-topic-content')
7-
const rewriteAssetPathsToS3 = require('./rewrite-asset-paths-to-s3')
87
const getApplicableVersions = require('./get-applicable-versions')
98
const encodeBracketedParentheses = require('./encode-bracketed-parentheses')
109
const generateRedirectsForPermalinks = require('./redirects/permalinks')
1110
const getEnglishHeadings = require('./get-english-headings')
12-
const useEnglishHeadings = require('./use-english-headings')
1311
const getTocItems = require('./get-toc-items')
1412
const pathUtils = require('./path-utils')
1513
const Permalink = require('./permalink')
@@ -154,6 +152,12 @@ class Page {
154152
this.markdown = await this.getMarkdown()
155153
}
156154

155+
// use English IDs/anchors for translated headings, so links don't break (see #8572)
156+
if (this.languageCode !== 'en') {
157+
const englishHeadings = getEnglishHeadings(this, context.pages)
158+
context.englishHeadings = englishHeadings
159+
}
160+
157161
this.intro = await renderContent(this.rawIntro, context)
158162
this.introPlainText = await renderContent(this.rawIntro, context, { textOnly: true })
159163
this.title = await renderContent(this.rawTitle, context, { textOnly: true, encodeEntities: true })
@@ -185,6 +189,7 @@ class Page {
185189
}))
186190
}
187191

192+
context.relativePath = this.relativePath
188193
const html = await renderContent(markdown, context)
189194

190195
// product frontmatter may contain liquid
@@ -227,29 +232,14 @@ class Page {
227232
})
228233
}
229234

230-
const $ = cheerio.load(html)
231-
232235
// set a flag so layout knows whether to render a mac/windows/linux switcher element
233-
this.includesPlatformSpecificContent = $('[class^="platform-"], .mac, .windows, .linux').length > 0
234-
235-
// rewrite asset paths to s3 if it's a dotcom article on any GHE version
236-
// or if it's an enterprise article on any GHE version EXCEPT latest version
237-
rewriteAssetPathsToS3($, context.currentVersion, this.relativePath)
238-
239-
// use English IDs/anchors for translated headings, so links don't break (see #8572)
240-
if (this.languageCode !== 'en') {
241-
const englishHeadings = getEnglishHeadings(this, context.pages)
242-
if (englishHeadings) useEnglishHeadings($, englishHeadings)
243-
}
244-
245-
// wrap ordered list images in a container div
246-
$('ol > li img').each((i, el) => {
247-
$(el).wrap('<div class="procedural-image-wrapper" />')
248-
})
249-
250-
const cleanedHTML = $('body').html()
236+
this.includesPlatformSpecificContent = (
237+
html.includes('extended-markdown mac') ||
238+
html.includes('extended-markdown windows') ||
239+
html.includes('extended-markdown linux')
240+
)
251241

252-
return cleanedHTML
242+
return html
253243
}
254244

255245
// Allow other modules (like custom liquid tags) to make one-off requests

lib/render-content/create-processor.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ const graphql = require('highlightjs-graphql').definer
1111
const remarkCodeExtra = require('remark-code-extra')
1212
const codeHeader = require('./plugins/code-header')
1313
const rewriteLocalLinks = require('./plugins/rewrite-local-links')
14+
const useEnglishHeadings = require('./plugins/use-english-headings')
15+
const rewriteAssetPathsToS3 = require('./plugins/rewrite-asset-paths-to-s3')
16+
const wrapInElement = require('./plugins/wrap-in-element')
1417

1518
module.exports = function createProcessor (context) {
1619
return unified()
@@ -19,9 +22,12 @@ module.exports = function createProcessor (context) {
1922
.use(emoji)
2023
.use(remark2rehype, { allowDangerousHTML: true })
2124
.use(slug)
25+
.use(useEnglishHeadings, context)
2226
.use(autolinkHeadings, { behavior: 'wrap' })
2327
.use(highlight, { languages: { graphql }, subset: false })
2428
.use(raw)
29+
.use(rewriteAssetPathsToS3, context)
30+
.use(wrapInElement, { selector: 'ol > li img', wrapper: 'div.procedural-image-wrapper' })
2531
.use(rewriteLocalLinks, { languageCode: context.currentLanguage, version: context.currentVersion })
2632
.use(html)
2733
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
const visit = require('unist-util-visit')
2+
const latestEnterpriseRelease = require('../../enterprise-server-releases').latest
3+
const nonEnterpriseDefaultVersion = require('../../non-enterprise-default-version')
4+
const { getS3BucketPathFromVersion } = require('../../s3-bucket-path-utils')
5+
const allVersions = require('../../all-versions')
6+
const s3BasePath = 'https://github-images.s3.amazonaws.com'
7+
8+
const matcher = node => (
9+
node.type === 'element' &&
10+
node.tagName === 'img' &&
11+
node.properties.src &&
12+
node.properties.src.startsWith('/assets/images')
13+
)
14+
15+
// This module rewrites asset paths on Enterprise versions to S3 paths.
16+
// Source example: /assets/images/foo.png
17+
// Rewritten: https://github-images.s3.amazonaws.com/enterprise/2.20/assets/images/foo.png
18+
// The one exception is Admin pages on the latest GHES release.
19+
module.exports = function rewriteAssetPathsToS3 ({ currentVersion, relativePath }) {
20+
// Bail if we don't have a relativePath in this context
21+
if (!relativePath) return
22+
23+
// skip if this is the homepage
24+
if (relativePath === 'index.md') return
25+
26+
// if the current version is non-enterprise, do not rewrite
27+
if (currentVersion === nonEnterpriseDefaultVersion) return
28+
29+
// the relativePath starts with the product, like /admin/foo or /github/foo
30+
const product = relativePath.split('/')[0]
31+
32+
// if this is an Admin page on the latest GHES release, do not rewrite
33+
if (product === 'admin' && allVersions[currentVersion].currentRelease === latestEnterpriseRelease) return
34+
35+
// if the version is [email protected], use `enterprise/2.22` as the bucket path
36+
// otherwise, use the plan name, e.g., `github-ae`
37+
const bucketPath = getS3BucketPathFromVersion(currentVersion)
38+
39+
return tree => {
40+
visit(tree, matcher, node => {
41+
// Rewrite the node's src
42+
node.properties.src = `${s3BasePath}/${bucketPath}${node.properties.src}`
43+
})
44+
}
45+
}

lib/render-content/plugins/rewrite-local-links.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ module.exports = function rewriteLocalLinks ({ languageCode, version }) {
2626
// There's no languageCode or version passed, so nothing to do
2727
if (!languageCode || !version) return
2828

29-
return ast => {
30-
visit(ast, matcher, node => {
29+
return tree => {
30+
visit(tree, matcher, node => {
3131
const newHref = getNewHref(node, languageCode, version)
3232
if (newHref) {
3333
node.properties.href = newHref
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
const GithubSlugger = require('github-slugger')
2+
const Entities = require('html-entities').XmlEntities
3+
const visit = require('unist-util-visit')
4+
const slugger = new GithubSlugger()
5+
const entities = new Entities()
6+
7+
const matcher = node => (
8+
node.type === 'element' &&
9+
['h2', 'h3', 'h4'].includes(node.tagName)
10+
)
11+
12+
// replace translated IDs and links in headings with English
13+
module.exports = function useEnglishHeadings ({ englishHeadings }) {
14+
if (!englishHeadings) return
15+
return tree => {
16+
visit(tree, matcher, node => {
17+
const text = node.children.find(n => n.type === 'text').value
18+
// find English heading in the collection
19+
const englishHeading = englishHeadings[entities.encode(text)]
20+
// get English slug
21+
const englishSlug = slugger.slug(englishHeading)
22+
// use English slug for heading ID and link
23+
node.properties.id = englishSlug
24+
})
25+
}
26+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
const visit = require('unist-util-visit')
2+
const { selectAll } = require('hast-util-select')
3+
const parseSelector = require('hast-util-parse-selector')
4+
5+
/*
6+
* Attacher
7+
*/
8+
module.exports = options => {
9+
options = options || {}
10+
const selector = options.selector || options.select || 'body'
11+
const wrapper = options.wrapper || options.wrap
12+
13+
/*
14+
* Transformer
15+
*/
16+
return tree => {
17+
if (typeof wrapper !== 'string') {
18+
throw new TypeError('Expected a `string` as wrapper')
19+
}
20+
21+
if (typeof selector !== 'string') {
22+
throw new TypeError('Expected a `string` as selector')
23+
}
24+
25+
for (const match of selectAll(options.selector, tree)) {
26+
visit(tree, match, (node, i, parent) => {
27+
const wrapper = parseSelector('div')
28+
wrapper.children = [node]
29+
parent.children[i] = wrapper
30+
})
31+
}
32+
}
33+
}

package-lock.json

Lines changed: 59 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
"got": "^9.6.0",
5050
"gray-matter": "^4.0.1",
5151
"hast-util-from-parse5": "^6.0.1",
52+
"hast-util-parse-selector": "^2.2.5",
53+
"hast-util-select": "^4.0.2",
5254
"hastscript": "^6.0.0",
5355
"helmet": "^3.21.2",
5456
"highlightjs-graphql": "^1.0.2",

0 commit comments

Comments
 (0)