[DX-538] Add navigation footer to generated markdown pages#3208
[DX-538] Add navigation footer to generated markdown pages#3208
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR exposes navigation utility functions and enhances MDX-to-Markdown transpilation to generate navigation footers with previous/next links, related topics, and documentation index sections based on site navigation structure. New functions flatten navigation, build page sets, map metadata, and construct per-page navigation contexts for footer generation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sacOO7
changed the title
sacOO7
force-pushed
the
feature/markdown-navigation-footer
branch
from
6d9d0f7 to
bff2e82
Compare
sacOO7
force-pushed
the
feature/markdown-navigation-footer
branch
from
bff2e82 to
428dc99
Compare
sacOO7
force-pushed
the
feature/markdown-navigation-footer
branch
from
428dc99 to
5200170
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
data/onPostBuild/transpileMdxToMarkdown.test.ts (1)
957-999: Consider the brittleness of hardcoded nav data assertions.Tests like
expect(products.size).toBe(7)(line 966) and specific path/name assertions (e.g., line 988, 1018) are tightly coupled to the current navigation data. Any nav restructuring will break these tests. This is a reasonable trade-off for integration-level confidence, but worth keeping in mind for maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/onPostBuild/transpileMdxToMarkdown.test.ts` around lines 957 - 999, The tests for flattenNavPages are brittle due to hardcoded counts and exact-path assertions; update the specs to assert that expected products are included (use subset checks like expect(products).toEqual(expect.arrayContaining([...])) or check products.size toBeGreaterThanOrEqual(expectedCount) instead of exact 7), avoid asserting exact indices by asserting relative order (e.g., use indexOf and expect(gsOverviewIdx).toBeLessThan(gsJsIdx) already done — extend this pattern for other order checks) and replace exact-path existence checks with presence checks (arrayContaining) so nav restructures won't break tests; focus changes in the test suite surrounding flattenNavPages and the specific it blocks that assert products.size and concrete link values.data/onPostBuild/transpileMdxToMarkdown.ts (1)
831-836: Consider typingreporterinstead ofany.The
reporterparameter usesany, which the coding guidelines discourage. ImportReporterfrom the 'gatsby' package and use it as the type—other parts of the codebase follow this pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/onPostBuild/transpileMdxToMarkdown.ts` around lines 831 - 836, The reporter parameter in processFile is currently typed as any; import Reporter from 'gatsby' and change the parameter type to Reporter to match project conventions—update the function signature async function processFile(node: MdxNode, siteUrl: string, reporter: Reporter, navLookup: Map<string, NavContext>): Promise<void> and add the import { Reporter } from 'gatsby' at the top of the file so other usages of reporter inside processFile are correctly typed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/onPostBuild/transpileMdxToMarkdown.test.ts`:
- Around line 957-1047: The test block for describe('flattenNavPages') has
incorrect extra indentation causing many Prettier errors; open the tests around
the flattenNavPages describe/its (the it(...) blocks and their contents) and
normalize indentation to match the rest of the file (use 2 or 4 spaces per
nesting level consistent with existing style), then run the formatter (npx
prettier --write) to auto-fix remaining spacing; ensure the it blocks and nested
expectations around flattenNavPages, chatPages, chatLinks, messagesPage,
historyPage, setupPage, hivePage, introPage retain their original logic but with
corrected indentation.
In `@data/onPostBuild/transpileMdxToMarkdown.ts`:
- Around line 558-560: In the else-if branch checking ('link' in item) (the
"Leaf page" branch) the single-line if uses continue without braces; update the
conditional to use a block with curly braces around the continue (i.e., change
if (item.external) continue; to if (item.external) { continue; }) so ESLint's
brace rule is satisfied and the behavior remains the same within the ('link' in
item) handling code.
- Around line 715-718: ESLint flags the single-line if statements that add
prev/next URLs to navUrls; update the block around navUrls so both conditional
branches use curly braces (i.e., wrap the bodies of the if (navContext.prev) and
if (navContext.next) statements) to satisfy the `curly` rule while preserving
the existing behavior of adding navContext.prev.url and navContext.next.url to
the Set.
- Around line 670-674: The .map callback building NavLink objects in
transpileMdxToMarkdown.ts is failing Prettier/ESLint formatting; run the
project's Prettier formatter (or apply the project's formatting rules) to
reformat the .map(...) block so it matches the configured style. Locate the .map
that returns NavLink (references: NavLink type, linkToMdUrl(), metaDescriptions)
and re-run the formatter (or fix spacing/line breaks around the arrow function
and object literal) so lint/Prettier no longer flags lines 670–674.
---
Nitpick comments:
In `@data/onPostBuild/transpileMdxToMarkdown.test.ts`:
- Around line 957-999: The tests for flattenNavPages are brittle due to
hardcoded counts and exact-path assertions; update the specs to assert that
expected products are included (use subset checks like
expect(products).toEqual(expect.arrayContaining([...])) or check products.size
toBeGreaterThanOrEqual(expectedCount) instead of exact 7), avoid asserting exact
indices by asserting relative order (e.g., use indexOf and
expect(gsOverviewIdx).toBeLessThan(gsJsIdx) already done — extend this pattern
for other order checks) and replace exact-path existence checks with presence
checks (arrayContaining) so nav restructures won't break tests; focus changes in
the test suite surrounding flattenNavPages and the specific it blocks that
assert products.size and concrete link values.
In `@data/onPostBuild/transpileMdxToMarkdown.ts`:
- Around line 831-836: The reporter parameter in processFile is currently typed
as any; import Reporter from 'gatsby' and change the parameter type to Reporter
to match project conventions—update the function signature async function
processFile(node: MdxNode, siteUrl: string, reporter: Reporter, navLookup:
Map<string, NavContext>): Promise<void> and add the import { Reporter } from
'gatsby' at the top of the file so other usages of reporter inside processFile
are correctly typed.
| describe('flattenNavPages', () => { | ||
| it('should return a non-empty array of pages from all products', () => { | ||
| const pages = flattenNavPages(); | ||
| expect(pages.length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it('should include pages from all 7 products', () => { | ||
| const pages = flattenNavPages(); | ||
| const products = new Set(pages.map((p) => p.product)); | ||
| expect(products.size).toBe(7); | ||
| expect(products).toContain('Platform'); | ||
| expect(products).toContain('Ably Pub/Sub'); | ||
| expect(products).toContain('Ably Chat'); | ||
| expect(products).toContain('Ably AI Transport'); | ||
| expect(products).toContain('Ably Spaces'); | ||
| expect(products).toContain('Ably LiveObjects'); | ||
| expect(products).toContain('Ably LiveSync'); | ||
| }); | ||
|
|
||
| it('should skip external links', () => { | ||
| const pages = flattenNavPages(); | ||
| const externalPages = pages.filter((p) => p.link.startsWith('http')); | ||
| expect(externalPages).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should preserve depth-first order within a product', () => { | ||
| const pages = flattenNavPages(); | ||
| const chatPages = pages.filter((p) => p.product === 'Ably Chat'); | ||
| const chatLinks = chatPages.map((p) => p.link); | ||
|
|
||
| // About Chat should come first | ||
| expect(chatLinks[0]).toBe('/docs/chat'); | ||
|
|
||
| // Getting started overview should come before individual getting started pages | ||
| const gsOverviewIdx = chatLinks.indexOf('/docs/chat/getting-started'); | ||
| const gsJsIdx = chatLinks.indexOf('/docs/chat/getting-started/javascript'); | ||
| expect(gsOverviewIdx).toBeLessThan(gsJsIdx); | ||
|
|
||
| // Messages should come before History (both in Chat features) | ||
| const messagesIdx = chatLinks.indexOf('/docs/chat/rooms/messages'); | ||
| const historyIdx = chatLinks.indexOf('/docs/chat/rooms/history'); | ||
| expect(messagesIdx).toBeLessThan(historyIdx); | ||
| }); | ||
|
|
||
| it('should include both content and API pages', () => { | ||
| const pages = flattenNavPages(); | ||
| const chatPages = pages.filter((p) => p.product === 'Ably Chat'); | ||
| const chatLinks = chatPages.map((p) => p.link); | ||
|
|
||
| // Content page | ||
| expect(chatLinks).toContain('/docs/chat/rooms/messages'); | ||
| // API page (non-external) | ||
| expect(chatLinks).toContain('/docs/chat/api'); | ||
| }); | ||
|
|
||
| it('should track section keys correctly for sibling grouping', () => { | ||
| const pages = flattenNavPages(); | ||
| const messagesPage = pages.find((p) => p.link === '/docs/chat/rooms/messages'); | ||
| const historyPage = pages.find((p) => p.link === '/docs/chat/rooms/history'); | ||
| const setupPage = pages.find((p) => p.link === '/docs/chat/setup'); | ||
|
|
||
| expect(messagesPage?.sectionKey).toBe('Ably Chat::Chat features'); | ||
| expect(historyPage?.sectionKey).toBe('Ably Chat::Chat features'); | ||
| // Setup is in Concepts section, different from Chat features | ||
| expect(setupPage?.sectionKey).toBe('Ably Chat::Concepts'); | ||
| }); | ||
|
|
||
| it('should use __root__ section key for top-level pages', () => { | ||
| const pages = flattenNavPages(); | ||
| const aboutChat = pages.find((p) => p.link === '/docs/chat'); | ||
| expect(aboutChat?.sectionKey).toBe('Ably Chat::__root__'); | ||
| }); | ||
|
|
||
| it('should handle nested sections correctly', () => { | ||
| const pages = flattenNavPages(); | ||
| // Chat > Moderation > Direct integrations > Hive (Model Only) | ||
| const hivePage = pages.find((p) => p.link === '/docs/chat/moderation/direct/hive-model-only'); | ||
| expect(hivePage).toBeDefined(); | ||
| expect(hivePage?.sectionKey).toBe('Ably Chat::Direct integrations'); | ||
| }); | ||
|
|
||
| it('should handle mixed sections with both direct pages and nested subsections', () => { | ||
| const pages = flattenNavPages(); | ||
| // Chat > Moderation has Introduction (direct page) + Direct integrations (subsection) | ||
| const introPage = pages.find((p) => p.link === '/docs/chat/moderation'); | ||
| const hivePage = pages.find((p) => p.link === '/docs/chat/moderation/direct/hive-model-only'); | ||
|
|
||
| expect(introPage?.sectionKey).toBe('Ably Chat::Moderation'); | ||
| expect(hivePage?.sectionKey).toBe('Ably Chat::Direct integrations'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Fix indentation: all new test blocks use extra 4 spaces of indentation.
Lines 957–1394 are indented with 8 spaces for it blocks (and proportionally deeper for nested code), whereas the rest of the file uses 4 spaces at the same nesting level. This triggers ~200 Prettier errors. Run npx prettier --write on this file to auto-fix.
🧰 Tools
🪛 ESLint
[error] 958-958: Delete ····
(prettier/prettier)
[error] 959-959: Delete ····
(prettier/prettier)
[error] 960-960: Delete ····
(prettier/prettier)
[error] 961-961: Delete ····
(prettier/prettier)
[error] 962-962: Delete ····
(prettier/prettier)
[error] 963-963: Delete ····
(prettier/prettier)
[error] 964-964: Delete ····
(prettier/prettier)
[error] 965-965: Delete ····
(prettier/prettier)
[error] 966-966: Delete ····
(prettier/prettier)
[error] 967-967: Delete ····
(prettier/prettier)
[error] 968-968: Replace ·········· with ······
(prettier/prettier)
[error] 969-969: Delete ····
(prettier/prettier)
[error] 970-970: Delete ····
(prettier/prettier)
[error] 971-971: Delete ····
(prettier/prettier)
[error] 972-972: Delete ····
(prettier/prettier)
[error] 973-973: Delete ····
(prettier/prettier)
[error] 974-974: Delete ····
(prettier/prettier)
[error] 975-975: Delete ····
(prettier/prettier)
[error] 976-976: Delete ····
(prettier/prettier)
[error] 977-977: Delete ····
(prettier/prettier)
[error] 978-978: Delete ····
(prettier/prettier)
[error] 979-979: Delete ····
(prettier/prettier)
[error] 980-980: Delete ····
(prettier/prettier)
[error] 981-981: Delete ····
(prettier/prettier)
[error] 982-982: Delete ····
(prettier/prettier)
[error] 983-983: Delete ····
(prettier/prettier)
[error] 984-984: Delete ····
(prettier/prettier)
[error] 985-985: Delete ····
(prettier/prettier)
[error] 986-986: Delete ····
(prettier/prettier)
[error] 987-987: Delete ····
(prettier/prettier)
[error] 988-988: Delete ····
(prettier/prettier)
[error] 989-989: Delete ····
(prettier/prettier)
[error] 990-990: Delete ····
(prettier/prettier)
[error] 991-991: Delete ····
(prettier/prettier)
[error] 992-992: Replace ·········· with ······
(prettier/prettier)
[error] 993-993: Delete ····
(prettier/prettier)
[error] 994-994: Delete ····
(prettier/prettier)
[error] 995-995: Delete ····
(prettier/prettier)
[error] 996-996: Delete ····
(prettier/prettier)
[error] 997-997: Delete ····
(prettier/prettier)
[error] 998-998: Delete ····
(prettier/prettier)
[error] 999-999: Delete ····
(prettier/prettier)
[error] 1000-1000: Delete ····
(prettier/prettier)
[error] 1001-1001: Delete ····
(prettier/prettier)
[error] 1002-1002: Delete ····
(prettier/prettier)
[error] 1003-1003: Delete ····
(prettier/prettier)
[error] 1004-1004: Delete ····
(prettier/prettier)
[error] 1005-1005: Delete ····
(prettier/prettier)
[error] 1006-1006: Delete ····
(prettier/prettier)
[error] 1007-1007: Delete ····
(prettier/prettier)
[error] 1008-1008: Replace ·········· with ······
(prettier/prettier)
[error] 1009-1009: Delete ····
(prettier/prettier)
[error] 1010-1010: Delete ····
(prettier/prettier)
[error] 1011-1011: Delete ····
(prettier/prettier)
[error] 1012-1012: Delete ····
(prettier/prettier)
[error] 1013-1013: Delete ····
(prettier/prettier)
[error] 1014-1014: Replace ·········· with ······
(prettier/prettier)
[error] 1015-1015: Delete ····
(prettier/prettier)
[error] 1016-1016: Replace ·········· with ······
(prettier/prettier)
[error] 1017-1017: Delete ····
(prettier/prettier)
[error] 1018-1018: Delete ····
(prettier/prettier)
[error] 1019-1019: Delete ····
(prettier/prettier)
[error] 1020-1020: Replace ·········· with ······
(prettier/prettier)
[error] 1021-1021: Delete ····
(prettier/prettier)
[error] 1022-1022: Replace ········ with ····
(prettier/prettier)
[error] 1023-1023: Delete ····
(prettier/prettier)
[error] 1024-1024: Delete ····
(prettier/prettier)
[error] 1025-1025: Delete ····
(prettier/prettier)
[error] 1026-1026: Replace ·········· with ······
(prettier/prettier)
[error] 1027-1027: Delete ····
(prettier/prettier)
[error] 1028-1028: Replace ········ with ····
(prettier/prettier)
[error] 1029-1029: Delete ····
(prettier/prettier)
[error] 1030-1030: Delete ····
(prettier/prettier)
[error] 1031-1031: Delete ····
(prettier/prettier)
[error] 1032-1032: Replace ·········· with ······
(prettier/prettier)
[error] 1033-1033: Delete ····
(prettier/prettier)
[error] 1034-1034: Delete ····
(prettier/prettier)
[error] 1035-1035: Replace ·········· with ······
(prettier/prettier)
[error] 1036-1036: Delete ····
(prettier/prettier)
[error] 1037-1037: Delete ····
(prettier/prettier)
[error] 1038-1038: Replace ········ with ····
(prettier/prettier)
[error] 1039-1039: Delete ····
(prettier/prettier)
[error] 1040-1040: Delete ····
(prettier/prettier)
[error] 1041-1041: Replace ·········· with ······
(prettier/prettier)
[error] 1042-1042: Delete ····
(prettier/prettier)
[error] 1043-1043: Delete ····
(prettier/prettier)
[error] 1044-1044: Replace ·········· with ······
(prettier/prettier)
[error] 1045-1045: Delete ····
(prettier/prettier)
[error] 1046-1046: Delete ····
(prettier/prettier)
[error] 1047-1047: Delete ····
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/onPostBuild/transpileMdxToMarkdown.test.ts` around lines 957 - 1047, The
test block for describe('flattenNavPages') has incorrect extra indentation
causing many Prettier errors; open the tests around the flattenNavPages
describe/its (the it(...) blocks and their contents) and normalize indentation
to match the rest of the file (use 2 or 4 spaces per nesting level consistent
with existing style), then run the formatter (npx prettier --write) to auto-fix
remaining spacing; ensure the it blocks and nested expectations around
flattenNavPages, chatPages, chatLinks, messagesPage, historyPage, setupPage,
hivePage, introPage retain their original logic but with corrected indentation.
| } else if ('link' in item) { | ||
| // Leaf page | ||
| if (item.external) continue; |
There was a problem hiding this comment.
ESLint: Add curly braces around continue statement.
Static analysis flags the missing curly braces after the if condition on line 560.
Proposed fix
- if (item.external) continue;
+ if (item.external) {
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if ('link' in item) { | |
| // Leaf page | |
| if (item.external) continue; | |
| } else if ('link' in item) { | |
| // Leaf page | |
| if (item.external) { | |
| continue; | |
| } |
🧰 Tools
🪛 ESLint
[error] 560-560: Expected { after 'if' condition.
(curly)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/onPostBuild/transpileMdxToMarkdown.ts` around lines 558 - 560, In the
else-if branch checking ('link' in item) (the "Leaf page" branch) the
single-line if uses continue without braces; update the conditional to use a
block with curly braces around the continue (i.e., change if (item.external)
continue; to if (item.external) { continue; }) so ESLint's brace rule is
satisfied and the behavior remains the same within the ('link' in item) handling
code.
| .map((s): NavLink => ({ | ||
| name: s.name, | ||
| url: linkToMdUrl(s.link), | ||
| description: metaDescriptions.get(s.link) || s.name, | ||
| })); |
There was a problem hiding this comment.
Prettier formatting issue in .map() callback.
ESLint/Prettier flags incorrect formatting on lines 670–674. Run the formatter to fix.
Formatted version
const siblings = (sectionPages.get(page.sectionKey) || [])
.filter((s) => s.link !== page.link)
- .map((s): NavLink => ({
- name: s.name,
- url: linkToMdUrl(s.link),
- description: metaDescriptions.get(s.link) || s.name,
- }));
+ .map(
+ (s): NavLink => ({
+ name: s.name,
+ url: linkToMdUrl(s.link),
+ description: metaDescriptions.get(s.link) || s.name,
+ }),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .map((s): NavLink => ({ | |
| name: s.name, | |
| url: linkToMdUrl(s.link), | |
| description: metaDescriptions.get(s.link) || s.name, | |
| })); | |
| .map( | |
| (s): NavLink => ({ | |
| name: s.name, | |
| url: linkToMdUrl(s.link), | |
| description: metaDescriptions.get(s.link) || s.name, | |
| }), | |
| ); |
🧰 Tools
🪛 ESLint
[error] 670-670: Insert ⏎··········
(prettier/prettier)
[error] 671-671: Insert ··
(prettier/prettier)
[error] 672-672: Insert ··
(prettier/prettier)
[error] 673-673: Insert ··
(prettier/prettier)
[error] 674-674: Replace }) with ··}),⏎········
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/onPostBuild/transpileMdxToMarkdown.ts` around lines 670 - 674, The .map
callback building NavLink objects in transpileMdxToMarkdown.ts is failing
Prettier/ESLint formatting; run the project's Prettier formatter (or apply the
project's formatting rules) to reformat the .map(...) block so it matches the
configured style. Locate the .map that returns NavLink (references: NavLink
type, linkToMdUrl(), metaDescriptions) and re-run the formatter (or fix
spacing/line breaks around the arrow function and object literal) so
lint/Prettier no longer flags lines 670–674.
| // Related Topics section (siblings, excluding pages already in Page Navigation) | ||
| const navUrls = new Set<string>(); | ||
| if (navContext.prev) navUrls.add(navContext.prev.url); | ||
| if (navContext.next) navUrls.add(navContext.next.url); |
There was a problem hiding this comment.
ESLint: Add curly braces around single-line if bodies.
Lines 717–718 are flagged by ESLint (curly rule) for missing braces.
Proposed fix
- if (navContext.prev) navUrls.add(navContext.prev.url);
- if (navContext.next) navUrls.add(navContext.next.url);
+ if (navContext.prev) {
+ navUrls.add(navContext.prev.url);
+ }
+ if (navContext.next) {
+ navUrls.add(navContext.next.url);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Related Topics section (siblings, excluding pages already in Page Navigation) | |
| const navUrls = new Set<string>(); | |
| if (navContext.prev) navUrls.add(navContext.prev.url); | |
| if (navContext.next) navUrls.add(navContext.next.url); | |
| // Related Topics section (siblings, excluding pages already in Page Navigation) | |
| const navUrls = new Set<string>(); | |
| if (navContext.prev) { | |
| navUrls.add(navContext.prev.url); | |
| } | |
| if (navContext.next) { | |
| navUrls.add(navContext.next.url); | |
| } |
🧰 Tools
🪛 ESLint
[error] 717-717: Expected { after 'if' condition.
(curly)
[error] 718-718: Expected { after 'if' condition.
(curly)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/onPostBuild/transpileMdxToMarkdown.ts` around lines 715 - 718, ESLint
flags the single-line if statements that add prev/next URLs to navUrls; update
the block around navUrls so both conditional branches use curly braces (i.e.,
wrap the bodies of the if (navContext.prev) and if (navContext.next) statements)
to satisfy the `curly` rule while preserving the existing behavior of adding
navContext.prev.url and navContext.next.url to the Set.
moved related tests to generateNavigationFooter.test.ts
2 participants
Summary by CodeRabbit
New Features