feat(api): add example API endpoints #190
Conversation
WalkthroughAdds runtime API endpoints for tab redirects, raw page text, example listings, and example source; extends ApiIndex with per-tab examples and extraction logic; moves some routes from prerendered to runtime and updates tests and OpenAPI docs accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MainRoute as /api/{version}/{section}/{page}/{tab}
participant Index as ApiIndex (fetchApiIndex)
participant TextRoute as /api/{version}/{section}/{page}/{tab}/text
participant ExamplesRoute as /api/{version}/{section}/{page}/{tab}/examples
participant ExampleRoute as /api/{version}/{section}/{page}/{tab}/examples/{example}
participant Collections as Content Collections
participant FileSystem as File System
Client->>MainRoute: GET /api/v/.../{tab}
MainRoute->>Index: fetchApiIndex(url)
Index-->>MainRoute: index
MainRoute-->>Client: 302 Redirect to /text
Client->>TextRoute: GET /api/v/.../{tab}/text
TextRoute->>Collections: load entries for version
Collections-->>TextRoute: entries
TextRoute->>TextRoute: validate params & find entry
TextRoute-->>Client: 200 text/markdown
Client->>ExamplesRoute: GET /api/v/.../{tab}/examples
ExamplesRoute->>Index: fetchApiIndex(url)
Index-->>ExamplesRoute: index.examples[key]
ExamplesRoute-->>Client: 200 JSON [{exampleName,title},...]
Client->>ExampleRoute: GET /api/v/.../{tab}/examples/{example}
ExampleRoute->>Collections: load entries for version
Collections-->>ExampleRoute: entries
ExampleRoute->>ExampleRoute: resolve import -> example file path
ExampleRoute->>FileSystem: read example file
FileSystem-->>ExampleRoute: source
ExampleRoute-->>Client: 200 text/plain (source)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts (4)
src/pages/api/[version]/[section]/[page]/[tab]/text.ts (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (6)
Comment |
Deploying patternfly-doc-core with
|
| Latest commit: |
07fc9ad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2a7e59c2.patternfly-doc-core.pages.dev |
| Branch Preview URL: | https://add-example-api.patternfly-doc-core.pages.dev |
![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 (7)
src/utils/apiIndex/generate.ts (1)
57-85: Consider making the regex more robust.The regex
/<LiveExample.*src={(\w*)}.*\/?>/ghas a few potential edge cases:
\w*allows empty captures - consider\w+to require at least one character- The greedy
.*could over-match if multipleLiveExampletags appear on the same lineFor the current documentation content this likely works fine, but a more precise pattern would be safer.
🔎 Optional improvement
- const exampleRegex = /<LiveExample.*src={(\w*)}.*\/?>/g + const exampleRegex = /<LiveExample[^>]*src={(\w+)}[^>]*\/?>/gUsing
[^>]*instead of.*prevents matching across tag boundaries.src/utils/__tests__/apiIndex.test.ts (1)
126-148: Consider adding tests forgetExamples.The test file covers
getVersions,getSections,getPages, andgetTabs, but the newgetExamplesfunction (added inget.ts) doesn't have dedicated tests. Once exported from the index module, consider adding adescribe('getExamples')block.src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts (2)
57-74: Regex patterns may miss valid imports and could cause silent failures.The import regex on line 58 requires both single quotes around the path, but imports can use double quotes. The
?character requirement in the path regex (line 69) seems intentional for raw imports but isn't documented.Consider making the patterns more robust:
🔎 Proposed improvements
function getImports(fileContent: string) { - const importRegex = /import.*from.*['"]\..*\/[\w\.\?]*['"]/gm + const importRegex = /import.*from\s*['"]\..*\/[\w.?]*['"]/gm const matches = fileContent.match(importRegex) return matches } function getExampleFilePath(imports: string[], exampleName: string) { const exampleImport = imports.find((imp) => imp.includes(exampleName)) if (!exampleImport) { console.error('No import path found for example', exampleName) return null } - const match = exampleImport.match(/['"]\..*\/[\w\.]*\?/) + // Match relative path ending with `?raw` query string + const match = exampleImport.match(/['"]\..*\/[\w.]+\?/) if (!match) { return null } return match[0].replace(/['"?]/g, '') }
76-91: Code duplication:getCollectionsis duplicated across files.This function appears nearly identical to code in
text.ts(lines 107-122). Consider extracting to a shared utility.src/pages/api/[version]/[section]/[page]/[tab]/examples.ts (1)
51-53: Remove trailing blank lines.src/pages/api/[version]/[section]/[page]/[tab].ts (1)
26-33: Unused variablesectionKeycreated prematurely.
sectionKeyis created on line 27 but the section check on line 28 usesindex.sections[version]. ThesectionKeyis only needed later for the page check. Consider moving it closer to its usage.🔎 Proposed fix
// Check if section exists for this version - const sectionKey = createIndexKey(version, section) if (!index.sections[version]?.includes(section)) { return createJsonResponse( { error: `Section '${section}' not found for version '${version}'` }, 404, ) } // Check if page exists for this section + const sectionKey = createIndexKey(version, section) const pageKey = createIndexKey(version, section, page)src/pages/api/[version]/[section]/[page]/[tab]/text.ts (1)
68-104: Validation logic duplicated with[tab].ts.The index validation (lines 68-104) is nearly identical to the validation in
[tab].ts(lines 18-54). Consider extracting to a shared validation helper.🔎 Suggested approach
Create a shared validation function in
apiHelpers.ts:export async function validateTabPath( index: ApiIndex, version: string, section: string, page: string, tab: string ): { valid: true } | { valid: false; response: Response } { if (!index.versions.includes(version)) { return { valid: false, response: createJsonResponse({ error: `Version '${version}' not found` }, 404) } } // ... rest of validation return { valid: true } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/pages/api/[version]/[section]/[page]/[tab].ts(3 hunks)src/pages/api/[version]/[section]/[page]/[tab]/examples.ts(1 hunks)src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts(1 hunks)src/pages/api/[version]/[section]/[page]/[tab]/text.ts(1 hunks)src/pages/api/index.ts(4 hunks)src/pages/api/openapi.json.ts(3 hunks)src/utils/__tests__/apiIndex.test.ts(1 hunks)src/utils/apiIndex/generate.ts(5 hunks)src/utils/apiIndex/get.ts(2 hunks)src/utils/apiIndex/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/utils/__tests__/apiIndex.test.ts (2)
src/utils/apiIndex/get.ts (5)
getApiIndex(13-48)getVersions(55-58)getSections(66-69)getPages(78-83)getTabs(93-98)src/utils/apiHelpers.ts (1)
createIndexKey(48-50)
src/pages/api/[version]/[section]/[page]/[tab]/text.ts (5)
src/utils/apiIndex/generate.ts (1)
generateAndWriteApiIndex(212-216)src/utils/apiHelpers.ts (3)
createIndexKey(48-50)createJsonResponse(18-27)createTextResponse(29-37)src/utils/apiIndex/get.ts (1)
getApiIndex(13-48)src/utils/packageUtils.ts (2)
getDefaultTabForApi(69-69)addDemosOrDeprecated(44-58)src/utils/case.ts (1)
kebabCase(9-10)
src/pages/api/[version]/[section]/[page]/[tab]/examples.ts (3)
src/pages/api/[version]/[section]/[page]/[tab].ts (1)
GET(8-58)src/utils/apiIndex/get.ts (1)
getApiIndex(13-48)src/utils/apiHelpers.ts (2)
createIndexKey(48-50)createJsonResponse(18-27)
src/utils/apiIndex/get.ts (1)
src/utils/apiHelpers.ts (1)
createIndexKey(48-50)
src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts (6)
src/utils/apiIndex/generate.ts (1)
generateAndWriteApiIndex(212-216)src/utils/apiIndex/index.ts (1)
generateAndWriteApiIndex(21-21)src/utils/apiHelpers.ts (2)
createIndexKey(48-50)createTextResponse(29-37)src/__mocks__/astro-content.ts (1)
getCollection(4-4)src/utils/packageUtils.ts (1)
getDefaultTab(60-65)src/utils/case.ts (1)
kebabCase(9-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (22.x)
🔇 Additional comments (17)
src/utils/apiIndex/get.ts (2)
26-28: LGTM!The validation for the
examplesobject is consistent with the existingversionsvalidation pattern and provides a clear error message.
109-119: LGTM!The
getExamplesfunction follows the established pattern of other getter functions (getTabs,getPages,getSections) with consistent use of dynamic imports forcreateIndexKeyand safe fallback to an empty array when the key is not found.src/utils/apiIndex/generate.ts (2)
46-48: LGTM!The
examplesfield definition with its descriptive comment clearly documents the key format and value structure.
159-165: LGTM!The examples collection logic correctly extracts examples per tab and only stores entries when examples are found.
src/utils/__tests__/apiIndex.test.ts (2)
1-67: LGTM!Comprehensive test coverage for the API index structure, including the new
examplesfield validation with proper checks for key format and value structure.
150-185: LGTM!The architecture test validates the full hierarchical navigation flow including the new examples layer, with appropriate handling for tabs that may not have examples.
src/pages/api/index.ts (3)
135-173: LGTM!The updated tab endpoint documentation correctly describes the redirect behavior to
/textand provides complete parameter documentation.
214-304: LGTM!The new endpoints for
/examples(list) and/examples/{example}(code retrieval) are well-documented with appropriate parameters, return types, and examples that align with the PR objectives.
306-323: LGTM!The updated usage flow and architecture documentation clearly explains the API navigation pattern including the new examples endpoints and the runtime optimization strategy.
src/pages/api/openapi.json.ts (3)
309-379: LGTM!The OpenAPI spec for the tab validation/redirect endpoint is well-documented with appropriate 302 and 404 response definitions.
460-553: Verify the 400 response is reachable.The spec documents a
400response for "Missing required parameters", but since all parameters are path parameters (not query params), they should always be present when the route matches. Consider if404would be more appropriate here for invalid combinations, or verify that the handler implementation can actually return 400.
554-643: LGTM!The OpenAPI spec for the example code endpoint is complete with proper parameter definitions and response schemas including the 404 case for missing examples.
src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts (1)
13-55: Static path generation looks correct.The nested iteration over index structure properly builds all example paths. The use of
generateAndWriteApiIndex()ensures the index is available at build time.src/pages/api/[version]/[section]/[page]/[tab]/examples.ts (1)
34-50: GET handler implementation is clean.The handler correctly validates params, fetches the index, and returns examples. The fallback to empty array (
|| []) handles missing keys gracefully.src/pages/api/[version]/[section]/[page]/[tab].ts (1)
8-57: Validation and redirect logic is well-structured.The cascading validation provides clear error messages at each level, and the redirect to
/textcleanly separates the routing concern from content delivery.src/pages/api/[version]/[section]/[page]/[tab]/text.ts (2)
47-55: Defensive coding for tabless paths is good.The warning and filter provide safety against build crashes while alerting developers to potential issues.
149-152: Empty string fallback for missing body content.Returning an empty string when
matchingEntry.bodyis undefined may not be the desired behavior. Consider returning a 404 or 204 (No Content) if the body is genuinely empty.Is returning empty text intentional for entries without body content, or should this be treated as an error condition?
src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/api/[version]/[section]/[page]/[tab].ts (1)
17-53: Add error handling for fetchApiIndex failures.Similar to the examples endpoint, the
fetchApiIndex(url)call can reject if the fetch fails or the response cannot be parsed. Without error handling, this will result in an unclear 500 error.🔎 Recommended fix: Add try-catch wrapper
- // Validate using index first (fast path for 404s) - const index = await fetchApiIndex(url) - - // Check if version exists - if (!index.versions.includes(version)) { + // Validate using index first (fast path for 404s) + let index + try { + index = await fetchApiIndex(url) + } catch (error) { + return createJsonResponse( + { error: `Failed to fetch API index: ${error instanceof Error ? error.message : 'Unknown error'}` }, + 500, + ) + } + + // Check if version exists + if (!index.versions.includes(version)) {Otherwise, the validation logic is well-structured.
The hierarchical validation checks (version → section → page → tab) are logical and provide clear, specific error messages for each failure case.
🧹 Nitpick comments (1)
src/utils/apiIndex/generate.ts (1)
50-87: LGTM! Example extraction logic is sound.The function correctly extracts LiveExample components and associates them with preceding H3 headings. The regex pattern and logic are appropriate for typical JSX/TSX usage.
Optional: Document regex assumptions
Consider adding a comment noting that the regex expects
src={ExampleName}format (no quotes, standard JSX identifier syntax) to help future maintainers understand the supported patterns.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/__tests__/pages/api/__tests__/[version]/[section]/[page]/[tab].test.ts(5 hunks)src/pages/api/[version]/[section]/[page]/[tab].ts(3 hunks)src/pages/api/[version]/[section]/[page]/[tab]/examples.ts(1 hunks)src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts(1 hunks)src/utils/__tests__/apiIndex.test.ts(1 hunks)src/utils/apiIndex/generate.ts(5 hunks)src/utils/apiIndex/get.ts(2 hunks)src/utils/apiIndex/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/utils/tests/apiIndex.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/api/[version]/[section]/[page]/[tab]/examples/[example].ts
- src/utils/apiIndex/index.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/__tests__/pages/api/__tests__/[version]/[section]/[page]/[tab].test.ts (3)
src/pages/api/[version]/[section]/[page]/[tab].ts (1)
GET(7-57)src/pages/api/[version]/[section].ts (1)
GET(7-37)src/pages/api/[version]/[section]/[page].ts (1)
GET(7-38)
src/pages/api/[version]/[section]/[page]/[tab]/examples.ts (2)
src/utils/apiHelpers.ts (2)
createJsonResponse(18-27)createIndexKey(48-50)src/utils/apiIndex/index.ts (1)
fetchApiIndex(27-27)
src/utils/apiIndex/generate.ts (1)
src/utils/packageUtils.ts (1)
addDemosOrDeprecated(44-58)
src/utils/apiIndex/get.ts (2)
src/utils/apiIndex/index.ts (2)
getExamples(25-25)getApiIndex(25-25)src/utils/apiHelpers.ts (1)
createIndexKey(48-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (22.x)
🔇 Additional comments (14)
src/pages/api/[version]/[section]/[page]/[tab]/examples.ts (2)
1-5: LGTM! Configuration is correct for SSR.The prerender flag and imports are appropriate for an on-demand server-rendered route. The past review concern about
getStaticPathshas been addressed (it's not present in the current code).
7-15: LGTM! Parameter validation is thorough.The validation correctly checks all required parameters and returns a clear 400 error when any are missing.
src/utils/apiIndex/get.ts (2)
26-28: LGTM! Validation aligns with existing patterns.The runtime validation for
index.examplesis consistent with the existing validation forindex.versionsand provides a clear error message.
100-119: LGTM! Implementation follows established patterns.The
getExamplesfunction is consistent with the existing getter functions (getVersions,getSections,getPages,getTabs) in terms of structure, error handling, and the use of dynamic imports forcreateIndexKey.src/utils/apiIndex/generate.ts (4)
46-48: LGTM! Interface extension is well-documented.The
examplesfield addition to theApiIndexinterface is clear, well-typed, and includes a helpful JSDoc example.
162-167: LGTM! Example collection logic is correct.The code correctly builds the composite key, extracts examples from the entry body, and stores them in the map only when examples are found.
181-183: LGTM! Index population follows established patterns.The examples are populated into the index using the same approach as pages and tabs, maintaining consistency across the codebase.
156-156: No action needed. The use ofentry.filePathinaddDemosOrDeprecated(entryTab, entry.filePath)is correct. The function checks whether the file path contains 'demos' or 'deprecated' directory segments to append appropriate suffixes to the tab name. Sinceentry.filePathrepresents the actual file path, it is the appropriate data source for detecting these directory-based classifications. Comprehensive test coverage confirms this behavior works as intended.src/__tests__/pages/api/__tests__/[version]/[section]/[page]/[tab].test.ts (3)
134-150: LGTM! Mock setup is appropriate.The
fetchApiIndexmock returns data consistent with the existinggetApiIndexmock, providing the necessary structure for the redirect-based tests.
156-221: LGTM! Redirect test coverage is thorough.The tests correctly verify the redirect behavior, including proper status codes (302), target paths, and coverage across different tabs. The use of fresh
mockRedirectinstances per test prevents cross-test pollution.
223-314: LGTM! Error handling test coverage is comprehensive.The tests validate all error scenarios (404 for nonexistent entities, 400 for missing parameters) with appropriate status codes and descriptive error messages.
src/pages/api/[version]/[section]/[page]/[tab].ts (3)
1-5: LGTM! Configuration is correct for SSR.The imports and
prerender = falsesetting are appropriate for the new redirect-based, server-rendered approach.
7-15: LGTM! Parameter validation is consistent and thorough.The validation correctly checks all required parameters and provides a clear error message for missing values.
55-56: LGTM! Redirect implementation is clean.The redirect to the
/textendpoint is straightforward and maintains good separation of concerns between validation and content retrieval.
closes #169
Summary by CodeRabbit
New Features
Behavior Changes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.