feat(API): Add CSS API table support #205
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds a GET API route to expose per-page CSS tokens, a token extractor for PatternFly react-tokens, integration to populate the API index with per-page CSS token maps, an accessor to read those tokens, and comprehensive unit tests; route prerendering disabled. (28 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as CSS Route Handler
participant Index as API Index Manager
participant Extractor as Token Extractor
participant FS as File System
Client->>Route: GET /api/{version}/{section}/{page}/css
Route->>Route: validate params
Route->>Index: fetchApiIndex(url)
Index-->>Route: apiIndex (includes css map)
Route->>Route: build pageKey & lookup index.css[pageKey]
alt tokens present
Route-->>Client: 200 + tokens JSON
else tokens empty
Route-->>Client: 200 + [] JSON
end
Note over Index,Extractor: Offline index generation flow
Index->>Extractor: extractReactTokens(prefixes)
Extractor->>FS: import componentIndex & token modules
loop per matching token entry
FS-->>Extractor: token data
Extractor->>Extractor: filter, normalize, build {name,value,var}
end
Extractor-->>Index: return sorted tokens
Index->>Index: store tokens in index.css[pageKey]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
dlabaj
changed the title
Deploying patternfly-doc-core with
|
| Latest commit: |
667eb46
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4e18ab36.patternfly-doc-core.pages.dev |
| Branch Preview URL: | https://css-update.patternfly-doc-core.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/api/index.ts`:
- Around line 135-169: The OpenAPI schema for the route with path
'/api/{version}/{section}/{page}/css' defines CSS tokens as tokenName and
variableName but the actual response uses name, value, and var; update the
route's returns schema and example (the returns object in that route definition)
to use properties name (string), value (string), and var (string) and adjust the
example array item accordingly so docs match the real payload.
🧹 Nitpick comments (1)
src/utils/apiIndex/generate.ts (1)
137-138: Consider parallelizing token extraction to speed builds.
Extraction is awaited serially per page; this can add noticeable build time as the number of pages grows.⚡️ Parallelize extraction
- for (const [pageKey, cssPrefix] of Object.entries(pageCssPrefixes)) { - try { - const tokens = await extractReactTokens(cssPrefix) - if (tokens.length > 0) { - pageCss[pageKey] = tokens - } - } catch (error) { - console.warn(`Failed to extract CSS tokens for ${pageKey}:`, error) - } - } + await Promise.all( + Object.entries(pageCssPrefixes).map(async ([pageKey, cssPrefix]) => { + try { + const tokens = await extractReactTokens(cssPrefix) + if (tokens.length > 0) { + pageCss[pageKey] = tokens + } + } catch (error) { + console.warn(`Failed to extract CSS tokens for ${pageKey}:`, error) + } + }), + )Also applies to: 175-191
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/utils/__tests__/extractReactTokens.test.ts`:
- Line 124: Tests in src/utils/__tests__/extractReactTokens.test.ts assign the
return value to result (from calling extractReactTokens) but never use it,
causing lint errors; either remove the unused assignment or assert on result
where appropriate. Update the test lines that call extractReactTokens (the const
result = await extractReactTokens(...) statements) by deleting the "const result
= " portion when only mock call assertions are needed, or add explicit
assertions using result (e.g., expect(result).toEqual(...)) if the returned
value should be verified; ensure this change is applied for all occurrences of
"const result = await extractReactTokens" in the file.
In `@src/utils/apiIndex/generate.ts`:
- Around line 49-50: The css property type uses forbidden Array<T> syntax;
update the type annotation in src/utils/apiIndex/generate.ts for the css field
(the Record<string, Array<{ name: string; value: string; var: string }>>) to use
the bracket form instead (Record<string, { name: string; value: string; var:
string }[]>) so it passes ESLint's rule against Array<T>.
- Around line 137-138: The lint error is from using Array<...> syntax; update
the types in generate.ts to use the [] shorthand: change pageCss's type from
Record<string, Array<{ name: string; value: string; var: string }>> to
Record<string, { name: string; value: string; var: string }[]> and ensure
pageCssPrefixes uses string | string[] (i.e., no Array<string> form) so both
pageCss and pageCssPrefixes use the bracket-style array types.
In `@src/utils/apiIndex/get.ts`:
- Around line 133-142: Update the return type of getCssTokens to use the
square-bracket array syntax instead of Array<T>; specifically change
Promise<Array<{ name: string; value: string; var: string }>> to Promise<{ name:
string; value: string; var: string }[]> in the getCssTokens function signature
so it complies with the ESLint rule forbidding Array<T>.
In `@src/utils/extractReactTokens.ts`:
- Around line 65-115: Change the tokenObjects type from Array<{ name: string;
value: string; var: string }> to the more idiomatic TypeScript array syntax {
name: string; value: string; var: string }[] for the tokenObjects declaration,
and add targeted ESLint disables: place an eslint-disable-next-line no-new-func
immediately above the new Function(...) call (where tokenObject is evaluated)
and an eslint-disable-next-line
`@typescript-eslint/no-unsafe-assignment/`@typescript-eslint/no-unsafe-call as
needed where tokenObject is assigned/used to suppress the type-safety warnings;
reference tokenObjects, matchingFiles, objectMatch, new Function and tokenObject
to locate the spots to change.
- Around line 22-42: In extractReactTokens update the return type from
Promise<Array<{ name: string; value: string; var: string }>> to use shorthand
array syntax Promise<{ name: string; value: string; var: string }[]> and replace
any other Array<T> occurrences with T[]; then explicitly suppress the
intentional console.error in the tokensDir check by adding an
eslint-disable-next-line no-console comment immediately above the console.error
call (inside extractReactTokens, near the tokensDir variable/existsSync check)
so linting passes while preserving the diagnostic output.
♻️ Duplicate comments (1)
src/pages/api/index.ts (1)
162-169: Docs mismatch: CSS token fields don't match actual payload.The schema uses
tokenNameandvariableName, but the actual response fromgetCssTokensreturns{ name, value, var }. Align the docs with the real response shape.
🧹 Nitpick comments (1)
src/utils/__tests__/extractReactTokens.test.ts (1)
94-97: Consider suppressing no-console for test file assertions.The ESLint rule is flagging
expect(console.error)assertions as unexpected console statements. These are legitimate test assertions verifying error logging behavior, not actual console calls. Consider adding an eslint-disable comment at the top of the test file or configuring the rule to allow console in test files.🔧 Proposed fix at top of file
+/* eslint-disable no-console */ import { readdir, readFile } from 'fs/promises' import { existsSync } from 'fs'
src/utils/extractReactTokens.ts
Outdated
Comment on lines
65
to
115
| // Import and extract objects from each matching file | ||
| const tokenObjects: Array<{ name: string; value: string; var: string }> = [] | ||
|
|
||
| for (const file of matchingFiles) { | ||
| try { | ||
| const filePath = join(tokensDir, file) | ||
| const fileContent = await readFile(filePath, 'utf8') | ||
|
|
||
| // Extract the exported object using regex | ||
| // Pattern: export const variableName = { "name": "...", "value": "...", "var": "..." }; | ||
| // Use non-greedy match to get just the first exported const object | ||
| const objectMatch = fileContent.match( | ||
| /export const \w+ = \{[\s\S]*?\n\};/, | ||
| ) | ||
|
|
||
| if (objectMatch) { | ||
| // Parse the object string to extract the JSON-like object | ||
| const objectContent = objectMatch[0] | ||
| .replace(/export const \w+ = /, '') | ||
| .replace(/;$/, '') | ||
|
|
||
| try { | ||
| // Use Function constructor for safe evaluation | ||
| // The object content is valid JavaScript, so we can evaluate it | ||
| const tokenObject = new Function(`return ${objectContent}`)() as { | ||
| name: string | ||
| value: string | ||
| var: string | ||
| } | ||
|
|
||
| if ( | ||
| tokenObject && | ||
| typeof tokenObject === 'object' && | ||
| typeof tokenObject.name === 'string' && | ||
| typeof tokenObject.value === 'string' && | ||
| typeof tokenObject.var === 'string' | ||
| ) { | ||
| tokenObjects.push({ | ||
| name: tokenObject.name, | ||
| value: tokenObject.value, | ||
| var: tokenObject.var, | ||
| }) | ||
| } | ||
| } catch (evalError) { | ||
| console.warn(`Failed to parse object from ${file}:`, evalError) | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.warn(`Failed to read file ${file}:`, error) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Fix array type syntax and add eslint-disable comments for warnings.
🔧 Proposed fix
// Import and extract objects from each matching file
- const tokenObjects: Array<{ name: string; value: string; var: string }> = []
+ const tokenObjects: { name: string; value: string; var: string }[] = []
for (const file of matchingFiles) {
try {
const filePath = join(tokensDir, file)
const fileContent = await readFile(filePath, 'utf8')
// Extract the exported object using regex
// Pattern: export const variableName = { "name": "...", "value": "...", "var": "..." };
// Use non-greedy match to get just the first exported const object
const objectMatch = fileContent.match(
/export const \w+ = \{[\s\S]*?\n\};/,
)
if (objectMatch) {
// Parse the object string to extract the JSON-like object
const objectContent = objectMatch[0]
.replace(/export const \w+ = /, '')
.replace(/;$/, '')
try {
// Use Function constructor for safe evaluation
// The object content is valid JavaScript, so we can evaluate it
const tokenObject = new Function(`return ${objectContent}`)() as {
name: string
value: string
var: string
}
if (
tokenObject &&
typeof tokenObject === 'object' &&
typeof tokenObject.name === 'string' &&
typeof tokenObject.value === 'string' &&
typeof tokenObject.var === 'string'
) {
tokenObjects.push({
name: tokenObject.name,
value: tokenObject.value,
var: tokenObject.var,
})
}
} catch (evalError) {
+ // eslint-disable-next-line no-console
console.warn(`Failed to parse object from ${file}:`, evalError)
}
}
} catch (error) {
+ // eslint-disable-next-line no-console
console.warn(`Failed to read file ${file}:`, error)
}
}🧰 Tools
🪛 ESLint
[error] 66-66: Array type using 'Array' is forbidden. Use 'T[]' instead.
(@typescript-eslint/array-type)
[error] 109-109: Unexpected console statement.
(no-console)
[error] 113-113: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
In `@src/utils/extractReactTokens.ts` around lines 65 - 115, Change the
tokenObjects type from Array<{ name: string; value: string; var: string }> to
the more idiomatic TypeScript array syntax { name: string; value: string; var:
string }[] for the tokenObjects declaration, and add targeted ESLint disables:
place an eslint-disable-next-line no-new-func immediately above the new
Function(...) call (where tokenObject is evaluated) and an
eslint-disable-next-line
`@typescript-eslint/no-unsafe-assignment/`@typescript-eslint/no-unsafe-call as
needed where tokenObject is assigned/used to suppress the type-safety warnings;
reference tokenObjects, matchingFiles, objectMatch, new Function and tokenObject
to locate the spots to change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/__tests__/pages/api/__tests__/`[version]/[section]/[page]/css.test.ts:
- Around line 205-236: The test title is misleading: change the it(...)
description to reflect that a 404 with an error message is expected (e.g.,
"returns 404 when CSS tokens array exists but is empty") so it matches the
assertions; locate the test case using the GET handler invocation and the mocked
fetchApiIndex object in the test that currently asserts response.status
toBe(404) and body.error contains 'No CSS tokens found' and update only the
string passed to it(...) to a description matching the expectations.
src/__tests__/pages/api/__tests__/[version]/[section]/[page]/css.test.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/extractReactTokens.ts`:
- Around line 67-120: The code mutates the shared tokenObjects array from within
concurrent callbacks (matchingFiles.map used inside Promise.all), causing a race
condition; change each async callback in matchingFiles.map to return either the
parsed token object or null (or an array of objects) instead of calling
tokenObjects.push, then after await Promise.all(...) flatten the returned
results into tokenObjects (e.g., const results = await Promise.all(...);
tokenObjects.push(...results.flat().filter(Boolean))). Keep the existing parsing
logic (objectMatch, new Function, tokenObject) but remove in-callback pushes and
move the single-threaded aggregation to after the Promise.all resolution.
🧹 Nitpick comments (1)
src/utils/extractReactTokens.ts (1)
88-95: Clarify comment:new Function()is not inherently "safe".The comment "Use Function constructor for safe evaluation" is misleading—
new Function()executes arbitrary code similar toeval(). While acceptable here since the source is trusted (node_modules), the comment should reflect that safety relies on the trusted input source, not the mechanism itself.📝 Suggested comment update
try { - // Use Function constructor for safe evaluation - // The object content is valid JavaScript, so we can evaluate it + // Evaluate the object content using Function constructor + // This is acceptable as input comes from trusted node_modules source const tokenObject = new Function(`return ${objectContent}`)() as {
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/utils/extractReactTokens.ts`:
- Around line 115-119: The function returns tokenObjects sorted by name twice —
remove the duplicate unreachable return; locate the sort/return lines operating
on tokenObjects (the Array.sort((a, b) => a.name.localeCompare(b.name)) call) in
extractReactTokens (or the surrounding function) and delete the repeated return
so only a single return tokenObjects.sort(...) remains.
- Around line 46-56: The filter callback assigned to matchingFiles is left
unclosed and missing its return logic; inside the files.filter((file) => { ...
}) callback (in extractReactTokens / matchingFiles), add the missing test that
returns true for .js files that start with any token prefix and are not the main
component file (e.g., ensure it checks the token prefixes array and excludes
files like 'c_accordion.js'), then close the callback and the .filter(...)
expression (add the closing brace and closing parenthesis/semicolon) before
starting the results = await Promise.all(...) block so the async/await is inside
a valid async function scope.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/extractReactTokens.ts`:
- Around line 67-105: Wrap the per-file processing inside the matchingFiles.map
async callback in a try/catch (or switch to Promise.allSettled) so a single
read/parse/eval error doesn't reject the whole Promise.all; catch errors thrown
by readFile, the regex match, or the new Function(...)() evaluation (references:
matchingFiles.map callback, fileContent.match, new Function(...), and
tokenObjects.push) and log the error with the project logger before continuing
to the next file. Ensure the catch avoids rethrowing so token extraction
proceeds for other files.
src/utils/extractReactTokens.ts
Outdated
Comment on lines
67
to
105
| await Promise.all( | ||
| matchingFiles.map(async (file) => { | ||
| const filePath = join(tokensDir, file) | ||
| const fileContent = await readFile(filePath, 'utf8') | ||
|
|
||
| // Extract the exported object using regex | ||
| // Pattern: export const variableName = { "name": "...", "value": "...", "var": "..." }; | ||
| // Use non-greedy match to get just the first exported const object | ||
| const objectMatch = fileContent.match( | ||
| /export const \w+ = \{[\s\S]*?\n\};/, | ||
| ) | ||
|
|
||
| if (objectMatch) { | ||
| // Parse the object string to extract the JSON-like object | ||
| const objectContent = objectMatch[0] | ||
| .replace(/export const \w+ = /, '') | ||
| .replace(/;$/, '') | ||
|
|
||
| // Use Function constructor for safe evaluation | ||
| // The object content is valid JavaScript, so we can evaluate it | ||
| const tokenObject = new Function(`return ${objectContent}`)() as { | ||
| name: string | ||
| value: string | ||
| var: string | ||
| } | ||
|
|
||
| if ( | ||
| tokenObject && | ||
| typeof tokenObject === 'object' && | ||
| typeof tokenObject.name === 'string' && | ||
| typeof tokenObject.value === 'string' && | ||
| typeof tokenObject.var === 'string' | ||
| ) { | ||
| tokenObjects.push({ | ||
| name: tokenObject.name, | ||
| value: tokenObject.value, | ||
| var: tokenObject.var, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Prevent one bad token file from aborting the whole extraction
Any read/parse/eval error will reject Promise.all, which can fail API index generation. Wrap per‑file work in try/catch (or use Promise.allSettled) and continue, optionally logging via the project’s logger.
🔧 Proposed fix
await Promise.all(
matchingFiles.map(async (file) => {
- const filePath = join(tokensDir, file)
- const fileContent = await readFile(filePath, 'utf8')
+ try {
+ const filePath = join(tokensDir, file)
+ const fileContent = await readFile(filePath, 'utf8')
- // Extract the exported object using regex
- // Pattern: export const variableName = { "name": "...", "value": "...", "var": "..." };
- // Use non-greedy match to get just the first exported const object
- const objectMatch = fileContent.match(
- /export const \w+ = \{[\s\S]*?\n\};/,
- )
+ // Extract the exported object using regex
+ // Pattern: export const variableName = { "name": "...", "value": "...", "var": "..." };
+ // Use non-greedy match to get just the first exported const object
+ const objectMatch = fileContent.match(
+ /export const \w+ = \{[\s\S]*?\n\};/,
+ )
- if (objectMatch) {
- // Parse the object string to extract the JSON-like object
- const objectContent = objectMatch[0]
- .replace(/export const \w+ = /, '')
- .replace(/;$/, '')
+ if (objectMatch) {
+ // Parse the object string to extract the JSON-like object
+ const objectContent = objectMatch[0]
+ .replace(/export const \w+ = /, '')
+ .replace(/;$/, '')
- // Use Function constructor for safe evaluation
- // The object content is valid JavaScript, so we can evaluate it
- const tokenObject = new Function(`return ${objectContent}`)() as {
- name: string
- value: string
- var: string
- }
+ // Use Function constructor for safe evaluation
+ // The object content is valid JavaScript, so we can evaluate it
+ const tokenObject = new Function(`return ${objectContent}`)() as {
+ name: string
+ value: string
+ var: string
+ }
- if (
- tokenObject &&
- typeof tokenObject === 'object' &&
- typeof tokenObject.name === 'string' &&
- typeof tokenObject.value === 'string' &&
- typeof tokenObject.var === 'string'
- ) {
- tokenObjects.push({
- name: tokenObject.name,
- value: tokenObject.value,
- var: tokenObject.var,
- })
- }
- }
+ if (
+ tokenObject &&
+ typeof tokenObject === 'object' &&
+ typeof tokenObject.name === 'string' &&
+ typeof tokenObject.value === 'string' &&
+ typeof tokenObject.var === 'string'
+ ) {
+ tokenObjects.push({
+ name: tokenObject.name,
+ value: tokenObject.value,
+ var: tokenObject.var,
+ })
+ }
+ }
+ } catch (_error) {
+ // optionally log or collect diagnostics
+ }
}),
)🤖 Prompt for AI Agents
In `@src/utils/extractReactTokens.ts` around lines 67 - 105, Wrap the per-file
processing inside the matchingFiles.map async callback in a try/catch (or switch
to Promise.allSettled) so a single read/parse/eval error doesn't reject the
whole Promise.all; catch errors thrown by readFile, the regex match, or the new
Function(...)() evaluation (references: matchingFiles.map callback,
fileContent.match, new Function(...), and tokenObjects.push) and log the error
with the project logger before continuing to the next file. Ensure the catch
avoids rethrowing so token extraction proceeds for other files.
28 tasks
wise-king-sullyman
requested changes
Jan 26, 2026
| { | ||
| name: '--pf-v6-c-alert--BackgroundColor', | ||
| value: '#ffffff', | ||
| description: 'Alert background color', |
Contributor
There was a problem hiding this comment.
I'm not seeing a description entry in the responses, should this be the var field?
src/utils/extractReactTokens.ts
Outdated
|
|
||
| // Use Function constructor for safe evaluation | ||
| // The object content is valid JavaScript, so we can evaluate it | ||
| const tokenObject = new Function(`return ${objectContent}`)() as { |
Contributor
There was a problem hiding this comment.
This feels.... funky. Is this actually safe or is this effectively just running eval with extra steps?
Comment on lines
22
to
24
| if (cssTokens.length === 0) { | ||
| return createJsonResponse( | ||
| { | ||
| error: `No CSS tokens found for page '${page}' in section '${section}' for version '${version}'. CSS tokens are only available for content with a cssPrefix in the front matter.`, | ||
| }, | ||
| 404, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
If there are no cssTokens does it really make sense for us to 404 instead of just returning the empty array?
Contributor
Author
There was a problem hiding this comment.
yeah good point. Done.
src/utils/extractReactTokens.ts
Outdated
Comment on lines
30
to
37
| const tokensDir = join( | ||
| process.cwd(), | ||
| 'node_modules', | ||
| '@patternfly', | ||
| 'react-tokens', | ||
| 'dist', | ||
| 'esm', | ||
| ) |
Contributor
There was a problem hiding this comment.
Is this going to be reliable in monorepos?
src/utils/extractReactTokens.ts
Outdated
| }) | ||
|
|
||
| // Import and extract objects from each matching file | ||
| const tokenObjects: { name: string; value: string; var: string }[] = [] |
Contributor
There was a problem hiding this comment.
What about the values array?
src/utils/extractReactTokens.ts
Outdated
Comment on lines
30
to
108
| const tokensDir = join( | ||
| process.cwd(), | ||
| 'node_modules', | ||
| '@patternfly', | ||
| 'react-tokens', | ||
| 'dist', | ||
| 'esm', | ||
| ) | ||
|
|
||
| if (!existsSync(tokensDir)) { | ||
| return [] | ||
| } | ||
|
|
||
| // Get all files in the directory | ||
| const files = await readdir(tokensDir) | ||
|
|
||
| // Filter for .js files that match any of the token prefixes | ||
| // Exclude componentIndex.js and main component files (like c_accordion.js without underscores after the prefix) | ||
| const matchingFiles = files.filter((file) => { | ||
| if (!file.endsWith('.js') || file === 'componentIndex.js') { | ||
| return false | ||
| } | ||
| // Check if file starts with any of the token prefixes | ||
| // We want individual token files (e.g., c_accordion__toggle_FontFamily.js) | ||
| // but not the main component index file (e.g., c_accordion.js) | ||
| return tokenPrefixes.some((prefix) => { | ||
| if (file === `${prefix}.js`) { | ||
| // This is the main component file, skip it | ||
| return false | ||
| } | ||
| return file.startsWith(prefix) | ||
| }) | ||
| }) | ||
|
|
||
| // Import and extract objects from each matching file | ||
| const tokenObjects: { name: string; value: string; var: string }[] = [] | ||
|
|
||
| await Promise.all( | ||
| matchingFiles.map(async (file) => { | ||
| const filePath = join(tokensDir, file) | ||
| const fileContent = await readFile(filePath, 'utf8') | ||
|
|
||
| // Extract the exported object using regex | ||
| // Pattern: export const variableName = { "name": "...", "value": "...", "var": "..." }; | ||
| // Use non-greedy match to get just the first exported const object | ||
| const objectMatch = fileContent.match( | ||
| /export const \w+ = \{[\s\S]*?\n\};/, | ||
| ) | ||
|
|
||
| if (objectMatch) { | ||
| // Parse the object string to extract the JSON-like object | ||
| const objectContent = objectMatch[0] | ||
| .replace(/export const \w+ = /, '') | ||
| .replace(/;$/, '') | ||
|
|
||
| // Use Function constructor for safe evaluation | ||
| // The object content is valid JavaScript, so we can evaluate it | ||
| const tokenObject = new Function(`return ${objectContent}`)() as { | ||
| name: string | ||
| value: string | ||
| var: string | ||
| } | ||
|
|
||
| if ( | ||
| tokenObject && | ||
| typeof tokenObject === 'object' && | ||
| typeof tokenObject.name === 'string' && | ||
| typeof tokenObject.value === 'string' && | ||
| typeof tokenObject.var === 'string' | ||
| ) { | ||
| tokenObjects.push({ | ||
| name: tokenObject.name, | ||
| value: tokenObject.value, | ||
| var: tokenObject.var, | ||
| }) | ||
| } | ||
| } | ||
| }), | ||
| ) |
Contributor
There was a problem hiding this comment.
Actually I'm wondering.... could we not just import the componentIndex file to get the tokens?
src/utils/apiIndex/get.ts
Outdated
| page: string, | ||
| ): Promise<{ name: string; value: string; var: string }[]> { | ||
| const index = await getApiIndex() | ||
| const { createIndexKey } = await import('../apiHelpers') |
Contributor
There was a problem hiding this comment.
Why do this async import instead of just importing the function at the top of the file?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/apiIndex/get.ts`:
- Around line 31-33: The current check throws if parsed.css is missing or not an
object, which breaks consumers when older apiIndex.json lacks CSS; update the
validation in the parsed handling (the block that checks parsed.css) to
gracefully default parsed.css = {} when it's missing or not an object instead of
throwing, preserving backward compatibility for callers that don't rely on CSS;
ensure any downstream code that expects parsed.css remains typed/guarded to
accept an empty object.
Comment on lines
+31
to
+33
| if (!parsed.css || typeof parsed.css !== 'object') { | ||
| throw new Error('Invalid API index structure: missing or invalid "css" object') | ||
| } |
Contributor
There was a problem hiding this comment.
Consider a backward-compatible fallback for missing css.
If older apiIndex.json files can exist in dev/test, this hard-fail will break all API consumers even when they don’t need CSS. You could default to {} instead.
💡 Possible fallback
- if (!parsed.css || typeof parsed.css !== 'object') {
- throw new Error('Invalid API index structure: missing or invalid "css" object')
- }
+ if (!parsed.css || typeof parsed.css !== 'object') {
+ parsed.css = {}
+ }📝 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.
Suggested change
| if (!parsed.css || typeof parsed.css !== 'object') { | |
| throw new Error('Invalid API index structure: missing or invalid "css" object') | |
| } | |
| if (!parsed.css || typeof parsed.css !== 'object') { | |
| parsed.css = {} | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/apiIndex/get.ts` around lines 31 - 33, The current check throws if
parsed.css is missing or not an object, which breaks consumers when older
apiIndex.json lacks CSS; update the validation in the parsed handling (the block
that checks parsed.css) to gracefully default parsed.css = {} when it's missing
or not an object instead of throwing, preserving backward compatibility for
callers that don't rely on CSS; ensure any downstream code that expects
parsed.css remains typed/guarded to accept an empty object.
Added code to get and create the index at build time. Updated css.ts to retrieve code from apiIndex.json added unit tests for extracting the patternfly css.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…parsing for safer token extraction
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/apiIndex/generate.ts`:
- Around line 194-211: The code is incorrectly keying CSS tokens by section
(pageKey = `${version}::${section}`) and skips subsequent pages because of the
`!pageCssPrefixes[pageKey]` guard; change the key to include the page name (use
`${version}::${section}::${page}` or the existing page identifier) wherever
`pageCssPrefixes` and `pageCss` are set and read, remove the single-entry guard
so multiple pages per section are stored, and keep using
`extractReactTokens(cssPrefix)` to populate `pageCss[pageKey]` (and update the
loop that iterates Object.entries(pageCssPrefixes)) so tokens are stored and
looked up per page (symbols to change: pageCssPrefixes, pageCss, pageKey,
cssPrefix, extractReactTokens).
🧹 Nitpick comments (1)
src/utils/extractReactTokens.ts (1)
7-10: Consider parameterizing the version prefix for future multi-version support.The regex
^pf-v6-hardcodes the v6 version. If the documentation needs to support multiple PatternFly versions (v5, v6, etc.) with different CSS prefixes, this would require modification.For now this is acceptable since the current scope targets v6, but worth noting for future extensibility.
Comment on lines
194
to
211
|
|
||
| // Collect CSS prefixes for pages - we'll extract tokens later | ||
| if (entry.data.cssPrefix && !pageCssPrefixes[pageKey]) { | ||
| pageCssPrefixes[pageKey] = entry.data.cssPrefix | ||
| } | ||
| }) | ||
|
|
||
| // Extract CSS tokens for pages that have cssPrefix | ||
| for (const [pageKey, cssPrefix] of Object.entries(pageCssPrefixes)) { | ||
| try { | ||
| const tokens = await extractReactTokens(cssPrefix) | ||
| if (tokens.length > 0) { | ||
| pageCss[pageKey] = tokens | ||
| } | ||
| } catch (error) { | ||
| console.warn(`Failed to extract CSS tokens for ${pageKey}:`, error) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Critical: CSS tokens are keyed by section instead of page, causing data loss.
The code uses pageKey (${version}::${section}) but the interface specifies keys should be version::section::page (e.g., 'v6::components::accordion').
Current behavior:
- Only the first page's
cssPrefixper section is stored (due to!pageCssPrefixes[pageKey]check) - All subsequent pages in that section are silently ignored
- CSS tokens for most pages will be missing
The key should include the page name to store tokens per page, not per section.
🐛 Proposed fix
// Collect examples for this tab
const exampleKey = `${tabKey}::${tab}`
const examplesWithTitles = extractExamplesWithTitles(entry.body || '')
if (examplesWithTitles.length > 0) {
tabExamples[exampleKey] = examplesWithTitles
}
// Collect CSS prefixes for pages - we'll extract tokens later
- if (entry.data.cssPrefix && !pageCssPrefixes[pageKey]) {
- pageCssPrefixes[pageKey] = entry.data.cssPrefix
+ // Use tabKey (version::section::page) to key by page, not section
+ if (entry.data.cssPrefix && !pageCssPrefixes[tabKey]) {
+ pageCssPrefixes[tabKey] = entry.data.cssPrefix
}
})
// Extract CSS tokens for pages that have cssPrefix
- for (const [pageKey, cssPrefix] of Object.entries(pageCssPrefixes)) {
+ for (const [cssKey, cssPrefix] of Object.entries(pageCssPrefixes)) {
try {
const tokens = await extractReactTokens(cssPrefix)
if (tokens.length > 0) {
- pageCss[pageKey] = tokens
+ pageCss[cssKey] = tokens
}
} catch (error) {
- console.warn(`Failed to extract CSS tokens for ${pageKey}:`, error)
+ console.warn(`Failed to extract CSS tokens for ${cssKey}:`, error)
}
}📝 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.
Suggested change
| // Collect CSS prefixes for pages - we'll extract tokens later | |
| if (entry.data.cssPrefix && !pageCssPrefixes[pageKey]) { | |
| pageCssPrefixes[pageKey] = entry.data.cssPrefix | |
| } | |
| }) | |
| // Extract CSS tokens for pages that have cssPrefix | |
| for (const [pageKey, cssPrefix] of Object.entries(pageCssPrefixes)) { | |
| try { | |
| const tokens = await extractReactTokens(cssPrefix) | |
| if (tokens.length > 0) { | |
| pageCss[pageKey] = tokens | |
| } | |
| } catch (error) { | |
| console.warn(`Failed to extract CSS tokens for ${pageKey}:`, error) | |
| } | |
| } | |
| // Collect examples for this tab | |
| const exampleKey = `${tabKey}::${tab}` | |
| const examplesWithTitles = extractExamplesWithTitles(entry.body || '') | |
| if (examplesWithTitles.length > 0) { | |
| tabExamples[exampleKey] = examplesWithTitles | |
| } | |
| // Collect CSS prefixes for pages - we'll extract tokens later | |
| // Use tabKey (version::section::page) to key by page, not section | |
| if (entry.data.cssPrefix && !pageCssPrefixes[tabKey]) { | |
| pageCssPrefixes[tabKey] = entry.data.cssPrefix | |
| } | |
| }) | |
| // Extract CSS tokens for pages that have cssPrefix | |
| for (const [cssKey, cssPrefix] of Object.entries(pageCssPrefixes)) { | |
| try { | |
| const tokens = await extractReactTokens(cssPrefix) | |
| if (tokens.length > 0) { | |
| pageCss[cssKey] = tokens | |
| } | |
| } catch (error) { | |
| console.warn(`Failed to extract CSS tokens for ${cssKey}:`, error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/apiIndex/generate.ts` around lines 194 - 211, The code is
incorrectly keying CSS tokens by section (pageKey = `${version}::${section}`)
and skips subsequent pages because of the `!pageCssPrefixes[pageKey]` guard;
change the key to include the page name (use `${version}::${section}::${page}`
or the existing page identifier) wherever `pageCssPrefixes` and `pageCss` are
set and read, remove the single-entry guard so multiple pages per section are
stored, and keep using `extractReactTokens(cssPrefix)` to populate
`pageCss[pageKey]` (and update the loop that iterates
Object.entries(pageCssPrefixes)) so tokens are stored and looked up per page
(symbols to change: pageCssPrefixes, pageCss, pageKey, cssPrefix,
extractReactTokens).
wise-king-sullyman
approved these changes
Feb 4, 2026
|
🎉 This PR is included in version 1.20.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added CSS table support for the end point "/api/[version]/[section]/[page]/css". This closes issue #171 .
Summary by CodeRabbit
New Features
Behavior
Tests