Improvements and chores - 5 Aug 2025 [OFFICIAL TS SUPPORT] #1300
Conversation
WalkthroughThis change simplifies the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
78-81: Inconsistent list formatting in “Credits and references” sectionLine 80 reintroduces plain text after a numbered list item (Line 78), breaking Markdown list continuity and rendering unpredictably on some platforms. Align it with the list style and add a link for clarity.
-This project is tested with BrowserStack +2. This project is tested with [BrowserStack](https://www.browserstack.com/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.husky/pre-commit(0 hunks)README.md(2 hunks)
💤 Files with no reviewable changes (1)
- .husky/pre-commit
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: in the `rad-ui/ui` repository, the `security-fixes` branch only updates dependencies, so linting and...
Learnt from: kotAPI
PR: rad-ui/ui#640
File: .github/workflows/build-rad-ui.yml:5-5
Timestamp: 2024-12-14T02:25:41.034Z
Learning: In the `rad-ui/ui` repository, the `security-fixes` branch only updates dependencies, so linting and Chromatic workflows are not required for this branch.
Applied to files:
README.md
📚 Learning: the accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props l...
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
Applied to files:
README.md
⏰ 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: Analyze (javascript)
🔇 Additional comments (1)
README.md (1)
63-65: Extraneous blank line removed—looks goodRemoving the superfluous blank line tightens the markdown section without affecting readability. No further action needed.
kotAPI
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
rollup.config.cjs (1)
70-82: LGTM! Well-configured TypeScript declaration builds.The DTS bundle configuration correctly mirrors the JS build structure and properly handles external dependencies.
Consider adding
react/jsx-runtimeto the external dependencies list for consistency with the JS bundles:- external: ['react', 'react-dom'] + external: ['react', 'react-dom', 'react/jsx-runtime']
📜 Review details
Configuration used: CodeRabbit 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)
package.json(4 hunks)rollup.config.cjs(4 hunks)scripts/generate-exports.cjs(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tsconfig.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: GoldGroove06
PR: rad-ui/ui#1214
File: package.json:64-64
Timestamp: 2025-07-14T18:13:36.606Z
Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script.
📚 Learning: in npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their co...
Learnt from: GoldGroove06
PR: rad-ui/ui#1214
File: package.json:64-64
Timestamp: 2025-07-14T18:13:36.606Z
Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script.
Applied to files:
scripts/generate-exports.cjspackage.json
⏰ 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
🔇 Additional comments (11)
scripts/generate-exports.cjs (1)
1-38: Excellent documentation!The comprehensive comment block clearly explains the script's purpose, benefits, and output format. This will greatly help future maintainers understand the automation.
package.json (5)
7-7: LGTM! Exports field prepared for dynamic generation.The empty exports object correctly serves as a placeholder that will be populated by the
generate-exports.cjsscript during the build process.
21-21: LGTM! Updated to handle TypeScript declaration files.The script correctly copies both
.jsand.d.tsfiles, supporting the new build process that generates type declarations.
23-23: LGTM! Export generation script properly integrated.The new script is well-named and follows the established naming convention for build-related tasks.
26-26: LGTM! Export generation correctly integrated into build pipeline.The export generation step is properly placed after component compilation, ensuring the exports are generated from the built artifacts.
102-102: rollup-plugin-dts version verifiedConfirmed that
^6.2.1is the latest published release as of March 2025. No changes needed.rollup.config.cjs (5)
9-9: LGTM! Correct import for TypeScript declaration generation.The destructured import efficiently imports only the needed
dtsfunction from the plugin.
27-27: LGTM! Good documentation improvement.The comment clearly explains the purpose of the shared plugin instances section, improving code organization.
44-44: LGTM! Clear separation of JS and TS builds.The explicit
jsBundlesnaming improves code clarity and prepares for the dual-build architecture.Also applies to: 68-68
49-49: LGTM! Cleaner warning suppression logic.The simplified conditional maintains the same functionality while being more concise.
84-84: LGTM! Proper combination of build configurations.The spread operator correctly combines both JS and TypeScript declaration builds into a single configuration array for Rollup.
| exportsMap['./themes/default.css'] = './dist/themes/default.css'; | ||
| exportsMap['./themes/tailwind-presets/default.js'] = './dist/themes/tailwind-presets/default.js'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify hardcoded theme file paths exist.
Ensure these theme files are actually generated during the build process.
🏁 Script executed:
#!/bin/bash
# Check if the hardcoded theme files exist in the expected locations
echo "Checking for theme files..."
if [ -f "dist/themes/default.css" ]; then
echo "✅ dist/themes/default.css exists"
else
echo "❌ dist/themes/default.css not found"
fi
if [ -f "dist/themes/tailwind-presets/default.js" ]; then
echo "✅ dist/themes/tailwind-presets/default.js exists"
else
echo "❌ dist/themes/tailwind-presets/default.js not found"
fiLength of output: 348
Missing generated theme files in exports map
The hardcoded entries in scripts/generate-exports.cjs reference dist/themes/default.css and dist/themes/tailwind-presets/default.js, but neither file exists after the build. Please either:
- Update the build to emit these files at the expected paths
- Or adjust the exportsMap keys/values to match the actual output filenames
Files to fix:
- scripts/generate-exports.cjs (around lines 50–51)
🤖 Prompt for AI Agents
In scripts/generate-exports.cjs around lines 50 to 51, the exportsMap references
files that do not exist after the build. To fix this, verify the actual output
filenames and paths generated by the build process and update the exportsMap
keys and values accordingly to match those real output files, or modify the
build configuration to emit the missing files at the expected paths.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… into kotapi/5-aug-maintenance
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/process-components.cjs (1)
39-59: Eliminate code duplication in file processing loops.The two loops for
.jsand.d.tsfiles are nearly identical and can be consolidated to improve maintainability.Apply this diff to consolidate the loops:
-// Copy .js files -files.forEach(file => { - if (file.endsWith('.js')) { - const sourcePath = path.join(tempCleanupPath, file); - const destPath = path.join(componentsPath, file); - fs.copyFileSync(sourcePath, destPath); - copiedFiles++; - console.log(`📄 Copied: ${file}`); - } -}); - -// Copy .d.ts files -files.forEach(file => { - if (file.endsWith('.d.ts')) { - const sourcePath = path.join(tempCleanupPath, file); - const destPath = path.join(componentsPath, file); - fs.copyFileSync(sourcePath, destPath); - copiedFiles++; - console.log(`📄 Copied: ${file}`); - } -}); +// Copy .js and .d.ts files +files.forEach(file => { + if (file.endsWith('.js') || file.endsWith('.d.ts')) { + const sourcePath = path.join(tempCleanupPath, file); + const destPath = path.join(componentsPath, file); + fs.copyFileSync(sourcePath, destPath); + copiedFiles++; + console.log(`📄 Copied: ${file}`); + } +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(4 hunks)scripts/process-components.cjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: GoldGroove06
PR: rad-ui/ui#1214
File: package.json:64-64
Timestamp: 2025-07-14T18:13:36.606Z
Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script.
📚 Learning: in npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their co...
Learnt from: GoldGroove06
PR: rad-ui/ui#1214
File: package.json:64-64
Timestamp: 2025-07-14T18:13:36.606Z
Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script.
Applied to files:
scripts/process-components.cjspackage.json
⏰ 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
🔇 Additional comments (8)
scripts/process-components.cjs (3)
1-15: Excellent documentation and clear rationale.The comprehensive documentation clearly explains the purpose, benefits, and rationale for this approach over shell commands. This will help future maintainers understand the design decisions.
17-33: Good path resolution and error handling.The script properly resolves paths relative to the script location and handles missing directories gracefully with informative logging.
61-67: Proper cleanup and informative logging.The cleanup process uses appropriate options (
recursive: true, force: true) and provides helpful summary information for build processes.package.json (5)
21-21: Good migration from shell to Node.js script.The change from shell command to Node.js script improves cross-platform compatibility and error handling, as documented in the
process-components.cjsfile.
23-23: New script for dynamic export generation.The addition of
build:generate-exportssupports the dynamic generation of the exports field, which should improve maintainability over static definitions.
26-26: Proper integration of export generation step.The build:rollup script correctly includes the new
build:generate-exportsstep to ensure the exports field is populated after component processing.
102-102: rollup-plugin-dts version is up to dateThe devDependency
"rollup-plugin-dts": "^6.2.1"uses the latest stable release (6.2.1) for bundling TypeScript declaration files, so no changes are needed.
7-7: Exports Automation VerifiedThe
scripts/generate-exports.cjsscript exists, is referenced by thebuild:generate-exportstask in package.json, and callsfs.writeFileSynconpackage.jsonto populate theexportsfield. It’s also chained into thebuild:rollupscript, ensuring exports are regenerated on each build. No additional changes are needed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
Chores
Documentation