doc: fix broken links in modules.md

[Trott]

Checklist

• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

Review requested:

• @nodejs/modules

[DerekNonGeneric]

This probably should have been caught by tools/doc/checkLinks.js. I haven't looked into it yet, but perhaps it's broken? There are literally pages of PRs fixing broken links (some of which are very recent). 😕

[lpinca]

This probably should have been caught by tools/doc/checkLinks.js

I think it skips api dirs.

[richardlau]

This probably should have been caught by tools/doc/checkLinks.js. I haven't looked into it yet, but perhaps it's broken? There are literally pages of PRs fixing broken links (some of which are very recent). 😕

I haven't dug into why but it looks like the api docs are deliberately excluded:

node/tools/doc/checkLinks.js

Line 29 in 9d12c14

entry.name !== 'api' &&

[aduh95]

This should be tested when building the HTML, but apparently it only tests links internal to all.html page:

node/tools/doc/allhtml.js

Lines 85 to 88 in 9d12c14

	const hrefRe = / href="#(\w+)"/g;
	while (match = hrefRe.exec(all)) {
	if (!ids.has(match[1])) throw new Error(`link not found: ${match[1]}`);
	}

Usually that would cover all links internal to the docs, as all links to doc pages are stripped from the filename to keep only the hash part:

node/tools/doc/allhtml.js

Lines 39 to 45 in 9d12c14

	apicontent += data.slice(match.index + match[0].length)
	.replace(/<!-- API END -->[\s\S]*/, '')
	.replace(/<a href="(\w[^#"]*)#/g, (match, href) => {
	return htmlFiles.includes(href) ? '<a href="#' : match;
	})
	.trim() + '\n';

But in this case, because modules_module.html doesn't exist, it's treated as an external page and the broken links slip through the test…

I haven't dug into why but it looks like the api docs are deliberately excluded

Yes indeed. The reason checkLinks.js does not cover api is because we are using .html extension to reference other doc pages, while checkLinks.js would expect .md. One way of fixing it would be to use .md extensions in the Markdown files, and use checkLinks.js to test said links – it would also improve the experience of navigating the docs through GitHub web UI. Another way would be to tweak the current test to make sure this doesn't reproduce.

[DerekNonGeneric]

Thanks for looking into this @aduh95; I've opened a separate issue @ #35189 so as not to derail.

Seeing as how others have already taken interest in tackling it themselves (see #35186), this should probably be fast-tracked.

Please 👍 this comment to approve fast-tracking

[Trott]

Landed in fb1ab19