docs: custom docs rendering instead of autodocs for API #756
Conversation
WalkthroughThis pull request involves multiple changes across the Changes
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/api/src/helpers/Highlight.tsx (1)
Line range hint
4-5: Review global Prism modification and top-level awaitThere are potential issues to address:
- Modifying the global object could cause conflicts
- Top-level await might not be supported in all environments
Consider refactoring to:
-await import("prismjs/components/prism-bash" as string); +(typeof global !== "undefined" ? global : window).Prism = Prism; + +// Move to an async initialization function +async function initializePrism() { + await import("prismjs/components/prism-bash"); +} + +// Call during component initialization +useEffect(() => { + initializePrism(); +}, []);
🧹 Nitpick comments (9)
packages/api/src/getting-started/Overview.mdx (1)
Line range hint
19-24: Consider enhancing image accessibility.While the image includes an alt text, it could be more descriptive to better convey the visual content to screen reader users.
<img className="my-0 ml-auto mr-auto h-52 w-52 object-cover sm:h-52 sm:w-52 md:h-full md:w-60" src={Hero} - alt="An illustration of web page rendered on both desktop and mobile devices" + alt="An illustration showing responsive web page layouts across desktop and mobile devices, demonstrating the UI library's adaptability" />packages/api/src/stories/ButtonIcon.stories.tsx (2)
12-14: Document the reason for excluding the spacing controlWhile excluding the spacing control is valid, it would be helpful to add a comment explaining why this control is being excluded from the docs.
parameters: { layout: "centered", docs: { + // Spacing control is excluded because... controls: { exclude: ["spacing"] }, }, },
23-29: Consider adding a description field to the storyThe implementation looks good, but adding a description field would improve the documentation quality.
export const Basic: Story = { + description: 'A basic example of ButtonIcon with Settings icon', render: (args) => ( <ButtonIcon {...args}> <IconSettings /> </ButtonIcon> ), };packages/api/src/stories/Button.stories.tsx (2)
23-31: Consider adding aria-label for better accessibilityWhile the implementation is good, consider adding descriptive aria-labels to differentiate between the buttons in screen readers.
export const Basic: Story = { render: (args) => ( <div className="flex flex-wrap gap-2"> - <Button {...args}>Button lorem ipsum</Button> - <Button {...args}>Button lorem ipsum dolor</Button> - <Button {...args}>Button lorem ipsum dolor sit amet</Button> + <Button {...args} aria-label="Short button">Button lorem ipsum</Button> + <Button {...args} aria-label="Medium button">Button lorem ipsum dolor</Button> + <Button {...args} aria-label="Long button">Button lorem ipsum dolor sit amet</Button> </div> ), };
33-52: Enhance code example accessibility and stylingThe implementation is good, but the code example could benefit from better semantic markup and styling.
<div className="pt-4"> <p> For text to truncate, you need to limit the width of the buttons. </p> <p>This can be done by using Tailwind width classes, for example</p> - <code>{args.className}</code> + <pre className="mt-2 rounded bg-gray-100 p-2"> + <code>{args.className}</code> + </pre> </div>packages/api/src/helpers/Usage.tsx (2)
5-7: Add JSDoc documentation for the UsageProps interfaceConsider adding documentation to describe the purpose and expected format of the codeBlock prop.
+/** + * Props for the Usage component + * @interface UsageProps + * @property {string} codeBlock - The code snippet to be displayed + */ interface UsageProps { codeBlock: string; }
9-17: Consider adding explicit return type and component documentationThe implementation looks good, but could benefit from additional type safety and documentation.
+/** + * Renders a code block with consistent styling using Storybook's Unstyled wrapper + * @param {UsageProps} props - The component props + * @returns {JSX.Element} A styled code block + */ -export default ({ codeBlock }: UsageProps) => { +export default ({ codeBlock }: UsageProps): JSX.Element => {packages/api/src/helpers/Title.tsx (2)
5-8: Add prop validation for required props.The props interface is well-defined, but since both props are required, consider adding runtime validation.
+import { type ReactElement } from "react"; interface TitleProps { label: string; level: "beta" | "stable" | "alpha"; } +const validateProps = ({ label, level }: TitleProps) => { + if (!label) { + throw new Error("Title: label prop is required"); + } + if (!level) { + throw new Error("Title: level prop is required"); + } +};
10-40: Consider memoizing the component for performance.The component implementation is clean, but since it's likely to be used across multiple documentation pages, it could benefit from memoization.
-export default ({ label, level }: TitleProps) => { +import { memo } from "react"; + +const Title = ({ label, level }: TitleProps): ReactElement => { + validateProps({ label, level }); // ... rest of the implementation -}; +}; + +export default memo(Title);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/api/.storybook/main.js(0 hunks)packages/api/.storybook/preview.jsx(1 hunks)packages/api/package.json(1 hunks)packages/api/src/getting-started/Changelogs.mdx(1 hunks)packages/api/src/getting-started/Configuration.mdx(1 hunks)packages/api/src/getting-started/Installation.mdx(1 hunks)packages/api/src/getting-started/Overview.mdx(1 hunks)packages/api/src/getting-started/ReleaseTags.mdx(1 hunks)packages/api/src/getting-started/Usage.mdx(1 hunks)packages/api/src/helpers/Highlight.tsx(1 hunks)packages/api/src/helpers/Title.tsx(1 hunks)packages/api/src/helpers/Usage.tsx(1 hunks)packages/api/src/stories/Button.mdx(1 hunks)packages/api/src/stories/Button.stories.tsx(1 hunks)packages/api/src/stories/ButtonIcon.mdx(1 hunks)packages/api/src/stories/ButtonIcon.stories.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/api/.storybook/main.js
✅ Files skipped from review due to trivial changes (4)
- packages/api/src/stories/Button.mdx
- packages/api/src/stories/ButtonIcon.mdx
- packages/api/src/getting-started/Installation.mdx
- packages/api/src/getting-started/Configuration.mdx
🔇 Additional comments (10)
packages/api/src/getting-started/Changelogs.mdx (1)
4-4: LGTM! Verify helper components restructuring
The import path change aligns with centralizing helper components at the parent directory level.
Let's verify the consistency of this restructuring:
✅ Verification successful
Import path change verified and consistent
The verification confirms:
- The Navigators component exists at
packages/api/src/helpers/Navigators.tsx - All MDX files in
getting-starteddirectory consistently import Navigators from the helpers directory - No remaining old-style imports using
./helpers/were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the helpers directory structure and its usage
# Check if helpers directory exists at the new location
fd -t d "helpers" packages/api/src
# Verify no remaining helpers directory in getting-started
fd -t d "helpers" packages/api/src/getting-started
# Check for any remaining old import paths that might need updating
rg -t mdx "from ['\"]./helpers/" packages/api/src/getting-started
Length of output: 209
Script:
#!/bin/bash
# Let's verify the import paths and Navigators component usage with more generic commands
# Check if Navigators component exists in the helpers directory
rg -l "Navigators" packages/api/src/helpers/
# Check all imports of Navigators across the codebase to verify consistency
rg -l "import.*Navigators.*from.*helpers" packages/api/src/
# Check for any potential old paths still using ./helpers
rg -l "from ['\"]./helpers/" packages/api/src/
Length of output: 484
packages/api/src/getting-started/Overview.mdx (1)
8-8: LGTM! Verifying the new helper component location.
The import path change aligns with the directory restructuring effort.
Let's verify the new location of the Navigators component:
✅ Verification successful
✅ Import path change is correct and consistent
The Navigators component exists at packages/api/src/helpers/Navigators.tsx and all MDX files in the getting-started directory consistently use the "../helpers/Navigators" import path. The change aligns perfectly with the codebase structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and consistency of the Navigators component
# in the new location
# Check if the Navigators component exists in the new location
fd -t f "Navigators\.(js|jsx|ts|tsx)" packages/api/src/helpers
# Verify consistent usage of the new path across other MDX files
rg -t mdx "import.*Navigators.*from.*helpers/Navigators" packages/api/src
Length of output: 208
Script:
#!/bin/bash
# Let's modify our search to handle MDX files and get a better view of the imports
# First, verify the Navigators component exists
ls -l packages/api/src/helpers/Navigators.tsx
# Then search for Navigators imports without file type restriction
rg "import.*Navigators.*from.*helpers/Navigators" packages/api/src
# Also search for any other potential Navigators import patterns
rg "import.*Navigators.*from" packages/api/src
Length of output: 1404
packages/api/src/stories/ButtonIcon.stories.tsx (1)
1-7: LGTM! Well-structured type definitions and imports
The changes follow Storybook's best practices for TypeScript integration. The Story type alias improves type safety and maintainability.
packages/api/src/stories/Button.stories.tsx (1)
8-20: LGTM! Good use of Storybook features
The meta configuration is well structured with:
- Proper control exclusions for documentation
- Mock handler for onClick using
fn()
packages/api/src/helpers/Highlight.tsx (1)
Line range hint 6-10: LGTM: Optional language parameter with default value
The change to make language optional with a default value of "jsx" is a good improvement that maintains backward compatibility while adding flexibility.
packages/api/src/getting-started/Usage.mdx (2)
Line range hint 1-35: LGTM: Well-structured documentation with examples
The documentation provides a good mix of code examples and visual demonstrations, which aligns well with the PR's objective of improving API documentation.
5-7: Verify import path consistency across documentation files
The import path changes look correct, but let's ensure consistency across all documentation files.
✅ Verification successful
Let me verify the actual import statements in these files to ensure they are all using the correct ../helpers/ path consistently.
Import paths are consistent across documentation files
All documentation files in the getting-started directory are correctly using the ../helpers/ relative path for their imports. Each file follows the same pattern and there are no remaining instances of ./helpers/ paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining old import paths and verify consistency
# Check for any remaining old helper import paths
rg -l "from ['\"]./helpers/" "packages/api/src/getting-started/"
# Verify all new helper import paths
rg -l "from ['\"]../helpers/" "packages/api/src/getting-started/"
Length of output: 421
Script:
#!/bin/bash
# Show the actual import statements from each file
rg "from ['\"].*helpers/" packages/api/src/getting-started/ -A 1
Length of output: 1971
packages/api/src/getting-started/ReleaseTags.mdx (1)
5-7: Verify helper components directory structure
Let's verify that the helper components are correctly located in the parent directory structure.
✅ Verification successful
Helper components directory structure is correct
All imported helper components exist at the expected location:
packages/api/src/helpers/Navigators.tsxpackages/api/src/helpers/Highlight.tsxpackages/api/src/helpers/codeBlocks.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and location of helper components
# Check if the helpers directory exists in the parent directory
fd -t d "helpers" packages/api/src
# Verify the existence of the imported helper components
fd -t f "Navigators|Highlight|codeBlocks" packages/api/src/helpers
Length of output: 247
packages/api/.storybook/preview.jsx (1)
Line range hint 20-28: LGTM! Control configuration aligns with custom docs approach.
The addition of expanded controls and matchers enhances the documentation experience, properly supporting the transition from autodocs to custom docs.
Let's verify the control matchers are being utilized:
✅ Verification successful
Let me analyze the results. The search shows extensive usage of args in story files, which is the standard way Storybook handles control props. Let me search specifically for color and date properties to verify if the matchers are being utilized.
Based on the search results, I can now generate the final response:
Control matchers configuration is properly utilized in the codebase.
The control matchers for color and date properties are being effectively used throughout the codebase:
- Color properties are extensively used in components with proper color tokens and theme support (e.g.,
background,colorproperties in UI components) - Date properties are used in Table components and system utilities (e.g.,
Datecolumn in tables, date formatting)
The expanded controls and matchers configuration aligns well with the component documentation needs and follows Storybook's best practices for control customization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for color and date properties in stories
rg -g '*.stories.*' -g '*.mdx' -A 3 'props|args'
Length of output: 59702
Script:
#!/bin/bash
# Search for color and date properties in component props and stories
rg -g '*.tsx' -g '*.jsx' -g '*.ts' -g '*.js' '(background|color|Date)' -B 2 -A 2
Length of output: 50231
packages/api/package.json (1)
7-7: Verify ESM compatibility across documentation.
The switch to ESM ("type": "module") is good for modernization, but ensure all documentation files and imports are compatible.
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information to verify ESM compatibility:
ESM configuration is properly set up across the codebase
The codebase already follows ESM import/export patterns consistently:
- All imports use ESM syntax (
import from) - No CommonJS
require()usage in source files .jsextensions are properly included in imports- Configuration files that need CommonJS are correctly using
.cjsextension
The only CommonJS usage is in configuration files (like postcss.config.cjs) which explicitly use the .cjs extension to maintain compatibility with tools that require CommonJS.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential CommonJS usage that needs updating
rg "require\(|module\.exports"
# Check for .js extensions in imports which are required in ESM
rg "from ['\"]\.[^'\"]+['\"]"
Length of output: 51799
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/api/src/helpers/AutoDoc.tsx (3)
7-13: Consider using a type alias for stage values.For better maintainability and reusability, consider extracting the stage values into a separate type alias.
+type Stage = "alpha" | "beta" | "stable"; + interface AutoDocProps { importName: string; header?: string; importPackage?: string; - stage?: "alpha" | "beta" | "stable"; + stage?: Stage; }
15-15: Name the component for better debugging.Consider naming the component for better debugging experience in React DevTools.
-export default ({ importName, importPackage, stage, header }: AutoDocProps) => { +const AutoDoc = ({ importName, importPackage, stage, header }: AutoDocProps) => { + +export default AutoDoc;
46-50: Remove redundant prose class and improve code organization.The
proseclassName is duplicated, and the Highlight component section could be better organized.- <div className="prose"> - <Highlight - codeBlock={`import { ${importName} } from "@versini/${packageName}";`} - /> - </div> + <Highlight + codeBlock={`import { ${importName} } from "@versini/${packageName}";`} + />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/api/src/helpers/AutoDoc.tsx(1 hunks)packages/api/src/stories/Button.mdx(1 hunks)packages/api/src/stories/ButtonIcon.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/api/src/stories/Button.mdx
- packages/api/src/stories/ButtonIcon.mdx
🔇 Additional comments (2)
packages/api/src/helpers/AutoDoc.tsx (2)
1-5: LGTM! Imports are well-organized.
The imports are properly structured, following the convention of external dependencies first, followed by internal imports.
33-56: 🛠️ Refactor suggestion
Consider adding prop validation for better error handling.
The component should validate the stage prop to ensure it receives valid values.
Let's verify if there are any PropTypes or validation patterns used in other components:
+const isValidStage = (stage?: string): stage is Stage => {
+ return stage === undefined || ["alpha", "beta", "stable"].includes(stage);
+};
const AutoDoc = ({ importName, importPackage, stage, header }: AutoDocProps) => {
+ if (stage && !isValidStage(stage)) {
+ console.warn(`Invalid stage value: ${stage}. Defaulting to "alpha"`);
+ }
const packageName = importPackage || `ui-${importName.toLowerCase()}`;
Bundle Size (components)
Overall bundle size: 84.72 KB (-12 B -0.01%) Bundle Size (fingerprint)
Overall bundle size: 48.28 KB (-1 B 0.00%) Bundle Size (form components)
Overall bundle size: 54.01 KB (-4 B -0.01%) Bundle Size (system)
Overall bundle size: 49.13 KB (+1 B 0.00%) |
aversini
deleted the
docs-custom-docs-rendering-instead-of-autodocs-for-API
branch
Summary by CodeRabbit
New Features
AutoDoccomponent for generating structured documentation for UI components.Bug Fixes
Refactor
Chores