Skip to content

Commit d577190

Browse files
richardlaucodebytere
richardlau
authored and
codebytere
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 <[email protected]> PR-URL: #32518Fixes: #32512 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Myles Borins <[email protected]>
1 parent 7123c0f commit d577190

File tree

6 files changed

+132
-88
lines changed

6 files changed

+132
-88
lines changed

‎Makefile‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,15 +738,22 @@ out/doc/api/assets/%: doc/api_assets/% out/doc/api/assets
738738
run-npm-ci = $(PWD)/$(NPM) ci
739739

740740
LINK_DATA = out/doc/apilinks.json
741+
VERSIONS_DATA = out/doc/previous-versions.json
741742
gen-api = tools/doc/generate.js --node-version=$(FULLVERSION)\
742-
--apilinks=$(LINK_DATA)$< --output-directory=out/doc/api
743+
--apilinks=$(LINK_DATA)$< --output-directory=out/doc/api \
744+
--versions-file=$(VERSIONS_DATA)
743745
gen-apilink = tools/doc/apilinks.js $(LINK_DATA)$(wildcard lib/*.js)
744746

745747
$(LINK_DATA): $(wildcard lib/*.js) tools/doc/apilinks.js
746748
$(call available-node, $(gen-apilink))
747749

750+
# Regenerate previous versions data if the current version changes
751+
$(VERSIONS_DATA): CHANGELOG.md src/node_version.h tools/doc/versions.js
752+
$(call available-node, tools/doc/versions.js $@)
753+
748754
out/doc/api/%.jsonout/doc/api/%.html: doc/api/%.md tools/doc/generate.js \
749-
tools/doc/markdown.js tools/doc/html.js tools/doc/json.js tools/doc/apilinks.js |$(LINK_DATA)
755+
tools/doc/markdown.js tools/doc/html.js tools/doc/json.js \
756+
tools/doc/apilinks.js $(VERSIONS_DATA)|$(LINK_DATA)
750757
$(call available-node, $(gen-api))
751758

752759
out/doc/api/all.html: $(apidocs_html) tools/doc/allhtml.js \

‎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-
asyncfunctiontoHTML({ input, filename, nodeVersion }){
39+
functiontoHTML({ input, filename, nodeVersion, versions}){
4040
constcontent=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-
returnhtml.toHTML({ input, content, filename, nodeVersion });
52+
returnhtml.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
constspaces=/\s/g;
132+
constversions=[
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
constexpected=html.replace(spaces,'');
136146

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

143154
constactual=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
constassert=require('assert');
5+
const{ spawnSync }=require('child_process');
6+
constfs=require('fs');
7+
constpath=require('path');
8+
consttmpdir=require('../common/tmpdir');
59
constutil=require('util');
6-
const{ versions }=require('../../tools/doc/versions.js');
10+
11+
constdebuglog=util.debuglog('test');
12+
constversionsTool=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-
asyncfunctiontest(){
25-
constvers=awaitversions();
26-
// Coherence checks for each returned version.
27-
for(constversionofvers){
28-
consttested=util.inspect(version);
29-
constparts=version.num.split('.');
30-
constexpectedLength=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-
constisEvenRelease=Number.parseInt(parts[expectedLength-2])%2===0;
36-
consthasLtsProperty=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+
constversionsFile=path.join(tmpdir.path,'versions.json');
32+
debuglog(versionsFile);
33+
constopts={cwd: tmpdir.path,encoding: 'utf8'};
34+
constcp=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+
constversions=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-
constthisMajor=Number.parseInt(process.versions.node.split('.')[0]);
50-
constprevMajorString=`${thisMajor-1}.x`;
51-
if(!expected.includes(prevMajorString)){
52-
expected.unshift(prevMajorString);
53-
}
54-
for(constversionofexpected){
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(constversionofversions){
45+
consttested=util.inspect(version);
46+
constparts=version.num.split('.');
47+
constexpectedLength=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+
constisEvenRelease=Number.parseInt(parts[expectedLength-2])%2===0;
53+
consthasLtsProperty=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+
constthisMajor=Number.parseInt(process.versions.node.split('.')[0]);
67+
constprevMajorString=`${thisMajor-1}.x`;
68+
if(!expected.includes(prevMajorString)){
69+
expected.unshift(prevMajorString);
70+
}
71+
for(constversionofexpected){
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
letnodeVersion=null;
4343
letoutputDir=null;
4444
letapilinks={};
45+
letversions={};
4546

4647
asyncfunctionmain(){
4748
for(constargofargs){
@@ -58,6 +59,13 @@ async function main(){
5859
thrownewError(`${linkFile} is empty`);
5960
}
6061
apilinks=JSON.parse(data);
62+
}elseif(arg.startsWith('--versions-file=')){
63+
constversionsFile=arg.replace(/^--versions-file=/,'');
64+
constdata=awaitfs.readFile(versionsFile,'utf8');
65+
if(!data.trim()){
66+
thrownewError(`${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-
constmyHtml=awaithtml.toHTML({ input, content, filename, nodeVersion });
95+
constmyHtml=awaithtml.toHTML({ input, content, filename, nodeVersion,
96+
versions });
8897
constbasename=path.basename(filename,'.md');
8998
consthtmlTarget=path.join(outputDir,`${basename}.html`);
9099
constjsonTarget=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
constcommon=require('./common.js');
2525
constfs=require('fs');
26-
constgetVersions=require('./versions.js');
2726
constunified=require('unified');
2827
constfind=require('unist-util-find');
2928
constvisit=require('unist-util-visit');
@@ -63,7 +62,7 @@ const gtocHTML = unified()
6362
consttemplatePath=path.join(docPath,'template.html');
6463
consttemplate=fs.readFileSync(templatePath,'utf8');
6564

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

6968
constid=filename.replace(/\W+/g,'-');
@@ -81,7 +80,7 @@ async function toHTML({input, content, filename, nodeVersion }){
8180
constdocCreated=input.match(
8281
/<!--\s*introduced_in\s*=\s*v([0-9]+)\.([0-9]+)\.[0-9]+\s*-->/);
8382
if(docCreated){
84-
HTML=HTML.replace('__ALTDOCS__',awaitaltDocs(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
returntext;
392391
}
393392

394-
asyncfunctionaltDocs(filename,docCreated){
393+
functionaltDocs(filename,docCreated,versions){
395394
const[,docCreatedMajor,docCreatedMinor]=docCreated.map(Number);
396395
consthost='https://nodejs.org';
397-
constversions=awaitgetVersions.versions();
398396

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

‎tools/doc/versions.js‎

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
'use strict';
22

3-
const{ readFileSync }=require('fs');
3+
const{ readFileSync, writeFileSync}=require('fs');
44
constpath=require('path');
55
constsrcRoot=path.join(__dirname,'..','..');
66

7-
let_versions;
8-
97
constisRelease=()=>{
108
constre=/#defineNODE_VERSION_IS_RELEASE0/;
119
constfile=path.join(srcRoot,'src','node_version.h');
@@ -15,7 +13,7 @@ const isRelease = () =>{
1513
constgetUrl=(url)=>{
1614
returnnewPromise((resolve,reject)=>{
1715
consthttps=require('https');
18-
constrequest=https.get(url,{timeout: 5000},(response)=>{
16+
constrequest=https.get(url,{timeout: 30000},(response)=>{
1917
if(response.statusCode!==200){
2018
reject(newError(
2119
`Failed to get ${url}, status code ${response.statusCode}`));
@@ -32,45 +30,51 @@ const getUrl = (url) =>{
3230
};
3331

3432
constkNoInternet=!!process.env.NODE_TEST_NO_INTERNET;
33+
constoutFile=(process.argv.length>2 ? process.argv[2] : undefined);
3534

36-
module.exports={
37-
asyncversions(){
38-
if(_versions){
39-
return_versions;
40-
}
41-
42-
// The CHANGELOG.md on release branches may not reference newer semver
43-
// majors of Node.js so fetch and parse the version from the master branch.
44-
consturl=
45-
'https://raw.githubusercontent.com/nodejs/node/master/CHANGELOG.md';
46-
letchangelog;
47-
constfile=path.join(srcRoot,'CHANGELOG.md');
48-
if(kNoInternet){
49-
changelog=readFileSync(file,{encoding: 'utf8'});
50-
}else{
51-
try{
52-
changelog=awaitgetUrl(url);
53-
}catch(e){
54-
// Fail if this is a release build, otherwise fallback to local files.
55-
if(isRelease()){
56-
throwe;
57-
}else{
58-
console.warn(`Unable to retrieve ${url}. Falling back to ${file}.`);
59-
changelog=readFileSync(file,{encoding: 'utf8'});
60-
}
35+
asyncfunctionversions(){
36+
// The CHANGELOG.md on release branches may not reference newer semver
37+
// majors of Node.js so fetch and parse the version from the master branch.
38+
consturl=
39+
'https://raw.githubusercontent.com/nodejs/node/master/CHANGELOG.md';
40+
letchangelog;
41+
constfile=path.join(srcRoot,'CHANGELOG.md');
42+
if(kNoInternet){
43+
changelog=readFileSync(file,{encoding: 'utf8'});
44+
}else{
45+
try{
46+
changelog=awaitgetUrl(url);
47+
}catch(e){
48+
// Fail if this is a release build, otherwise fallback to local files.
49+
if(isRelease()){
50+
throwe;
51+
}else{
52+
console.warn(`Unable to retrieve ${url}. Falling back to ${file}.`);
53+
changelog=readFileSync(file,{encoding: 'utf8'});
6154
}
6255
}
63-
constltsRE=/LongTermSupport/i;
64-
constversionRE=/\*\[Node\.js([0-9.]+)\]\S+(.*)\r?\n/g;
65-
_versions=[];
66-
letmatch;
67-
while((match=versionRE.exec(changelog))!=null){
68-
constentry={num: `${match[1]}.x`};
69-
if(ltsRE.test(match[2])){
70-
entry.lts=true;
71-
}
72-
_versions.push(entry);
56+
}
57+
constltsRE=/LongTermSupport/i;
58+
constversionRE=/\*\[Node\.js([0-9.]+)\]\S+(.*)\r?\n/g;
59+
const_versions=[];
60+
letmatch;
61+
while((match=versionRE.exec(changelog))!=null){
62+
constentry={num: `${match[1]}.x`};
63+
if(ltsRE.test(match[2])){
64+
entry.lts=true;
7365
}
74-
return_versions;
66+
_versions.push(entry);
7567
}
76-
};
68+
return_versions;
69+
}
70+
71+
versions().then((v)=>{
72+
if(outFile){
73+
writeFileSync(outFile,JSON.stringify(v));
74+
}else{
75+
console.log(JSON.stringify(v));
76+
}
77+
}).catch((err)=>{
78+
console.error(err);
79+
process.exit(1);
80+
});

0 commit comments

Comments
(0)