-
-
Notifications
You must be signed in to change notification settings - Fork 464
Search for <ThemeInit />
in the project and warn if it's not found
#1605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 87ea3d8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning Rate limit exceeded@SutuSebastian has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis change introduces a conditional warning system for the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command (build/dev)
participant Logger as initLogger
participant FS as File System
CLI->>Logger: createInitLogger(config)
CLI->>FS: Find all relevant files
loop For each file
CLI->>FS: Read file content
CLI->>Logger: check(file, content)
end
CLI->>Logger: log()
alt No <ThemeInit /> found and custom config
Logger->>CLI: Warn about missing <ThemeInit />
else <ThemeInit /> found or default config
Logger->>CLI: No warning
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (8)
packages/ui/src/cli/utils/normalize-path.test.ts (1)
1-1
: Same comment as above – check for strayvitest
importspackages/ui/src/helpers/convert-utilities-to-v4.test.ts (1)
1-1
: Same comment as above – check for strayvitest
importspackages/ui/src/cli/utils/extract-component-imports.test.ts (1)
1-1
: Same comment as above – check for strayvitest
importspackages/ui/src/cli/utils/wrap-default-export.test.ts (1)
1-1
: Same comment as above – check for strayvitest
importspackages/ui/src/cli/utils/add-to-config.test.ts (1)
1-1
: Same Bun-types requirement applies here – see previous comment.packages/ui/src/cli/utils/add-import.test.ts (1)
1-1
: Same Bun-types requirement applies here – see previous comment.packages/ui/src/cli/utils/compare-nodes.test.ts (1)
1-1
: Same Bun-types requirement applies here – see previous comment.packages/ui/src/cli/utils/add-plugin.test.ts (1)
1-1
: Same Bun-types requirement applies here – see previous comment.
🧹 Nitpick comments (4)
packages/ui/src/helpers/resolve-theme.test.ts (1)
16-18
: Use of@ts-expect-error
is acceptable but consider explicit castingThe directive suppresses a type-mismatch just for this line.
Alternatively, a one-off cast keeps tests type-safe without disabling TS checking:expect( resolveTheme([base, custom], []) as unknown ).toEqual({ color: "text-red-400", background: "text-blue-400", });Not blocking, just a style consideration.
packages/ui/src/cli/commands/setup-config.ts (1)
15-97
: Consider the impact of removing informational logging.The refactoring removed the code that logged informational messages when config properties (
dark
,prefix
,version
) differed from defaults. While this reduces verbosity, it might also reduce transparency for users who want to understand why their configuration was modified.Consider whether some level of informational logging should be retained to help users understand configuration changes, especially for critical properties like
version
detection.packages/ui/src/cli/commands/build.ts (1)
24-34
: Logic flow could be simplified for consistencyThe conditional file scanning based on
isCustomConfig
creates different execution paths. Consider extracting the file scanning logic to reduce duplication with the else branch.- if (initLogger.isCustomConfig) { - const files = await findFiles({ - patterns: allowedExtensions.map((ext) => `**/*${ext}`), - excludeDirs, - }); - - for (const file of files) { - const content = await fs.readFile(file, "utf-8"); - initLogger.check(file, content); - } - }Consider moving this logic to be consistent with the else branch pattern, or extract common file scanning into a helper function.
packages/ui/src/cli/utils/create-init-logger.ts (1)
22-24
: Consider performance optimization for showWarningThe current implementation uses
find()
to check if any value istrue
, butArray.from(this.checkedMap.values()).includes(true)
or using a simple loop might be more readable and potentially more performant.-get showWarning() { - return this.checkedMap.values().find((value) => value) === undefined; -}, +get showWarning() { + return !Array.from(this.checkedMap.values()).includes(true); +},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.changeset/dull-ducks-give.md
(1 hunks)packages/ui/package.json
(1 hunks)packages/ui/scripts/generate-metadata.test.ts
(1 hunks)packages/ui/src/cli/commands/build.ts
(3 hunks)packages/ui/src/cli/commands/dev.ts
(5 hunks)packages/ui/src/cli/commands/setup-config.ts
(1 hunks)packages/ui/src/cli/commands/setup-init.ts
(1 hunks)packages/ui/src/cli/utils/add-import.test.ts
(1 hunks)packages/ui/src/cli/utils/add-plugin.test.ts
(1 hunks)packages/ui/src/cli/utils/add-to-config.test.ts
(1 hunks)packages/ui/src/cli/utils/compare-nodes.test.ts
(1 hunks)packages/ui/src/cli/utils/create-config.ts
(1 hunks)packages/ui/src/cli/utils/create-init-logger.test.ts
(1 hunks)packages/ui/src/cli/utils/create-init-logger.ts
(1 hunks)packages/ui/src/cli/utils/extract-component-imports.test.ts
(1 hunks)packages/ui/src/cli/utils/get-config.ts
(1 hunks)packages/ui/src/cli/utils/normalize-path.test.ts
(1 hunks)packages/ui/src/cli/utils/update-build-config.test.ts
(1 hunks)packages/ui/src/cli/utils/wrap-default-export.test.ts
(1 hunks)packages/ui/src/helpers/apply-prefix-v3.test.ts
(1 hunks)packages/ui/src/helpers/apply-prefix.test.ts
(1 hunks)packages/ui/src/helpers/convert-utilities-to-v4.test.ts
(1 hunks)packages/ui/src/helpers/get.test.ts
(1 hunks)packages/ui/src/helpers/is-equal.test.ts
(1 hunks)packages/ui/src/helpers/merge-refs.test.ts
(3 hunks)packages/ui/src/helpers/resolve-props.test.ts
(2 hunks)packages/ui/src/helpers/resolve-theme.test.ts
(2 hunks)packages/ui/src/helpers/strip-dark.test.ts
(1 hunks)packages/ui/src/helpers/without-theming-props.test.ts
(1 hunks)packages/ui/vitest.config.js
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/ui/src/cli/commands/setup-config.ts (1)
Learnt from: SutuSebastian
PR: #1566
File: packages/ui/src/cli/commands/setup-init.ts:41-0
Timestamp: 2025-07-21T18:57:15.352Z
Learning: In CLI tools that generate files, using setTimeout with a small delay (like 10ms) before fs.writeFile operations can be intentional debouncing to prevent race conditions with IDE formatting extensions (Prettier, ESLint auto-fix, etc.) that automatically format files upon creation/modification. This ensures the file is visible to the IDE and formatting completes before proceeding with subsequent operations.
🧬 Code Graph Analysis (5)
packages/ui/src/cli/commands/build.ts (2)
packages/ui/src/cli/utils/create-init-logger.ts (1)
createInitLogger
(9-49)packages/ui/src/cli/utils/find-files.ts (1)
findFiles
(26-94)
packages/ui/src/cli/commands/setup-config.ts (2)
packages/ui/src/cli/utils/create-config.ts (2)
Config
(1-10)createConfig
(12-24)packages/ui/src/cli/utils/get-tailwind-version.ts (1)
getTailwindVersion
(11-24)
packages/ui/src/cli/utils/create-init-logger.test.ts (1)
packages/ui/src/cli/utils/create-init-logger.ts (1)
hasThemeInit
(57-74)
packages/ui/src/cli/commands/dev.ts (5)
packages/ui/src/cli/utils/create-init-logger.ts (1)
createInitLogger
(9-49)packages/ui/src/cli/consts.ts (4)
configFilePath
(15-15)initFilePath
(17-17)initJsxFilePath
(18-18)gitIgnoreFilePath
(16-16)packages/ui/src/cli/utils/get-config.ts (1)
getConfig
(14-16)packages/ui/src/cli/commands/setup-init.ts (1)
setupInit
(14-72)packages/ui/src/cli/commands/setup-gitignore.ts (1)
setupGitIgnore
(10-28)
packages/ui/src/cli/utils/create-init-logger.ts (2)
packages/ui/src/cli/utils/create-config.ts (2)
Config
(1-10)createConfig
(12-24)packages/ui/src/cli/consts.ts (3)
configFilePath
(15-15)initFilePath
(17-17)initJsxFilePath
(18-18)
⏰ 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 (35)
packages/ui/src/helpers/without-theming-props.test.ts (1)
1-1
: Remove lingering Vitest imports and configThe migration to Bun’s built-in runner is incomplete: numerous test files and a Vitest config remain in the
packages/ui
package.• Test files still importing from Vitest (e.g.
packages/ui/src/components/.../*.test.tsx
):- import { describe, expect, it } from "vitest"; + import { describe, expect, it } from "bun:test";Replace all
import … from "vitest"
(and anyvi
uses) withbun:test
equivalents.
• Remove or replace the Vitest configuration:
- Delete
packages/ui/vitest.config.js
and itsvitest-setup.ts
• Update CI workflows to run only Bun tests and remove any Vitest stepsLikely an incorrect or invalid review comment.
packages/ui/src/helpers/apply-prefix-v3.test.ts (1)
1-1
: Ensure Bun typings are available for TypeScript toolingSwitching the test runner import to
bun:test
is correct, but TypeScript will complain unless Bun’s ambient declarations are in scope.
Confirm that the workspace has either:
bun-types
(or an equivalent) listed indevDependencies
, and"types": ["bun-types"]
(or"bun-types"
intypesVersions
) in the roottsconfig.json
.Otherwise
describe/it/expect
will appear asany
.packages/ui/src/cli/utils/update-build-config.test.ts (1)
1-1
: Bun test import looks goodSwapping the test utilities to
bun:test
is coherent with the repo-wide migration; the rest of the test stays untouched.
No further action needed.packages/ui/src/helpers/is-equal.test.ts (1)
1-1
: Consistent framework migration – all goodImport updated to
bun:test
, test body unchanged. Implementation remains accurate.packages/ui/scripts/generate-metadata.test.ts (1)
1-1
: Import switch is fineMove to Bun’s test runner is correctly applied; no regressions spotted.
packages/ui/src/helpers/resolve-theme.test.ts (1)
1-1
: Bun import applied correctlyTests compile and run under Bun; change is straightforward.
packages/ui/src/helpers/resolve-props.test.ts (2)
1-1
: LGTM! Testing framework migration correctly implemented.The migration from Vitest to Bun testing framework is properly executed. The import statement and mock syntax have been correctly updated to use Bun's testing API.
Also applies to: 5-7
29-29
: Appropriate use of @ts-expect-error directive.The TypeScript expect-error comment is correctly used to bypass type checking for the test property in the expected result, which is appropriate for test scenarios.
packages/ui/src/helpers/merge-refs.test.ts (2)
1-2
: Well-executed testing framework migration with improved typing.The migration to Bun testing framework is correctly implemented, and the addition of explicit RefObject typing improves type safety.
19-19
: Correct mock function migration.The mock function syntax has been properly updated from Vitest's
vi.fn()
to Bun'smock()
API.Also applies to: 30-30
packages/ui/src/cli/commands/setup-init.ts (1)
6-6
: Configuration refactoring looks good.The import path change to use the centralized
create-config
utility module improves code organization and follows good practices for shared configuration types.packages/ui/src/helpers/strip-dark.test.ts (1)
1-1
: Clean testing framework migration.The import change from Vitest to Bun testing framework is correctly implemented with no impact on test logic.
packages/ui/src/helpers/get.test.ts (1)
1-1
: Proper testing framework migration.The import statement has been correctly updated to use Bun's testing framework while preserving all existing test functionality.
packages/ui/src/helpers/apply-prefix.test.ts (1)
1-1
: LGTM! Testing framework migration looks good.The import change from Vitest to Bun's test framework is consistent with the broader testing strategy update across the codebase. All test logic remains unchanged and should work seamlessly with Bun's test runner.
packages/ui/src/cli/utils/get-config.ts (1)
2-2
: LGTM! Import path update aligns with configuration refactoring.The import change reflects the centralization of the
Config
interface to the newcreate-config
utility module, which improves code organization and reusability..changeset/dull-ducks-give.md (1)
1-6
: LGTM! Changeset accurately documents the enhancement.The changeset correctly identifies this as a patch-level change and provides a clear description of the improvement to the
<ThemeInit />
warning system. The format follows standard changeset conventions.packages/ui/package.json (1)
260-260
: LGTM! Test script expansion supports the testing framework migration.The updated test script appropriately expands the scope of
bun test
to include thescripts
,src/cli
, andsrc/helpers
directories while maintaining Vitest for other tests. This supports the coordinated migration to Bun's test framework for specific parts of the codebase.packages/ui/src/cli/commands/setup-config.ts (2)
6-6
: LGTM! Configuration refactoring improves code organization.The import of
Config
type andcreateConfig
function from the new utility module centralizes configuration logic and improves code reusability across the CLI commands.
15-17
: LGTM! UsingcreateConfig
function improves consistency.The refactored approach using
createConfig
with the detected Tailwind version ensures consistent default configuration creation across the codebase.packages/ui/src/cli/commands/build.ts (3)
17-17
: LGTM: Clean integration of init loggerThe initialization of
initLogger
with the config is well-placed and follows the established pattern.
44-44
: Ensure consistent ThemeInit checkingGood placement of
initLogger.check()
call within the file processing loop.
52-52
: Appropriate placement of warning logThe final
initLogger.log()
call is correctly positioned after all file processing is complete.packages/ui/src/cli/utils/create-init-logger.test.ts (4)
1-3
: Good test framework usageCorrect usage of
bun:test
framework and proper import structure.
17-19
: Excellent edge case coverageThe test correctly verifies that
<ThemeInit>
with content is not detected, which aligns with the requirement for self-closing or empty tags only.
34-38
: Comprehensive comment handling testsGood coverage of different comment formats that should be excluded from detection.
146-151
: Thorough malformed tag validationExcellent test cases for malformed tags that should not be detected. This prevents false positives.
packages/ui/src/cli/utils/create-config.ts (2)
1-10
: Well-defined Config interfaceThe interface properly captures all necessary configuration properties with appropriate types. The version constraint to
3 | 4
is particularly good for type safety.
12-24
: Clean default configuration implementationThe
createConfig
function properly uses nullish coalescing and provides sensible defaults. The TODO comment for prefix inference is a good reminder for future enhancement.packages/ui/src/cli/commands/dev.ts (5)
16-16
: Consistent logger integrationGood addition of the
createInitLogger
import to match the build command.
29-29
: Proper logger initializationThe
initLogger
is correctly initialized with the config after setup.
47-47
: Appropriate initial file checkingThe
initLogger.check()
call is properly placed during the initial file scan.
71-78
: Improved config change handlingThe reorganization of the change handler to reload config and update the logger when config files change is a good improvement. This ensures the logger uses the latest configuration.
93-93
: Good memory managementProperly cleaning up the
checkedMap
when files are unlinked prevents memory leaks.packages/ui/src/cli/utils/create-init-logger.ts (2)
57-74
: Robust regex implementation for ThemeInit detectionThe
hasThemeInit
function has excellent coverage of edge cases:
- Properly excludes commented instances
- Handles malformed tags correctly
- Supports both self-closing and empty tag formats
- Allows optional attributes
The regex logic is complex but well-documented and thoroughly tested.
41-46
: Clear and actionable warning messageThe warning message is informative and provides specific guidance on where to place the
<ThemeInit />
component and why it's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/dull-ducks-give.md (2)
5-5
: Consider rephrasing the summary for concisenessChangeset titles are often written in the imperative mood and kept short.
Example: “Warn only when<ThemeInit />
is missing during build/dev”.This keeps the first-line changelog entry tight while conveying the same meaning.
9-10
: Grammar nit: add “to” in the bullet“switch tests
src/cli
src/helpers
fromvitest
→bun:test
”
→ “switch tests insrc/cli
andsrc/helpers
fromvitest
tobun:test
”Also consider back-ticking
bun:test
consistently (bun test
if that’s the CLI).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/dull-ducks-give.md
(1 hunks)
⏰ 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
Description
Search for
<ThemeInit />
in the project and warn if it's not found instead of warning all the timeIssue
#1604
Changes
build
anddev
check files content for custom configuration and display a warning if<ThemeInit />
is not foundsrc/cli
andsrc/helpers
fromvitest
->bun:test
Summary by CodeRabbit
Summary by CodeRabbit
New Features
<ThemeInit />
component, so warnings are only shown if the component is not present when needed.<ThemeInit />
usage in projects with custom theme configurations.Chores
Documentation