feat: restore legacy anchor alias to preserve old links#699
feat: restore legacy anchor alias to preserve old links#699shivxmsharma wants to merge 1 commit intonodejs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #699 +/- ##
==========================================
+ Coverage 75.44% 75.51% +0.07%
==========================================
Files 148 150 +2
Lines 13572 13690 +118
Branches 1023 1035 +12
==========================================
+ Hits 10239 10338 +99
- Misses 3328 3347 +19
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
avivkeller
left a comment
avivkeller
left a comment
There was a problem hiding this comment.
Any and all legacy slug generation should go in the legacy generator, not the metadata generator
| // Legacy anchor alias to preserve old external links | ||
| createElement('a', { id: legacySlug }), |
There was a problem hiding this comment.
We don't need this in the new generator
shivxmsharma
force-pushed
the
feat/restore-legacy-anchor-alias
branch
from
1eec08d to
9f06c83
Compare
|
@avivkeller I've updated the PR to remove the legacy logic from the metadata/jsx-ast generators completely. All the legacy slug generation and its tests are now strictly isolated within the legacy-html generator as requested. Let me know if there's anything else that needs adjusting! |
| // Legacy slug counter tracking for deduplication | ||
| const legacyIdCounters = { __proto__: null }; | ||
| /** Deduplicates legacy slugs by appending an incremented counter */ | ||
| const legacyDeduplicateFn = id => { | ||
| if (legacyIdCounters[id] !== undefined) { | ||
| return `${id}_${++legacyIdCounters[id]}`; | ||
| } | ||
| legacyIdCounters[id] = 0; | ||
| return id; | ||
| }; |
There was a problem hiding this comment.
| // Legacy slug counter tracking for deduplication | |
| const legacyIdCounters = { __proto__: null }; | |
| /** Deduplicates legacy slugs by appending an incremented counter */ | |
| const legacyDeduplicateFn = id => { | |
| if (legacyIdCounters[id] !== undefined) { | |
| return `${id}_${++legacyIdCounters[id]}`; | |
| } | |
| legacyIdCounters[id] = 0; | |
| return id; | |
| }; | |
| /** Deduplicates legacy slugs by appending an incremented counter */ | |
| const legacyDeduplicator = (counters = {}) => id => { | |
| counters[id] ??= 0; | |
| return `${id}_${counters[id]++}`; | |
| }; |
and move this into slugger
There was a problem hiding this comment.
I've refactored this and moved the deduplicator logic entirely into createLegacySlugger() inside slugger.mjs, so that it tracks its own state automatically. Let me know what you think!
shivxmsharma
force-pushed
the
feat/restore-legacy-anchor-alias
branch
from
9f06c83 to
592ee66
Compare
| const deduplicate = id => { | ||
| if (legacyIdCounters[id] !== undefined) { | ||
| return `${id}_${++legacyIdCounters[id]}`; | ||
| } | ||
| legacyIdCounters[id] = 0; | ||
| return id; | ||
| }; |
There was a problem hiding this comment.
Can you use the implementation I showed you for this inner function?
There was a problem hiding this comment.
Implemented your closure approach! I just made one tiny tweak: returning just id (instead of id_0) on the first pass so that the legacy backward compatibility remains 100% perfectly intact. It's updated and pushed up!
shivxmsharma
force-pushed
the
feat/restore-legacy-anchor-alias
branch
from
592ee66 to
aeeb9d2
Compare
| // Parses the Heading nodes into Heading elements | ||
| visit(content, UNIST.isHeading, buildHeading); | ||
| visit(content, UNIST.isHeading, (node, index, parent) => | ||
| buildHeading(node, index, parent, entry.api, legacySlugger) |
There was a problem hiding this comment.
Can we create the slug here and pass it to the function, rather than passing the slug creator only to be used?
There was a problem hiding this comment.
I've updated it to generate the slug inside the visit loop and pass the string directly into buildHeading. Pushed the update!"
shivxmsharma
force-pushed
the
feat/restore-legacy-anchor-alias
branch
from
aeeb9d2 to
8e5017f
Compare
2 participants
Fixes: #697
Description
Restores the legacy anchor alias for headings to preserve old documentation links.
The old Node.js doc generator (
tools/doc/html.mjs) produced two anchor IDs per heading — a modern GitHub-style slug (e.g.,#file-system) and a legacy underscore-based slug prefixed with the filename (e.g.,#fs_file_system). External sites like Express and many others still link to these legacy anchors, and since doc-kit dropped them, those links silently break.This PR ports the old
getLegacyIdalgorithm into doc-kit and renders a hidden<a>element with the legacy ID alongside every heading, so both old and new anchor links resolve correctly.Changes
src/generators/metadata/utils/slugger.mjs— AddedgetLegacySlug()function implementing the old underscore-based slug algorithm, and added alegacySlug()method tocreateNodeSluggerwith counter-based deduplication.src/generators/metadata/types.d.ts— AddedlegacySlug: stringfield toHeadingDatainterface.src/generators/metadata/utils/parse.mjs— GeneratelegacySlugfor every heading during metadata parsing.src/generators/legacy-html/utils/buildContent.mjs— Render a hidden legacy anchor<a>element in the legacy HTML output.src/generators/jsx-ast/utils/buildContent.mjs— Render a hidden legacy anchor<a>element in the JSX/AST (web) output.src/generators/metadata/utils/__tests__/slugger.test.mjs— Added tests forgetLegacySlugandcreateNodeSlugger.legacySlug.Validation
npm test).npm run lint,npm run format:check).fs.mdfile confirms both anchors are present:<a id=fs_file_system></a>(legacy) alongside<a class=mark id=file-system>(modern)<a id=fs_fs_readfile_path></a>(legacy) alongside<a class=mark id=fsreadfilepath>(modern)Related Issues
Fixes #697
Check List
node --run testand all tests passed.node --run format&node --run lint.