tools: only fetch previous versions when necessary · nodejs/node@ffdd82b · GitHub
Skip to content

Commit ffdd82b

Browse files
richardlauMylesBorins
richardlau
authored and
MylesBorins
committed
tools: only fetch previous versions when necessary
Refactor the logic for working out the previous versions of Node.js for the API documentation so that the parsing (including the potential https get) happens at most once per build (as opposed to the current once per generated API doc). Signed-off-by: Richard Lau <riclau@uk.ibm.com> PR-URL: #32518 Fixes: #32512 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
1 parent 8bcbb8d commit ffdd82b

6 files changed

Lines changed: 132 additions & 88 deletions

File tree

‎Makefile‎

Lines changed: 9 additions & 2 deletions

‎test/doctool/test-doctool-html.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const testLinksMapper = {
3636
}
3737
};
3838

39-
async function toHTML({ input, filename, nodeVersion }) {
39+
function toHTML({ input, filename, nodeVersion, versions }) {
4040
const content = unified()
4141
.use(replaceLinks, { filename, linksMapper: testLinksMapper })
4242
.use(markdown)
@@ -49,7 +49,7 @@ async function toHTML({ input, filename, nodeVersion }) {
4949
.use(htmlStringify)
5050
.processSync(input);
5151

52-
return html.toHTML({ input, content, filename, nodeVersion });
52+
return html.toHTML({ input, content, filename, nodeVersion, versions });
5353
}
5454

5555
// Test data is a list of objects with two properties.
@@ -129,16 +129,27 @@ const testData = [
129129
];
130130

131131
const spaces = /\s/g;
132+
const versions = [
133+
{ num: '10.x', lts: true },
134+
{ num: '9.x' },
135+
{ num: '8.x' },
136+
{ num: '7.x' },
137+
{ num: '6.x' },
138+
{ num: '5.x' },
139+
{ num: '4.x' },
140+
{ num: '0.12.x' },
141+
{ num: '0.10.x' }];
132142

133143
testData.forEach(({ file, html }) => {
134144
// Normalize expected data by stripping whitespace.
135145
const expected = html.replace(spaces, '');
136146

137147
readFile(file, 'utf8', common.mustCall(async (err, input) => {
138148
assert.ifError(err);
139-
const output = await toHTML({ input: input,
140-
filename: 'foo',
141-
nodeVersion: process.version });
149+
const output = toHTML({ input: input,
150+
filename: 'foo',
151+
nodeVersion: process.version,
152+
versions: versions });
142153

143154
const actual = output.replace(spaces, '');
144155
// Assert that the input stripped of all whitespace contains the

‎test/doctool/test-doctool-versions.js‎

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@
22

33
require('../common');
44
const assert = require('assert');
5+
const { spawnSync } = require('child_process');
6+
const fs = require('fs');
7+
const path = require('path');
8+
const tmpdir = require('../common/tmpdir');
59
const util = require('util');
6-
const { versions } = require('../../tools/doc/versions.js');
10+
11+
const debuglog = util.debuglog('test');
12+
const versionsTool = path.join('../../tools/doc/versions.js');
713

814
// At the time of writing these are the minimum expected versions.
915
// New versions of Node.js do not have to be explicitly added here.
@@ -21,39 +27,48 @@ const expected = [
2127
'0.10.x',
2228
];
2329

24-
async function test() {
25-
const vers = await versions();
26-
// Coherence checks for each returned version.
27-
for (const version of vers) {
28-
const tested = util.inspect(version);
29-
const parts = version.num.split('.');
30-
const expectedLength = parts[0] === '0' ? 3 : 2;
31-
assert.strictEqual(parts.length, expectedLength,
32-
`'num' from ${tested} should be '<major>.x'.`);
33-
assert.strictEqual(parts[parts.length - 1], 'x',
34-
`'num' from ${tested} doesn't end in '.x'.`);
35-
const isEvenRelease = Number.parseInt(parts[expectedLength - 2]) % 2 === 0;
36-
const hasLtsProperty = version.hasOwnProperty('lts');
37-
if (hasLtsProperty) {
38-
// Odd-numbered versions of Node.js are never LTS.
39-
assert.ok(isEvenRelease, `${tested} should not be an 'lts' release.`);
40-
assert.ok(version.lts, `'lts' from ${tested} should 'true'.`);
41-
}
42-
}
30+
tmpdir.refresh();
31+
const versionsFile = path.join(tmpdir.path, 'versions.json');
32+
debuglog(versionsFile);
33+
const opts = { cwd: tmpdir.path, encoding: 'utf8' };
34+
const cp = spawnSync(process.execPath, [ versionsTool, versionsFile ], opts);
35+
debuglog(cp.stderr);
36+
debuglog(cp.stdout);
37+
assert.strictEqual(cp.stdout, '');
38+
assert.strictEqual(cp.signal, null);
39+
assert.strictEqual(cp.status, 0);
40+
const versions = JSON.parse(fs.readFileSync(versionsFile));
41+
debuglog(versions);
4342

44-
// Check that the minimum number of versions were returned.
45-
// Later versions are allowed, but not checked for here (they were checked
46-
// above).
47-
// Also check for the previous semver major -- From master this will be the
48-
// most recent major release.
49-
const thisMajor = Number.parseInt(process.versions.node.split('.')[0]);
50-
const prevMajorString = `${thisMajor - 1}.x`;
51-
if (!expected.includes(prevMajorString)) {
52-
expected.unshift(prevMajorString);
53-
}
54-
for (const version of expected) {
55-
assert.ok(vers.find((x) => x.num === version),
56-
`Did not find entry for '${version}' in ${util.inspect(vers)}`);
43+
// Coherence checks for each returned version.
44+
for (const version of versions) {
45+
const tested = util.inspect(version);
46+
const parts = version.num.split('.');
47+
const expectedLength = parts[0] === '0' ? 3 : 2;
48+
assert.strictEqual(parts.length, expectedLength,
49+
`'num' from ${tested} should be '<major>.x'.`);
50+
assert.strictEqual(parts[parts.length - 1], 'x',
51+
`'num' from ${tested} doesn't end in '.x'.`);
52+
const isEvenRelease = Number.parseInt(parts[expectedLength - 2]) % 2 === 0;
53+
const hasLtsProperty = version.hasOwnProperty('lts');
54+
if (hasLtsProperty) {
55+
// Odd-numbered versions of Node.js are never LTS.
56+
assert.ok(isEvenRelease, `${tested} should not be an 'lts' release.`);
57+
assert.ok(version.lts, `'lts' from ${tested} should 'true'.`);
5758
}
5859
}
59-
test();
60+
61+
// Check that the minimum number of versions were returned.
62+
// Later versions are allowed, but not checked for here (they were checked
63+
// above).
64+
// Also check for the previous semver major -- From master this will be the
65+
// most recent major release.
66+
const thisMajor = Number.parseInt(process.versions.node.split('.')[0]);
67+
const prevMajorString = `${thisMajor - 1}.x`;
68+
if (!expected.includes(prevMajorString)) {
69+
expected.unshift(prevMajorString);
70+
}
71+
for (const version of expected) {
72+
assert.ok(versions.find((x) => x.num === version),
73+
`Did not find entry for '${version}' in ${util.inspect(versions)}`);
74+
}

‎tools/doc/generate.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ let filename = null;
4242
let nodeVersion = null;
4343
let outputDir = null;
4444
let apilinks = {};
45+
let versions = {};
4546

4647
async function main() {
4748
for (const arg of args) {
@@ -58,6 +59,13 @@ async function main() {
5859
throw new Error(`${linkFile} is empty`);
5960
}
6061
apilinks = JSON.parse(data);
62+
} else if (arg.startsWith('--versions-file=')) {
63+
const versionsFile = arg.replace(/^--versions-file=/, '');
64+
const data = await fs.readFile(versionsFile, 'utf8');
65+
if (!data.trim()) {
66+
throw new Error(`${versionsFile} is empty`);
67+
}
68+
versions = JSON.parse(data);
6169
}
6270
}
6371

@@ -84,7 +92,8 @@ async function main() {
8492
.use(htmlStringify)
8593
.process(input);
8694

87-
const myHtml = await html.toHTML({ input, content, filename, nodeVersion });
95+
const myHtml = await html.toHTML({ input, content, filename, nodeVersion,
96+
versions });
8897
const basename = path.basename(filename, '.md');
8998
const htmlTarget = path.join(outputDir, `${basename}.html`);
9099
const jsonTarget = path.join(outputDir, `${basename}.json`);

‎tools/doc/html.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323

2424
const common = require('./common.js');
2525
const fs = require('fs');
26-
const getVersions = require('./versions.js');
2726
const unified = require('unified');
2827
const find = require('unist-util-find');
2928
const visit = require('unist-util-visit');
@@ -63,7 +62,7 @@ const gtocHTML = unified()
6362
const templatePath = path.join(docPath, 'template.html');
6463
const template = fs.readFileSync(templatePath, 'utf8');
6564

66-
async function toHTML({ input, content, filename, nodeVersion }) {
65+
function toHTML({ input, content, filename, nodeVersion, versions }) {
6766
filename = path.basename(filename, '.md');
6867

6968
const id = filename.replace(/\W+/g, '-');
@@ -81,7 +80,7 @@ async function toHTML({ input, content, filename, nodeVersion }) {
8180
const docCreated = input.match(
8281
/<!--\s*introduced_in\s*=\s*v([0-9]+)\.([0-9]+)\.[0-9]+\s*-->/);
8382
if (docCreated) {
84-
HTML = HTML.replace('__ALTDOCS__', await altDocs(filename, docCreated));
83+
HTML = HTML.replace('__ALTDOCS__', altDocs(filename, docCreated, versions));
8584
} else {
8685
console.error(`Failed to add alternative version links to ${filename}`);
8786
HTML = HTML.replace('__ALTDOCS__', '');
@@ -391,10 +390,9 @@ function getId(text, idCounters) {
391390
return text;
392391
}
393392

394-
async function altDocs(filename, docCreated) {
393+
function altDocs(filename, docCreated, versions) {
395394
const [, docCreatedMajor, docCreatedMinor] = docCreated.map(Number);
396395
const host = 'https://nodejs.org';
397-
const versions = await getVersions.versions();
398396

399397
const getHref = (versionNum) =>
400398
`${host}/docs/latest-v${versionNum}/api/${filename}.html`;

‎tools/doc/versions.js‎

Lines changed: 45 additions & 41 deletions

0 commit comments

Comments
 (0)