Skip to content

Commit 44ce96b

Browse files
committed
code review
1 parent faaf875 commit 44ce96b

7 files changed

Lines changed: 310 additions & 79 deletions

File tree

.github/workflows/sync-orama.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ on:
1717
- main
1818
types:
1919
- labeled
20+
pull_request:
2021

2122
permissions:
2223
contents: read

apps/site/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
"@testing-library/user-event": "~14.6.1",
8989
"@types/mdx": "^2.0.13",
9090
"@types/semver": "~7.7.0",
91+
"dedent": "^1.6.0",
9192
"eslint-config-next": "15.5.0",
9293
"eslint-import-resolver-typescript": "~4.4.4",
9394
"eslint-plugin-mdx": "~3.6.2",
@@ -96,6 +97,9 @@
9697
"global-jsdom": "^26.0.0",
9798
"handlebars": "4.7.8",
9899
"jsdom": "^26.0.0",
100+
"mdast-util-from-markdown": "^2.0.2",
101+
"mdast-util-to-string": "^4.0.0",
102+
"nock": "^14.0.10",
99103
"remark-frontmatter": "^5.0.0",
100104
"stylelint": "16.23.0",
101105
"stylelint-config-standard": "39.0.0",
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import assert from 'node:assert/strict';
2+
import { test, mock } from 'node:test';
3+
4+
import nock from 'nock';
5+
6+
mock.module('node:fs/promises', {
7+
namedExports: {
8+
glob: () => ['filename'],
9+
readFile: name => name.endsWith('filename') && 'content',
10+
},
11+
});
12+
13+
const { getAPIDocs, getArticles } = await import('../get-documents.mjs');
14+
15+
test('getAPIDocs', async () => {
16+
nock('https://api.github.com')
17+
.get('/repos/nodejs/node/contents/doc/api')
18+
.query(true)
19+
.reply(200, [
20+
{
21+
name: 'fs.md',
22+
download_url: 'data:text/plain,fs',
23+
},
24+
]);
25+
26+
const result = await getAPIDocs();
27+
28+
assert.equal(result.length, 1);
29+
assert.equal(result[0].content, 'fs');
30+
assert.match(result[0].pathname, /^docs\/v[^/]+\/api\/fs\.html$/);
31+
});
32+
33+
test('getArticles', async () => {
34+
const result = await getArticles();
35+
assert.deepStrictEqual(result, [
36+
{ content: 'content', pathname: 'filename' },
37+
]);
38+
});
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import assert from 'node:assert';
2+
import test from 'node:test';
3+
4+
import dedent from 'dedent';
5+
6+
import { processDocument } from '../process-documents.mjs';
7+
8+
const testCases = [
9+
{
10+
name: 'Uses front matter title if available',
11+
input: {
12+
pathname: 'blog/my-post.html',
13+
content: dedent`
14+
---
15+
title: Custom Title
16+
---
17+
# Intro
18+
Hello world
19+
`,
20+
},
21+
expected: [
22+
{
23+
path: 'blog/my-post.html#intro',
24+
siteSection: 'Blog',
25+
pageTitle: 'Custom Title',
26+
pageSectionTitle: 'Intro',
27+
pageSectionContent: 'Hello world',
28+
},
29+
],
30+
},
31+
{
32+
name: 'Falls back to filename for title',
33+
input: {
34+
pathname: 'docs/another-post.html',
35+
content: dedent`
36+
# Start
37+
Content here
38+
`,
39+
},
40+
expected: [
41+
{
42+
path: 'docs/another-post.html#start',
43+
siteSection: 'Docs',
44+
pageTitle: 'another post',
45+
pageSectionTitle: 'Start',
46+
pageSectionContent: 'Content here',
47+
},
48+
],
49+
},
50+
{
51+
name: 'Handles multiple sections',
52+
input: {
53+
pathname: 'guides/test.html',
54+
content: dedent`
55+
# First
56+
Paragraph A
57+
58+
# Second
59+
Paragraph B
60+
`,
61+
},
62+
expected: [
63+
{
64+
path: 'guides/test.html#first',
65+
siteSection: 'Guides',
66+
pageTitle: 'test',
67+
pageSectionTitle: 'First',
68+
pageSectionContent: 'Paragraph A',
69+
},
70+
{
71+
path: 'guides/test.html#second',
72+
siteSection: 'Guides',
73+
pageTitle: 'test',
74+
pageSectionTitle: 'Second',
75+
pageSectionContent: 'Paragraph B',
76+
},
77+
],
78+
},
79+
{
80+
name: 'Section with no heading',
81+
input: {
82+
pathname: 'misc/untitled.html',
83+
content: dedent`
84+
Just some text without a heading
85+
`,
86+
},
87+
expected: [
88+
{
89+
path: 'misc/untitled.html#',
90+
siteSection: 'Misc',
91+
pageTitle: 'untitled',
92+
pageSectionTitle: '',
93+
pageSectionContent: 'Just some text without a heading',
94+
},
95+
],
96+
},
97+
];
98+
99+
for (const { name, input, expected } of testCases) {
100+
test(name, () => {
101+
const result = processDocument(input);
102+
assert.deepStrictEqual(result, expected);
103+
});
104+
}
Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { readFile, glob } from 'node:fs/promises';
2-
import { join, basename } from 'node:path';
2+
import { join, basename, posix, win32 } from 'node:path';
33

44
import generateReleaseData from '#site/next-data/generators/releaseData.mjs';
55
import { getRelativePath } from '#site/next.helpers.mjs';
@@ -15,16 +15,16 @@ const fetchOptions = process.env.GITHUB_TOKEN
1515
* Fetch Node.js API documentation directly from GitHub
1616
* for the current Active LTS version.
1717
*/
18-
const getAPIDocs = async () => {
18+
export const getAPIDocs = async () => {
1919
// Find the current Active LTS version
2020
const releaseData = await generateReleaseData();
21-
const version = releaseData.find(
21+
const { versionWithPrefix } = releaseData.find(
2222
r => r.status === 'Active LTS'
23-
).versionWithPrefix;
23+
);
2424

2525
// Get list of API docs from the Node.js repo
2626
const fetchResponse = await fetch(
27-
`https://api.github.com/repos/nodejs/node/contents/doc/api?ref=${version}`,
27+
`https://api.github.com/repos/nodejs/node/contents/doc/api?ref=${versionWithPrefix}`,
2828
fetchOptions
2929
);
3030
const documents = await fetchResponse.json();
@@ -34,10 +34,10 @@ const getAPIDocs = async () => {
3434
documents.map(async ({ name, download_url }) => {
3535
const res = await fetch(download_url, fetchOptions);
3636

37-
return processDocument({
37+
return {
3838
content: await res.text(),
39-
pathname: `docs/${version}/api/${basename(name, '.md')}.html`,
40-
});
39+
pathname: `docs/${versionWithPrefix}/api/${basename(name, '.md')}.html`,
40+
};
4141
})
4242
);
4343
};
@@ -46,7 +46,7 @@ const getAPIDocs = async () => {
4646
* Collect all local markdown/mdx articles under /pages/en,
4747
* excluding blog content.
4848
*/
49-
const getArticles = async () => {
49+
export const getArticles = async () => {
5050
const relativePath = getRelativePath(import.meta.url);
5151
const root = join(relativePath, '..', '..', 'pages', 'en');
5252

@@ -57,15 +57,14 @@ const getArticles = async () => {
5757
return Promise.all(
5858
files
5959
.filter(path => !path.startsWith('blog'))
60-
.map(async path =>
61-
processDocument(
62-
{
63-
content: await readFile(join(root, path), 'utf8'),
64-
pathname: path.replace(/\.mdx?$/, ''),
65-
},
66-
true
67-
)
68-
)
60+
.map(async path => ({
61+
content: await readFile(join(root, path), 'utf8'),
62+
pathname: path
63+
// Strip the extension
64+
.replace(/\.mdx?$/, '')
65+
// Normalize to a POSIX path
66+
.replaceAll(win32.sep, posix.sep),
67+
}))
6968
);
7069
};
7170

@@ -74,5 +73,7 @@ const getArticles = async () => {
7473
*/
7574
export const getDocuments = async () => {
7675
const documentPromises = await Promise.all([getAPIDocs(), getArticles()]);
77-
return documentPromises.flatMap(documents => documents.flat());
76+
return documentPromises.flatMap(documents =>
77+
documents.flatMap(processDocument)
78+
);
7879
};

0 commit comments

Comments
 (0)