feat(api): add api endpoint for component props #198
Conversation
WalkthroughAdds two new API endpoints that expose component props: one to list component names with props and another to return props for a specific component. Both load configuration, read Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Astro API Route
participant Config as getConfig (pf-docs.config.mjs)
participant FS as Filesystem (props.json)
Client->>API: GET /api/{version}/{section}/{page}/props
API->>API: validate params (version, section, page)
API->>Config: load config to determine outputDir
Config-->>API: outputDir
API->>FS: read `{outputDir}/props.json`
FS-->>API: props.json content
API->>API: select props by sentenceCased(page)
API-->>Client: 200 JSON (props) or 404/500 error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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
🤖 Fix all issues with AI agents
In `@src/pages/api/`[version]/[section]/[page]/props.ts:
- Around line 28-29: The handler currently assigns const propsData =
props[sentenceCase(page)] and always returns createJsonResponse(propsData);
instead, detect when propsData is undefined/null and return a 404 response for
missing component props. Modify the code around propsData (lookup using props
and sentenceCase(page)) so that if propsData == null you return an HTTP 404 (for
example by calling your JSON response helper with a 404 status or returning a
404 Response) otherwise continue to return createJsonResponse(propsData).
- Line 6: Replace the direct import of sentenceCase from the external
'change-case' package with the project's custom wrapper to ensure
PatternFly-specific casing; update the import to pull sentenceCase from the
local module (the wrapper defined in src/utils/case.ts) and keep all usages of
sentenceCase (the function) unchanged so component name lookups use the
PatternFly-aware casing logic.
In `@src/pages/api/openapi.json.ts`:
- Around line 266-282: The OpenAPI response block under responses '200' uses an
invalid media type key "array"; update the content key to a valid MIME type
("application/json") for the '200' response and keep the existing schema (type:
"array", items: { type: "string" }) and example intact. Locate the responses ->
'200' object in src/pages/api/openapi.json.ts (the schema and example currently
under the "array" key) and replace that key with "application/json" so the
OpenAPI document validates.
- Around line 412-450: The responses.content entry incorrectly uses the media
type key "array" instead of a valid media type; update the responses -> '200' ->
content block (the schema and example under content) to use "application/json"
as the media type key and keep the existing schema, items, properties and
example intact so the OpenAPI response for the component props remains the same
structure but under a valid content type.
🧹 Nitpick comments (4)
src/pages/api/[version]/[section]/names.ts (2)
31-36: Consider returning 404 for missingprops.jsoninstead of 500.Per the PR objectives, the endpoint should "return 404 when props do not exist for the requested component" and "handle the case where props.json has not been generated." Currently, all errors (including
ENOENTfor missing file) return a 500 status. Differentiating between "file not found" (404) and unexpected errors (500) would improve API clarity.Suggested improvement
} catch (error) { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { + return createJsonResponse( + { error: 'Component names data not found' }, + 404, + ) + } const details = error instanceof Error ? error.message : String(error) return createJsonResponse( - { error: 'Component names data not found', details }, + { error: 'Failed to load component names', details }, 500, ) }
39-41: Remove trailing blank lines.Minor: there are multiple trailing blank lines at the end of the file.
src/pages/api/[version]/[section]/[page]/props.ts (2)
31-36: Same error handling improvement asnames.ts—differentiate 404 from 500.Consider returning 404 for
ENOENT(missingprops.json) and 500 for unexpected errors, aligning with PR objectives.
39-41: Remove trailing blank lines.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/pages/api/[version]/[section]/[page]/props.tssrc/pages/api/[version]/[section]/names.tssrc/pages/api/index.tssrc/pages/api/openapi.json.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/api/[version]/[section]/names.ts (4)
src/pages/api/[version]/[section]/[page]/props.ts (2)
prerender(8-8)GET(10-38)src/pages/api/openapi.json.ts (2)
prerender(15-15)GET(17-824)src/utils/apiHelpers.ts (1)
createJsonResponse(18-27)cli/getConfig.ts (1)
getConfig(23-37)
src/pages/api/[version]/[section]/[page]/props.ts (3)
src/utils/apiHelpers.ts (1)
createJsonResponse(18-27)cli/getConfig.ts (1)
getConfig(23-37)src/utils/case.ts (1)
sentenceCase(12-16)
🔇 Additional comments (5)
src/pages/api/[version]/[section]/names.ts (1)
19-30: Theversionandsectionparameters are validated but not used.The endpoint validates that
versionandsectionare provided, but the implementation reads the sameprops.jsonand returns all component names regardless of these parameters. This creates a semantic inconsistency—the URL path suggests version/section scoping, but the response is global.If intentional (e.g., props.json is already version-specific at build time), consider documenting this behavior. Otherwise, the implementation should filter or route based on these parameters.
src/pages/api/index.ts (2)
101-130: LGTM!The documentation for the new
/namesendpoint is accurate and follows the established pattern in this file.
165-204: LGTM!The documentation for the new
/propsendpoint is well-structured with appropriate examples.src/pages/api/openapi.json.ts (2)
284-298: Spec documents 404 but implementation returns 500 for all errors.The OpenAPI spec shows a 404 response for "Props not found," but the current implementation in
names.tsreturns 500 for all errors. Consider aligning the implementation with the spec by returning 404 whenprops.jsonis missing.
452-466: Same spec/implementation mismatch for 404 response.The 404 response is documented here, but
props.tsreturns 500 for file errors and 200 with undefined for missing components. Align with the earlier suggestion to return proper 404 responses.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/api/`[version]/[section]/[page]/props.ts:
- Around line 10-18: The handler currently validates params in GET ({ params })
but never uses version or section when loading props.json; update the handler to
incorporate version and section into the props lookup path (e.g., build a scoped
path using version and section along with page when reading props.json) so
multi-version/section content is supported, and keep the existing parameter
validation and error response via createJsonResponse; alternatively, if the
routing-only intent is desired, add a clear comment above GET explaining that
version and section are validated for routing but intentionally not used for
props resolution (mirror the same choice in names.ts).
🧹 Nitpick comments (2)
src/pages/api/[version]/[section]/[page]/props.ts (2)
24-26: Prefer async file reading to avoid blocking the event loop.
readFileSyncblocks the Node.js event loop. In an SSR API handler, this can degrade performance under concurrent requests. Use the asyncfs.promises.readFileinstead.♻️ Suggested refactor
-import { readFileSync } from 'node:fs' +import { readFile } from 'node:fs/promises'const propsFilePath = join(outputDir, 'props.json') - const propsDataFile = readFileSync(propsFilePath) - const props = JSON.parse(propsDataFile.toString()) + const propsDataFile = await readFile(propsFilePath, 'utf-8') + const props = JSON.parse(propsDataFile)
39-45: Consider distinguishing "file not found" from other server errors.When
props.jsondoesn't exist, the current code returns a 500 error. Per PR objectives, the endpoint should gracefully handle the absence ofprops.json. A 404 would be more semantically appropriate for missing data, reserving 500 for unexpected failures.♻️ Suggested approach
} catch (error) { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { + return createJsonResponse( + { error: 'Props data not available' }, + 404, + ) + } const details = error instanceof Error ? error.message : String(error) return createJsonResponse( { error: 'Props data not found', details }, 500, ) }
| export const GET: APIRoute = async ({ params }) => { | ||
| const { version, section, page } = params | ||
|
|
||
| if (!version || !section || !page) { | ||
| return createJsonResponse( | ||
| { error: 'Version, section, and page parameters are required' }, | ||
| 400, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the full props.ts file to understand the complete logic
cat -n src/pages/api/[version]/[section]/[page]/props.tsRepository: patternfly/patternfly-doc-core
Length of output: 1775
🏁 Script executed:
# Check if props.json exists and its structure
find . -name 'props.json' -type f 2>/dev/null | head -10Repository: patternfly/patternfly-doc-core
Length of output: 56
🏁 Script executed:
# Check the names.ts file to understand parameter usage pattern
if [ -f 'src/pages/api/[version]/[section]/names.ts' ]; then
cat -n src/pages/api/[version]/[section]/names.ts
else
echo "names.ts not found at expected location"
fiRepository: patternfly/patternfly-doc-core
Length of output: 1595
🏁 Script executed:
# Search for any usage of version or section parameters in API route files
rg "version|section" src/pages/api --type ts -A 3 -B 1 | head -100Repository: patternfly/patternfly-doc-core
Length of output: 5826
version and section parameters are validated but never used in the handler logic.
The handler validates that version, section, and page are all present (lines 13–17), but only page is used to retrieve props data at line 28. The props.json file is loaded as a single file without version/section scoping. Either document this intentional design (the parameters serve only for routing), or refactor the lookup to incorporate version and section for proper multi-version support. Note that names.ts follows the same pattern.
🤖 Prompt for AI Agents
In `@src/pages/api/`[version]/[section]/[page]/props.ts around lines 10 - 18, The
handler currently validates params in GET ({ params }) but never uses version or
section when loading props.json; update the handler to incorporate version and
section into the props lookup path (e.g., build a scoped path using version and
section along with page when reading props.json) so multi-version/section
content is supported, and keep the existing parameter validation and error
response via createJsonResponse; alternatively, if the routing-only intent is
desired, add a clear comment above GET explaining that version and section are
validated for routing but intentionally not used for props resolution (mirror
the same choice in names.ts).
1 participant
Closes #170.
Adds
/api/{version}/{section}/{page}/propsand/api/{version}/{section}/namesendpoints.Based on slack conversation, the props endpoint is currently scoped to directly querying a single component for its props.
Eventually we would like subcomponent props to also be returned when applicable, but we need to generate a mapping of all our component families to properly fetch the data (as string matching based on the queried component name would likely not be a comprehensive list - "ProgressStepper" not returning "ProgressStep" props, or return too much data - "Alert" returning "FormAlert" props). In the interim I've added another endpoint to return all available component names based on our generated props data, so a user/AI could query that initially to understand what components props are available than try to guess at component/subcomponent names.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.