Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant User as User
participant CLI as CLI (commander)
participant Apply as Apply Command
participant Preflight as preFlightApply
participant FS as Filesystem
participant Preset as Preset Resolver
participant Init as runInit
User->>CLI: run `shadcn-vue apply [preset]`
CLI->>Apply: dispatch handler
Apply->>Apply: validate args (Zod)
Apply->>Preset: resolve preset (named / encoded / URL)
Apply->>Preflight: preFlightApply(cwd)
Preflight->>FS: check package.json & components.json
Preflight-->>Apply: config or errors
Apply->>Apply: derive base & RTL from config
Apply->>Apply: enumerate components (getProjectComponents)
Apply->>User: prompt for confirmation (unless --yes)
User-->>Apply: confirm
Apply->>FS: copy components.json -> components.json.apply.bak
Apply->>Init: runInit(inPlace=false, reinstall=true, yes=true, skipPreflight=true, preset/url, base, rtl)
Init->>FS: write files, update components
Init-->>Apply: success / failure
alt success
Apply->>FS: delete .apply.bak
Apply->>User: log success
else failure
Apply->>FS: restore components.json from .apply.bak
Apply->>User: log error
end
Apply->>Apply: clearRegistryContext (finally)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/src/commands/apply.test.ts (1)
68-221: Add one action-level test for the failure/rollback path.This suite only exercises helper exports. The new command flow itself—preflight, backup creation, restore on failure, and cleanup—still isn't covered. A mocked
runInit()failure test would lock down the risky path inapply()much better than helper-only coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/apply.test.ts` around lines 68 - 221, Add an action-level test that calls the top-level apply() flow and simulates a failing runInit() to exercise preflight, backup, rollback and cleanup: mock runInit() to throw, spy on preflight(), createBackup()/backup creation, restoreBackup() and cleanupBackup() (or their actual names in the module) and assert that createBackup() was called before the failure, restoreBackup() was invoked after the failure, cleanupBackup() ran, and apply() surfaces the error (or exits) as expected; ensure the test imports apply() from the CLI command module and restores all spies/mocks after the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/apply.ts`:
- Around line 153-185: The rollback never runs because runInit() funnels errors
into handleError(), which exits the process instead of throwing, so apply()
never reaches the catch block to restore applyBackupPath; fix by changing
runInit()/handleError() so errors are surfaced: add an option (e.g.,
exitOnError: boolean) to runInit and thread it to handleError, make handleError
throw the error instead of calling process.exit when exitOnError is false, and
call runInit({ ..., exitOnError: false }) from the apply command so a thrown
error reaches the catch that restores componentsJsonPath from applyBackupPath;
reference runInit, handleError, applyBackupPath and componentsJsonPath when
making the change.
In `@packages/cli/src/preflights/preflight-apply.ts`:
- Around line 30-36: The code currently force-asserts config with `config!`
after `const config = await getConfig(options.cwd)` even though `getConfig()`
can return null for an empty components.json; update preflight handling in
`preflight-apply.ts` to treat `!config` the same as the invalid-config branch:
do not use the non-null assertion, and when `config` is null return the `{
errors, config: null }` response but populate `errors` (or add a specific
invalid-config error key) so callers (like the logic in `apply.ts`) receive an
explicit invalid-config error instead of a silent exit — modify the return path
around the `config` value and ensure any downstream checks rely on the error
flag rather than a non-null assertion.
---
Nitpick comments:
In `@packages/cli/src/commands/apply.test.ts`:
- Around line 68-221: Add an action-level test that calls the top-level apply()
flow and simulates a failing runInit() to exercise preflight, backup, rollback
and cleanup: mock runInit() to throw, spy on preflight(), createBackup()/backup
creation, restoreBackup() and cleanupBackup() (or their actual names in the
module) and assert that createBackup() was called before the failure,
restoreBackup() was invoked after the failure, cleanupBackup() ran, and apply()
surfaces the error (or exits) as expected; ensure the test imports apply() from
the CLI command module and restores all spies/mocks after the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed1c65af-2bba-452d-ac2e-ea8dc2759daf
📒 Files selected for processing (6)
apps/v4/content/docs/06.cli.mdpackages/cli/src/commands/apply.test.tspackages/cli/src/commands/apply.tspackages/cli/src/index.tspackages/cli/src/preflights/preflight-apply.tspackages/cli/src/utils/get-project-info.ts
zernonia
force-pushed
the
feat-apply-preset
branch
from
d771bfb to
53ba6bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/cli/src/commands/apply.ts (1)
181-217:⚠️ Potential issue | 🔴 CriticalSurface init failures back to this catch before relying on rollback.
The backup is only restored if
runInit()throws into this block. IfrunInit()still handles its own failures viahandleError()/process.exit(), Lines 212-215 never run and.apply.bakbecomes the only copy of the originalcomponents.json.Please verify that
runInit()can return control here on failure. Expected result: its error path should rethrow (or support a non-exiting mode) instead of terminating the process.#!/bin/bash set -euo pipefail echo '--- init/apply error flow ---' rg -n -C4 'export async function runInit|catch \(|handleError\(|process\.exit\(' \ packages/cli/src/commands/init.ts \ packages/cli/src/utils/handle-error.ts \ packages/cli/src/commands/apply.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/apply.ts` around lines 181 - 217, runInit currently may terminate the process via handleError/process.exit which prevents the apply catch from running and restoring .apply.bak; change runInit (and/or handleError) to surface failures instead of exiting by either: 1) adding a non-exiting mode flag (e.g., runInit({ ..., abortOnError: false } or similar) so runInit will throw errors instead of calling process.exit, or 2) making handleError rethrow when invoked from runInit in non-interactive flows; then update the runInit invocation in apply.ts to pass the non-exiting flag (or call the throwing variant) so failures propagate to the surrounding catch and trigger the rollback code that copies applyBackupPath back to componentsJsonPath. Ensure any helper names referenced are runInit and handleError/process.exit and that tests or callers expecting process exit are updated accordingly.
🧹 Nitpick comments (1)
packages/cli/src/commands/apply.test.ts (1)
24-187: Please add an action-level test forapply().This suite only covers the exported helpers; it never exercises the command's
.action()body. That leaves the missing-config preflight branch and the.apply.bakrestore path unprotected even though they're the risky parts of this feature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/apply.test.ts` around lines 24 - 187, The test suite never invokes the command's action body, so add an action-level test that imports the exported apply command (the exported object/function named apply) and calls its .action(...) to exercise the command flow; set up a temporary cwd and fixtures to simulate missing config (to trigger the preflight/missing-config branch) and create a fake .apply.bak file to verify the restore path, mocking/stubbing filesystem ops (fs.rename/fs.exists/fs.unlink or the helper functions used in apply), network/URL resolution if needed (resolveApplyInitUrl), and process.exit behavior to assert the expected outcomes; ensure the test cleans up mocks and temp files and asserts that restore logic was executed and that the missing-config branch exits/returns as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/apply.ts`:
- Around line 18-20: The branch in the apply command is checking the wrong error
key (MISSING_CONFIG) so the friendly guidance from preFlightApply() never runs;
change the conditional(s) that compare the error key(s) to use
ERRORS.MISSING_CONFIG_FILE (the same constant used in
preflights/preflight-apply.ts) instead of MISSING_CONFIG so the components.json
missing flow triggers the guidance and avoids silently falling through to
process.exit(1); update every occurrence in the apply command (including the
block around the second occurrence referenced) to use
ERRORS.MISSING_CONFIG_FILE.
- Around line 249-258: The code uses JSON.stringify in quoteShellArg which
produces double-quoted args that allow shell variable expansion; update
quoteShellArg to produce safe POSIX single-quoted arguments (wrap the value in
single quotes and escape any internal single quotes using the '\'' pattern) so
getInitCommand returns a shell-safe string for presets; also update the tests
that expect double quotes to expect the new single-quoted/escaped output.
---
Duplicate comments:
In `@packages/cli/src/commands/apply.ts`:
- Around line 181-217: runInit currently may terminate the process via
handleError/process.exit which prevents the apply catch from running and
restoring .apply.bak; change runInit (and/or handleError) to surface failures
instead of exiting by either: 1) adding a non-exiting mode flag (e.g., runInit({
..., abortOnError: false } or similar) so runInit will throw errors instead of
calling process.exit, or 2) making handleError rethrow when invoked from runInit
in non-interactive flows; then update the runInit invocation in apply.ts to pass
the non-exiting flag (or call the throwing variant) so failures propagate to the
surrounding catch and trigger the rollback code that copies applyBackupPath back
to componentsJsonPath. Ensure any helper names referenced are runInit and
handleError/process.exit and that tests or callers expecting process exit are
updated accordingly.
---
Nitpick comments:
In `@packages/cli/src/commands/apply.test.ts`:
- Around line 24-187: The test suite never invokes the command's action body, so
add an action-level test that imports the exported apply command (the exported
object/function named apply) and calls its .action(...) to exercise the command
flow; set up a temporary cwd and fixtures to simulate missing config (to trigger
the preflight/missing-config branch) and create a fake .apply.bak file to verify
the restore path, mocking/stubbing filesystem ops (fs.rename/fs.exists/fs.unlink
or the helper functions used in apply), network/URL resolution if needed
(resolveApplyInitUrl), and process.exit behavior to assert the expected
outcomes; ensure the test cleans up mocks and temp files and asserts that
restore logic was executed and that the missing-config branch exits/returns as
expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3a17c43-1de0-4f3e-b746-5231c0a6babc
📒 Files selected for processing (6)
apps/v4/content/docs/06.cli.mdpackages/cli/src/commands/apply.test.tspackages/cli/src/commands/apply.tspackages/cli/src/index.tspackages/cli/src/preflights/preflight-apply.tspackages/cli/src/utils/get-project-info.ts
✅ Files skipped from review due to trivial changes (1)
- apps/v4/content/docs/06.cli.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/index.ts
- packages/cli/src/preflights/preflight-apply.ts
zernonia
force-pushed
the
feat-apply-preset
branch
from
53ba6bc to
780c2cb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/apply.test.ts`:
- Around line 61-78: The test "exits when positional and flag presets disagree"
creates an exitSpy via vi.spyOn(process, 'exit') with a mockImplementation but
calls exitSpy.mockRestore() unguarded; wrap the assertions and expectations in a
try/finally (or move cleanup to an afterEach) so exitSpy.mockRestore() always
runs even if an expectation fails—locate the exitSpy created around
vi.spyOn(process, 'exit') and ensure its mockRestore is executed in finally to
guarantee test isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9113b00b-087b-4ecf-bb29-d061c9627fde
📒 Files selected for processing (8)
apps/v4/content/docs/06.cli.mdpackages/cli/src/commands/apply.test.tspackages/cli/src/commands/apply.tspackages/cli/src/commands/init.tspackages/cli/src/index.tspackages/cli/src/preflights/preflight-apply.tspackages/cli/src/utils/file-helper.tspackages/cli/src/utils/get-project-info.ts
✅ Files skipped from review due to trivial changes (3)
- apps/v4/content/docs/06.cli.md
- packages/cli/src/index.ts
- packages/cli/src/commands/apply.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/utils/get-project-info.ts
- packages/cli/src/preflights/preflight-apply.ts
zernonia
force-pushed
the
feat-apply-preset
branch
2 times, most recently
from
a179ee3 to
12f578f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cli/src/commands/apply.test.ts (1)
61-78:⚠️ Potential issue | 🟡 MinorProtect
process.exitspy cleanup withfinally.At Line 77,
exitSpy.mockRestore()is unguarded. If an assertion fails first, the spy can leak into later tests.Proposed patch
it('exits when positional and flag presets disagree', () => { const exitSpy = vi .spyOn(process, 'exit') .mockImplementation(((_code?: number) => { throw new Error('process.exit called') }) as never) - expect(() => - resolveApplyPreset({ - ...baseOptions, - positionalPreset: 'vega', - preset: 'nova', - }), - ).toThrow('process.exit called') - - expect(exitSpy).toHaveBeenCalledWith(1) - exitSpy.mockRestore() + try { + expect(() => + resolveApplyPreset({ + ...baseOptions, + positionalPreset: 'vega', + preset: 'nova', + }), + ).toThrow('process.exit called') + + expect(exitSpy).toHaveBeenCalledWith(1) + } + finally { + exitSpy.mockRestore() + } })#!/bin/bash set -euo pipefail # Confirm the unguarded cleanup in this test block. cat -n packages/cli/src/commands/apply.test.ts | sed -n '61,80p' # Check whether global mock restoration is enabled in Vitest config/package settings. fd -i 'vitest.config.*' -x sh -c 'echo "== {} =="; sed -n "1,240p" "{}"' rg -n --iglob '*vitest*' --iglob 'package.json' '\brestoreMocks\b|\bclearMocks\b|\bmockReset\b'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/apply.test.ts` around lines 61 - 78, The test creates a process.exit spy (exitSpy) but calls exitSpy.mockRestore() unconditionally, which can leak the spy if an assertion throws; wrap the spy cleanup in a finally block so mockRestore always runs. Specifically, in the test case that calls resolveApplyPreset with positionalPreset and preset (the it block using exitSpy and calling resolveApplyPreset), move the exitSpy.mockRestore() into a finally that surrounds the expect(...).toThrow and expect(exitSpy).toHaveBeenCalledWith(1) assertions so the spy on process.exit is restored regardless of test failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/init.ts`:
- Around line 810-811: The current assignment lets opts.cssVariables (which
defaults to true) override project config and unintentionally mutate existing
tailwind.cssVariables; change the selection logic for cssVariables so you only
use opts.cssVariables when it was explicitly provided (e.g., check
opts.cssVariables !== undefined or use "cssVariables" in opts) and otherwise
fall back to defaultConfig.tailwind.cssVariables (leave baseColor behavior
unchanged), and update the caller/action layer to pass an explicit/optional
override flag from the Commander option source so only user-supplied flags
override project config.
---
Duplicate comments:
In `@packages/cli/src/commands/apply.test.ts`:
- Around line 61-78: The test creates a process.exit spy (exitSpy) but calls
exitSpy.mockRestore() unconditionally, which can leak the spy if an assertion
throws; wrap the spy cleanup in a finally block so mockRestore always runs.
Specifically, in the test case that calls resolveApplyPreset with
positionalPreset and preset (the it block using exitSpy and calling
resolveApplyPreset), move the exitSpy.mockRestore() into a finally that
surrounds the expect(...).toThrow and expect(exitSpy).toHaveBeenCalledWith(1)
assertions so the spy on process.exit is restored regardless of test failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6999bbd-00ff-44a4-b463-afbeb002e416
📒 Files selected for processing (9)
apps/v4/content/docs/06.cli.mdpackages/cli/src/commands/apply.test.tspackages/cli/src/commands/apply.tspackages/cli/src/commands/init.tspackages/cli/src/index.tspackages/cli/src/preflights/preflight-apply.tspackages/cli/src/utils/file-helper.tspackages/cli/src/utils/get-project-info.tspackages/cli/src/utils/updaters/update-css.ts
✅ Files skipped from review due to trivial changes (2)
- apps/v4/content/docs/06.cli.md
- packages/cli/src/commands/apply.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/preflights/preflight-apply.ts
- packages/cli/src/utils/file-helper.ts
zernonia
force-pushed
the
feat-apply-preset
branch
from
12f578f to
0b50099
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/cli/src/commands/apply.test.ts (1)
61-78:⚠️ Potential issue | 🟡 MinorGuard
process.exitspy cleanup withfinally.Line 77 cleanup is unguarded; if an expectation fails first, the spy can leak and break test isolation.
Suggested fix
it('exits when positional and flag presets disagree', () => { const exitSpy = vi .spyOn(process, 'exit') .mockImplementation(((_code?: number) => { throw new Error('process.exit called') }) as never) - expect(() => - resolveApplyPreset({ - ...baseOptions, - positionalPreset: 'vega', - preset: 'nova', - }), - ).toThrow('process.exit called') - - expect(exitSpy).toHaveBeenCalledWith(1) - exitSpy.mockRestore() + try { + expect(() => + resolveApplyPreset({ + ...baseOptions, + positionalPreset: 'vega', + preset: 'nova', + }), + ).toThrow('process.exit called') + + expect(exitSpy).toHaveBeenCalledWith(1) + } + finally { + exitSpy.mockRestore() + } })#!/bin/bash set -euo pipefail # Verify whether global Vitest config auto-restores mocks. fd -i 'vitest.config.*' -x sh -c 'echo "== {} =="; sed -n "1,220p" "{}"' rg -n --iglob '*vitest*' --iglob 'package.json' '\brestoreMocks\b|\bclearMocks\b|\bmockReset\b'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/apply.test.ts` around lines 61 - 78, The test uses a spy exitSpy on process.exit in the 'exits when positional and flag presets disagree' case but restores it only at the end, so a failing expectation can leak the spy; wrap the spy setup/restore in a try/finally around the expect block (or use before/after hooks) so exitSpy.mockRestore() always runs; update the test around resolveApplyPreset and exitSpy to ensure process.exit is mocked and then restored in a finally block referencing exitSpy, process.exit, and resolveApplyPreset.packages/cli/src/commands/init.ts (1)
810-811:⚠️ Potential issue | 🟠 Major
cssVariablesstill overrides project config without explicit user intent.Line 811 still reads
opts.cssVariablesdirectly, and Line 887 reassigns fromopts.cssVariablesagain. With Commander’s defaulted boolean option, this can still flip existingtailwind.cssVariablesunexpectedly in non-interactive runs.Suggested fix
- let cssVariables = opts.cssVariables ?? defaultConfig.tailwind.cssVariables + // Preserve project value by default; only override when the CLI flag was explicitly provided. + let cssVariables = defaultConfig.tailwind.cssVariables @@ - cssVariables = opts.cssVariables + cssVariables = options.tailwindCssVariables ?? cssVariablesThen wire an explicit override from the action layer (based on Commander option source), so only user-provided
--css-variables/--no-css-variableschanges this value.#!/bin/bash set -euo pipefail # Verify whether cssVariables is sourced from explicit CLI option intent or defaulted option value. sed -n '230,330p' packages/cli/src/commands/init.ts sed -n '800,910p' packages/cli/src/commands/init.ts rg -n --type=ts "getOptionValueSource|cssVariables|--css-variables|opts\\.cssVariables" \ packages/cli/src/commands/init.ts packages/cli/src/commands/apply.tsAlso applies to: 887-887
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/init.ts` around lines 810 - 811, The code currently assigns cssVariables from opts.cssVariables unconditionally which lets Commander’s defaulted boolean flip an existing default; change the action-layer assignment so cssVariables is only overridden when the CLI option was explicitly provided: use Commander’s getOptionValueSource for the css-variables flag (e.g. command.getOptionValueSource('--css-variables') or command.getOptionValueSource('cssVariables')) and only set cssVariables = opts.cssVariables when that source !== 'default' (otherwise keep defaultConfig.tailwind.cssVariables or the project config); apply the same guard where cssVariables is reassigned (referencing baseColor, cssVariables, defaultConfig.tailwind, and opts.cssVariables) so non-interactive runs don’t unintentionally flip the value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/utils/get-project-info.ts`:
- Around line 367-372: Replace the separate exists check and readdir with a
single atomic read guarded by try/catch: remove the fs.existsSync(uiDir)
pre-check and call await fs.readdir(uiDir, { withFileTypes: true }) inside a try
block (using the existing uiDir value), and on any error (e.g., ENOENT) return
[]; update the code around the uiDir variable and the entries usage so the
function (in get-project-info.ts where uiDir and entries are declared)
gracefully falls back to an empty array instead of throwing.
- Around line 375-385: The current loop over entries adds any immediate
directory (entry.isDirectory()) or many TS/JS files to names, causing
non-component artifacts to be included; update the logic so directories are only
added if they contain a component entry (e.g., contain
index.vue/index.tsx/index.ts or any *.vue file) and files are only added when
their basename looks like a component (e.g., PascalCase or ends with .component)
and is not in a short blacklist like ['utils','types','helpers','styles'];
modify the loop around entries, the checks using entry.isDirectory(),
entry.isFile(), and the regex for /\.(?:vue|ts|tsx|js|jsx)$/, and consult the
directory contents (fs.readdir or Dirent) to confirm an index or .vue exists
before calling names.add(entry.name) or names.add(path.basename(...)).
---
Duplicate comments:
In `@packages/cli/src/commands/apply.test.ts`:
- Around line 61-78: The test uses a spy exitSpy on process.exit in the 'exits
when positional and flag presets disagree' case but restores it only at the end,
so a failing expectation can leak the spy; wrap the spy setup/restore in a
try/finally around the expect block (or use before/after hooks) so
exitSpy.mockRestore() always runs; update the test around resolveApplyPreset and
exitSpy to ensure process.exit is mocked and then restored in a finally block
referencing exitSpy, process.exit, and resolveApplyPreset.
In `@packages/cli/src/commands/init.ts`:
- Around line 810-811: The code currently assigns cssVariables from
opts.cssVariables unconditionally which lets Commander’s defaulted boolean flip
an existing default; change the action-layer assignment so cssVariables is only
overridden when the CLI option was explicitly provided: use Commander’s
getOptionValueSource for the css-variables flag (e.g.
command.getOptionValueSource('--css-variables') or
command.getOptionValueSource('cssVariables')) and only set cssVariables =
opts.cssVariables when that source !== 'default' (otherwise keep
defaultConfig.tailwind.cssVariables or the project config); apply the same guard
where cssVariables is reassigned (referencing baseColor, cssVariables,
defaultConfig.tailwind, and opts.cssVariables) so non-interactive runs don’t
unintentionally flip the value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60673bcb-8ff3-4230-b90e-6e7e47a3df2a
📒 Files selected for processing (10)
apps/v4/content/docs/06.cli.mdpackages/cli/src/commands/apply.test.tspackages/cli/src/commands/apply.tspackages/cli/src/commands/init.tspackages/cli/src/index.tspackages/cli/src/preflights/preflight-apply.tspackages/cli/src/utils/add-components.tspackages/cli/src/utils/file-helper.tspackages/cli/src/utils/get-project-info.tspackages/cli/src/utils/updaters/update-css.ts
✅ Files skipped from review due to trivial changes (1)
- apps/v4/content/docs/06.cli.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/cli/src/index.ts
- packages/cli/src/preflights/preflight-apply.ts
- packages/cli/src/utils/file-helper.ts
- packages/cli/src/commands/apply.ts
zernonia
force-pushed
the
feat-apply-preset
branch
5 times, most recently
from
ae94646 to
62ada20
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zernonia
force-pushed
the
feat-apply-preset
branch
from
62ada20 to
18e3d02
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
shadcn-vue apply [preset], mirroringshadcn apply. Swaps the project's design preset and re-installs every UI component against it in one shot.reka-nova), an encoded preset from the web UI (00M9Mun7wYkt6), or a remote preset URL.baseandrtlare layered on top of the resolved preset so applying never silently switches the user's component library.components.jsontocomponents.json.apply.bakbefore delegating torunInit({ force: true, reinstall: true, skipPreflight: true, ... }), and restores it on failure.What's in the diff
packages/cli/src/commands/apply.ts— the new command + exported pure helpers (resolveApplyPreset,resolveApplyPresetConfig,getBase,getInitCommand).packages/cli/src/preflights/preflight-apply.ts— preflight that validatespackage.json+ a parseablecomponents.json.packages/cli/src/utils/get-project-info.ts— addsgetProjectComponents(cwd)which scans the resolvedui/dir to discover what to re-install.packages/cli/src/index.ts— registers the command.packages/cli/src/commands/apply.test.ts— 16 unit tests covering positional vs--presetresolution, base derivation, init-command formatting, and the three preset-resolution paths (named, encoded, URL via msw mock).apps/v4/content/docs/06.cli.md— adds the## applydoc section, matching shadcn-ui's docs style.Notes
applydoes not requirevue-metamorph. It works by re-downloading components from the registry under the new style URL, not by transforming local source. If a preset swap also changesiconLibraryorrtl, users still need to follow up withshadcn-vue migrate icons/migrate rtlto update their own (non-ui/) source files..apply.baksuffix to avoid colliding with the.baklifecycle thatinitalready manages internally via itsprocess.on('exit')handler.Test plan
pnpm --filter @shadcn/cli vitest run src/commands/apply.test.ts— 16 tests passnpx shadcn-vue apply reka-novain a cloned starter, confirmcomponents.json.styleupdates and existingui/components are re-fetched--preset <encoded>and a remote URLcomponents.jsonfrom.apply.bakSummary by CodeRabbit
New Features
Chores
Bug Fixes
Tests
Documentation