diff --git a/.changeset/build-list-json-registry-integrity.md b/.changeset/build-list-json-registry-integrity.md new file mode 100644 index 00000000..bf00de62 --- /dev/null +++ b/.changeset/build-list-json-registry-integrity.md @@ -0,0 +1,5 @@ +--- +"@sanring/cli": minor +--- + +`build`/`list --outdated` gain `--json` output. A new registry-integrity module (dangling `componentDeps`/`sharedDeps`/group references, unparseable peer versions, optional file-fetchability) is now shared across `doctor`, `build`, and the MCP `doctor_project` tool. `fetchRegistry`/`fetchFile` now throw a typed `RegistryFetchError` instead of calling `process.exit` directly — this also fixes `doctor`'s dead "Unreachable" catch block and 6 of 7 MCP tool handlers that had no `try`/`catch` around `getRegistry()` (a single failed fetch could previously kill the whole long-running MCP server). Also fixes a flaky-test root cause where `sync-registry.mjs`'s rm-then-async-copy raced with `mcp.e2e.test.ts`'s `npm run build`. diff --git a/.changeset/remove-exit-code-consistency.md b/.changeset/remove-exit-code-consistency.md new file mode 100644 index 00000000..f6c33188 --- /dev/null +++ b/.changeset/remove-exit-code-consistency.md @@ -0,0 +1,5 @@ +--- +"@sanring/cli": patch +--- + +`remove` now distinguishes unknown targets (hard exit 1, matching `diff`/`update`) from known-but-not-installed ones (soft skip), instead of silently exiting 0 whenever any target succeeded. Fixes a crash in `info`'s `alias:component` lookup, surfaced by newly added test coverage for `info`/`migrate`/`search`. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e65e9b62..6445bee8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,13 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # packages/cli/registry/ is gitignored, generated from the top-level + # registry/ by sync-registry.mjs — normally populated as part of `build`. + # Several tests (e.g. registry.test.ts's bundled-registry fallback) read + # it directly, so a clean checkout needs it synced before `test` runs. + - name: Sync registry fixtures + run: pnpm --filter @sanring/cli run sync-registry + - name: Test run: pnpm --filter @sanring/cli test diff --git a/DEVLOG.md b/DEVLOG.md index 045ed496..f0cf44d2 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -794,3 +794,92 @@ readonly isDisabled = computed(() => this.disabledInput() || this.disabledState( **TODOLIST.md P30 現況**:❌ 清單全部清空,15 項全部修復或查證排除完畢。 **沒有動姊妹 repo `date-picker`**:`date-picker-core` 刻意「zero DOM/CSS assumptions」,幫它加上這次的 ARIA 慣例會違背它自己的設計目標,也會讓它多一個要跨 repo 發版/更新依賴版本的環節,而這次的問題純粹是 `packages/ui` 自己組裝 grid 結構時的角色選擇,不是 headless engine 該管的事。`@sanring/date-picker`(組裝好的參考實作)也沒有需要跟進的東西——它沒有這個 bug,是因為它的元件形狀本來就不一樣(input+popup vs. inline grid),不是因為它比較新或比較對。 + + +--- + +## P27 — `remove` 混合 target 時 exit code 隱含成功 + +**查證**:`remove.ts` 原本把「registry 裡真的沒有這個元件(typo/未知)」跟「registry 裡有,只是這個專案沒裝」兩種完全不同的狀況混在同一個 `notInstalled` 陣列裡,只要陣列非空就印紅色 `✖` 錯誤,但退出碼只看 `plan.toRemove.length === 0` 這一個條件——只要至少一個 target 真的被移除,函式跑到底就是隱含 exit 0,即使其中混了一個打錯字的元件名稱。 + +對照 `diff.ts`/`update.ts` 兩者共用的 `resolveDiffTargets()` 既有慣例:`missing`(registry 裡完全找不到)一律視為輸入錯誤,印紅字後立即 `process.exit(1)`、不執行任何後續動作;`notInstalled`(registry 裡有,只是專案沒裝)只是軟性提示,印一行 dim 文字後繼續處理其餘 target,不影響最終 exit code。`remove` 從來沒有對齊這個既有區分——它自己的 `notInstalled` 實際上是「上述兩種情況的聯集」。 + +**修法**:`RemovalPlan` 拆成 `notInstalled`(registry 裡有、只是未安裝,沿用既有語意)與新增的 `unknown`(registry 裡完全沒有這個名字)。`planRemoval()` 用 `byName.has(n)` 區分兩者。command action 比照 `diff.ts` 的寫法:`plan.unknown.length > 0` 一律印紅字 `✖ Unknown component(s): ...` 並立即 `process.exit(1)`,在做任何刪除動作之前就擋下,徹底避免半調子的部分成功;`plan.notInstalled` 降級成 `pc.dim` 提示文字,不再影響 exit code——這修正了原本「視覺上宣告失敗但退出碼宣告成功」的不一致,做法是讓 `remove` 的兩種情境分別精確對齊 `diff`/`update` 各自既有的處理方式,而不是發明新規則。 + +**驗證**:`remove.test.ts` 新增/調整 3 個測試——`planRemoval` 單元測試拆成兩個(一個驗證已知但未裝的 `notInstalled`,一個驗證 registry 裡沒有的 `unknown`,原本的測試用例其實誤用了一個 registry 裡不存在的元件名稱去驗證「未安裝」語意,已修正成用真正已知的 `combobox`);整合測試新增「混合已知-未裝 + 可移除 target 時 exit 0」與「混合 unknown + 可移除 target 時 exit 1 且完全不刪除任何檔案」兩case,後者用 `vi.spyOn(process, 'exit')` 攔截驗證真的呼叫了 `exit(1)`,並斷言 `installedHashes` 裡原本可移除的元件的 hash 仍然存在(證明 unknown 檢查真的在任何刪除動作之前就擋下,不是刪了一半才失敗)。`pnpm --filter @sanring/cli exec vitest run`:15 個測試檔、178 個測試全過;`pnpm --filter @sanring/cli exec tsc --noEmit` 通過。 + +**TODOLIST.md P27 現況**:整體流程 4 項(CLI 主流程文件同步、`--json` 補齊、registry 完整性檢查抽共用工具、`fetchRegistry`/`fetchFile` typed error)與 `info`/`migrate`/`search` 缺測試這項仍待處理。 + +--- + +## P27 — `info`/`migrate`/`search` 補測試(過程中發現並修復一個真實 crash + 一個測試套件 flaky race) + +**執行**:`commands/` 下依 `check-registry-parity.mjs` 同一類手法先確認缺口範圍後,比照既有 command test(`doctor.test.ts`/`list.test.ts`)的慣例,新增 `info.test.ts`(7 個測試:project info 模式 `--json`/人類可讀、component 模式 `--json`/人類可讀/已安裝狀態、未知元件 exit 1、`alias:component` 語法)、`search.test.ts`(6 個測試:排序、無結果、`--json` 無結果、`--json` 含 `installed` 欄位的回歸測試、`--group`/`--tag` 過濾)、`migrate.test.ts`(9 個測試:up to date、breaking migration 印出步驟、`fromVersion` 早於已安裝版本時不重複觸發、`--check` 有/無待遷移時的 exit code、registry 裡已移除的元件、`alias:component` key 解析、`noData` 無 baseline 情境、config 不存在時的 exit 1)。 + +**寫測試過程中發現的真實 bug(`info.ts`)**:幫 `info` 補 `alias:component` 語法的回歸測試時(`sanring info other:widget --json`),命令直接丟 `TypeError: Cannot read properties of undefined (reading 'name')`。查證後發現:`info.ts` 稍早的 P27 修復(見 TODOLIST 已勾選項)只改對了「用哪個 registry 抓資料」(`resolveRegistrySource(parsedRef.alias, ...)`)跟「查 registry 用裸名稱」(`resolveInstallSet([bareComponentName], ...)`),但最後一行 `const component = toInstall.find((c) => c.name === componentName)!` 沒有跟著改——`toInstall` 裡的元件 `name` 是裸名稱,但這裡拿去比對的 `componentName` 是原始帶 alias 前綴的完整字串(例如 `"other:widget"`),永遠比對不到,`.find()` 回傳 `undefined`,後面用非空斷言 `!` 硬拆導致 crash。這正是 P27 這個小節的核心論點的具體案例——`info` 先前雖然「查過」也「修過」alias 支援,但因為沒有對應測試,一個明顯會炸掉的殘留 bug 完全沒被抓到。修法:把 `componentName` 改成 `bareComponentName`,一行修復,`sanring info : --json` 手動驗證正確輸出。 + +**寫測試過程中發現並修復的測試套件本身的 flaky race**(跟 command 程式碼無關,是測試基礎設施缺口):新增這三個檔案後,連續跑 `vitest run` 會間歇性(約 2-3/5 次)出現 `registry.test.ts` 裡完全不相關的測試失敗(`Cannot read properties of undefined (reading 'ok')`、`ENOENT: ... 'registry/registry.json'`)。追查後確認:`registry.test.ts` 有幾個測試用**相對路徑**(`'./registry'`)依賴 `process.cwd()` 停在 `packages/cli` 這個固定位置;但 `add`/`remove`/`doctor`/`list`/`info`/`migrate`/`search` 等每一個 command 的整合測試都會在 `beforeEach`/`afterEach` 呼叫 `process.chdir()` 切到各自的 temp project 目錄再切回來。`process.chdir()` 是**整個 process 共享的全域狀態**,不是 per-worker-thread 隔離的——Vitest 預設的 `threads` pool 用 `worker_threads` 在同一個 process 裡並行跑多個測試檔案,所以只要 `registry.test.ts` 剛好在另一個檔案的 `chdir` 視窗內執行,相對路徑就會解析到錯的目錄。用二分法驗證:拿掉新增的三個檔案,原本 178 個測試連續跑 10 次 0 次失敗;加回去後連續跑 5 次有 3 次失敗——不是我新測試邏輯本身有錯,是新增的三個「會 chdir」的檔案數量把既有的競速機率推高到容易觀察到的程度(這個 race 理論上原本就存在於 9 個既有的 chdir 檔案之間,只是機率較低沒被注意到)。**修法**:`packages/cli/vitest.config.ts` 加上 `pool: 'forks'`——改用真正獨立的 OS process(而非共享 process 的 worker thread)跑每個測試檔案,每個 process 有自己獨立的 `cwd`,徹底消除這整類競速,而不是逐一修 `registry.test.ts` 或新檔案去繞開它(那樣治標不治本,下一個新增的 chdir 檔案還是會重新觸發)。 + +**驗證**:`pnpm --filter @sanring/cli exec vitest run` 加上 `pool: 'forks'` 後連續跑 8 次、`pnpm --filter @sanring/cli exec vitest run --pool=forks` 也連續跑 8 次,共 16 次 0 次失敗(相同條件下拿掉這個設定會在 5 次內大概率重現);18 個測試檔、**200** 個測試全過(原 178 + 新增 22:`info` 7 + `search` 6 + `migrate` 9);`pnpm --filter @sanring/cli exec tsc --noEmit` 通過。 + +**TODOLIST.md P27 現況**:整體流程 4 項(CLI 主流程文件同步、`--json` 補齊、registry 完整性檢查抽共用工具、`fetchRegistry`/`fetchFile` typed error)仍待處理,`info`/`migrate`/`search` 缺測試與 `remove` exit code 兩項已完成。 + +--- + +## P28 — `packages/ui` 表單元件收斂至共用 `SanringCvaBase` + +**執行**:把 P14 只落在 registry 的 CVA 第二批重構完整移植到 `packages/ui`。新增 `packages/ui/src/lib/components/shared/cva-base.ts`,集中 `ControlValueAccessor` callback、disabled state、延後至 `ngOnInit()` 的 `NgControl` 解析、`control.events` 狀態橋接、Field described-by ids、focus/touched 與 `stateChanges`。`checkbox`、`switch`、`radio-group`、`slider`、`otp-input`、`calendar`、`date-picker`、`file-upload`、`combobox` 九個元件全部改成 `extends SanringCvaBase`,移除各檔重複的 lifecycle、signals、callbacks 與大型 `XxxFieldControlAdapter`。其中 `file-upload` 與 `combobox` 因前者用 `isDisabled`、後者用 plain-string `inputId` 作為 Field id,依 registry 設計保留薄型專用 adapter;其餘七個使用共用 `SanringFieldControlAdapter`。 + +**過程中補出的 registry 基底缺口**:第一次執行真正的 Angular library compile 時,`SanringCvaBase` 因使用 `inject()` 與 `OnInit`、但本身沒有 Angular decorator 而觸發 `NG2007`。在 `packages/ui` 與 `registry` 兩側 base 同步補上無 selector 的 `@Directive()`;這讓 base 能合法承載 Angular DI/lifecycle metadata,也讓兩份 `cva-base.ts` 維持完全相同。原先 registry source 沒有被 package TestBed 直接編譯,因此此前的靜態 parity check 不會抓到這類錯誤。 + +**驗證**:三批局部測試分別為 25、34、27 項,全數通過;完整 `pnpm ng test @sanring/ui --watch=false` 為 70 個 spec 檔、**404** 個測試全過;`pnpm ng build @sanring/ui` 成功;修改檔案 ESLint、Prettier、`git diff --check` 通過;`check-registry-sync.mjs`(52/52)與 `check-registry-parity.mjs`(52 個 shared component directories)皆綠燈。P28 已從 `TODOLIST.md` 移除。 + +--- + +## P27 — 整體流程 4 項收尾(`--json` 補齊、registry 完整性共用工具、typed error、help/README/docs 同步) + +P27 上一輪(見前面兩則條目)已修完 `remove` exit code 與 `info`/`migrate`/`search` 缺測試。這輪收掉「整體流程」剩下的 4 項,P27 在 `TODOLIST.md` 全部清空,整節移除。 + +**1. `build`/`list --outdated` 補 `--json`**:兩個 command 補 `--json` flag,全部既有 `console.log`/`console.error` 人類可讀輸出路徑改成 `if (!options.json)` 包住,成功/失敗都改印一份結構化 JSON(`build` 涵蓋 `ok`/`registryName`/`components`/`warnings`/`written`/`outDir`;`list --outdated` 涵蓋 `components`/`upToDate`/`outdated`/`conflicts`)。至此 `doctor`/`diff`/`search`/`info`/`build`/`list` 六個 CI/agent 常用 command 全部支援 `--json`。新增 `build.test.ts` 3 個、`list.test.ts` 2 個測試。 + +**2. registry 完整性檢查抽成共用工具**:新增 `packages/cli/src/registry-integrity.ts`——`findRegistryReferenceIssues(registry)`(同步:componentDeps/sharedDeps dangling reference、`groups[].components` dangling reference、peerDependencies 版本字串是否可解析)、`checkRegistryFilesFetchable(registry, source)`(非同步:逐檔 `fetchFile` 驗證 registry.json 宣告的每個檔案真的抓得到)、`checkRegistryIntegrity()`(整合兩者)。`isParseableVersionRange()` 是刻意輕量的 heuristic(不是完整 semver-range parser,這個 repo 沒有 bundle semver 套件),用「hyphen range 前後一定有空白、pre-release 的 hyphen 前後不會有空白」這個 npm 既有慣例區分兩者。 + +三個呼叫端各自按用途接線,不是每處都跑全部檢查:`doctor.ts` 在既有 registry fetch 成功後加一段(同步、零額外網路成本),把每個 dangling reference 各自轉成一筆獨立 `warn()`(不是彙總一行,讓 `--json` 模式也能拿到完整明細,而不是像既有 `orphaned`/`customized` 那組舊邏輯只有人類可讀模式看得到明細——這是新程式碼順手做對,沒有回頭改舊邏輯);`build.ts` 在 `validateRegistry` 成功、組完最終 `registry` 物件後跑同步檢查,主要抓 `registry.manifest.json` 手寫的 `groups` 引用到不存在的元件(`build.ts` 自己的 `validateReferencedTargets` 只驗證掃描到的 componentDeps/sharedDeps,從來沒驗證過 manifest 注入的 groups);`mcp.ts` 的 `doctor_project` tool 加一段,呼叫同步檢查並列出明細,不額外呼叫非同步的 file-fetchability 檢查(避免每次 agent 呼叫這個工具都多背一輪全量網路請求)。新增 `registry-integrity.test.ts`(29 測試)、`doctor.test.ts`/`build.test.ts`/`mcp.test.ts` 各補一個對應的整合回歸測試。 + +**3. `fetchRegistry`/`fetchFile` 改 throw typed error(過程中發現並修掉兩個真實的嚴重 bug)**:`registry.ts` 的 `die()`(`console.error` + `process.exit(1)`)換成 `export class RegistryFetchError extends Error`,兩個呼叫點(`fetchRegistry` 本地路徑分支、`fetchRegistryFromUrl`)改成 `throw new RegistryFetchError(message, { cause: e })`。`utils.ts` 新增 `reportRegistryFetchError(error, { json? })`,是「command 層決定怎麼印訊息與 exit」的共用落地點,9 個原本沒有包 try/catch 的呼叫端(`migrate`/`info`/`search`/`diff`/`remove`/`list`/`update`/`add`,`doctor` 已有既有 try/catch)全部補上。 + +過程中確認並修掉這個重構動機所指出的兩個真實、嚴重的既有 bug,而不只是理論上的程式碼異味: +- **`doctor.ts` 的 `catch { fail('Unreachable...') }` 之前是死碼**:`die()` 的 `process.exit(1)` 是同步、無條件終止整個 process,呼叫端任何 try/catch 都攔不到——`doctor --registry <壞掉的路徑>` 之前是直接印紅字終止,完全繞過 doctor 自己的 checks 陣列與 `--json` 輸出,`--json` 模式下會印出非法 JSON(其實是純文字錯誤訊息)而不是結構化錯誤。新增的回歸測試(`doctor.test.ts`)先確認這個情境下 `doctorCommand.parseAsync()` 真的能正常 resolve、`fail()` 真的執行到,而不是進程被殺掉。 +- **MCP server 之前會被單次 registry fetch 失敗整個殺掉**:`mcp.ts` 的 7 個 `getRegistry()` 呼叫點裡,只有 `doctor_project` 自己包了 try/catch,其餘 6 個(`list_components`/`search_components`/`get_component_info`/`plan_component_install`/`add_component`/`refresh_registry`)完全沒有——只要 registry 一次抓取失敗(typo 的 `--registry`、VPN 斷線、registry 端下線),`die()` 會直接砍掉這個長時間運行的 MCP server process,agent 整個 session 都會斷線,不只是這次工具呼叫失敗。查了 `@modelcontextprotocol/sdk` 原始碼確認 `Server.setRequestHandler` 本來就會把 handler 拋出的例外轉成正常的 JSON-RPC error response(`error.message` 直接變成回傳訊息),所以只要移除 `die()` 本身,不需要在每個呼叫點額外包 try/catch,SDK 自己的既有機制就會接住,不砍 process。新增的回歸測試(`mcp.test.ts`)驗證 `list_components` 對著壞掉的 registry 呼叫時,client 端拿到的是乾淨的 `McpError`(訊息就是 `Cannot read registry at: ...`),然後緊接著再呼叫一次 `search_components` 確認 server 還活著、還能回應(即使還是同一種失敗,重點是它有回應,不是連線直接斷掉)。 + +`registry.test.ts` 原本測 `die()` 行為的三個測試,用的是「spy `process.exit` 讓它拋一個假錯誤」這種間接手法(正是這次重構動機裡點名的「對測試不理想」);改成直接 `await expect(fetchRegistry(...)).rejects.toThrow(RegistryFetchError)`,不需要碰 `process.exit` 了。`add.test.ts` 也補了一個對應的端到端回歸(registry 不可達時 exit 1、印出乾淨錯誤訊息、不是 unhandled rejection)。 + +**過程中順手修掉的測試基礎設施 flaky race(跟本項無直接關係,但擋在驗證路上)**:改完 `fetchRegistry` 後連續跑 `vitest run` 出現間歇性、跟這次程式碼改動看似無關的 `registry.test.ts` 失敗。追查後發現:`mcp.e2e.test.ts` 會 `execSync('npm run build', ...)`,而 `packages/cli` 的 `build` script 含 `sync-registry`(`scripts/sync-registry.mjs`),這支腳本原本是 `rmSync(DEST_DIR)` → `mkdirSync` → 非同步 `cp(...)`——如果這支腳本跟 `registry.test.ts` 依賴同一個 `packages/cli/registry` 本地 bundle 目錄的測試同時跑,會有一個「目錄剛被刪、還沒複製完」的窗口,窗口大小等於整個非同步複製 52 個元件的時間。改成先複製進一個暫存的 sibling 目錄,複製完成後才用同步的 `rmSync` + `renameSync` 原地替換,把窗口從「複製所需時間」縮到「兩個同步 syscall 之間」,連續跑 10 次 `vitest run`(含觸發真正 `npm run build` 的 `mcp.e2e.test.ts`)全部通過,修復前同樣條件下 5 次內大概率重現。 + +**4. 重新定義 CLI 對外主流程並同步 help/README/docs**:`packages/cli/src/index.ts` 的 `program.addCommand()` 註冊順序改成對齊五個分組(Install: `init`/`add`/`remove`;Explore: `info`/`list`/`search`;Maintain: `diff`/`migrate`/`update`/`doctor`;Publish: `build`;Agent: `mcp`),讓 auto-generated `--help` 的 Commands 清單自然照這個順序列出,並在 `addHelpText('after', ...)` 補一段「Command groups」摘要。`packages/cli/README.md` 的 Common commands 區塊改成用同一組五分類呈現,補回原本完全沒列出的 `search`/`doctor`/`migrate`,結尾加一句指向 docs 站的完整文件連結(README 本來就刻意只列主流程,這點沒變)。 + +`apps/docs` CLI 頁面(`cli.overview.body`,en/zh 都改)原本寫「exposes nine commands」但實際 12 個,且完全沒提過 `migrate`;改成描述五分組敘事,並指向 Registry/MCP 頁面涵蓋 `build`/`mcp`。新增完整的 `migrate` section(title/body/code sample/flags list,en/zh 都補),補上原查證抓到的五個 flag 落差:`add` 補 `--check`、`remove` 補 `--dry-run`、`list` 補 `--outdated`/`--json`、`doctor` 補 `--fix`/`--json`、`diff` 補 `--summary`/`--json`;順手一併補了查證清單沒列出、但同樣過時的兩處:`search` 缺 `--group`/`--tag`/`--json`,以及 `search` 的 `--registry ` 沒跟上 `list` 已經做過的「URL or local path」措辭統一(`list --registry help` 那條 P27 舊項當時只改了 `list`,沒注意到 `search` 有一樣的落差)。`sections`/`commands` 陣列與模板裡的 section index 手動同步更新(新增 `migrate` section 讓後面的 `Requirements` section index 位移),`ng build docs --configuration=production` 成功驗證模板正確。 + +**驗證**:`pnpm --filter @sanring/cli exec vitest run` 連續 10 次、每次 18→19 個測試檔(新增 `registry-integrity.test.ts`)、**240** 個測試全過(原 200 + 本輪新增 40:`build` +3、`list` +2、`registry-integrity` 29、`doctor` +2、`add` +1、`mcp` +2、`migrate.test.ts` 拆分計數不變只是同一批已有的 22 個);`pnpm --filter @sanring/cli exec tsc --noEmit` 通過;`pnpm exec tsc --noEmit -p apps/docs/tsconfig.app.json` 通過;`pnpm exec ng build docs --configuration=production` 成功;`pnpm lint`(repo 全域)通過(含一個順手修掉的 `migrate.test.ts` 解構賦值 unused-var lint 錯誤,改用 `delete` 而非解構丟棄);`check-registry-sync.mjs`/`check-registry-parity.mjs` 皆綠燈。 + +**TODOLIST.md P27 現況**:整節(整體流程 4 項 + 各 command 弱點 26 項)全部完成,已從 `TODOLIST.md` 移除。 + +--- + +## P30 — 52 個元件 headless 掃描「建議修正」全數收斂 + +P30 先前已收完 15 個必修缺口;這輪把剩下 15 組建議項目逐一實作或查證排除,並同步修改 `packages/ui`、可安裝的 `registry` source、元件測試與中英文 Docs。P30 至此沒有未完成項目,整節已從 `TODOLIST.md` 移除。 + +**公開 API 與 Angular 結構**:`avatar-group-count` 補 `disabled` boolean coercion、停用語意與 click/keyboard guard;`breadcrumb` 補可在地化的 `ariaLabel`;`field` 與 `select` 的 `id` 改為可由消費者指定,同時保留自動產生 fallback 與 `SanringFieldControl` 相容性;`select-content`、`radio-item`、`sheet-content` 改用 signal query。所有新增 public input 與行為都同步進 Docs API table/範例/a11y 說明。 + +**Dialog、Sheet、Hover Card、Sidebar 語意**:`dialog`/`alert-dialog` 新增 `ariaLabel`、`ariaLabelledBy`、`ariaDescribedBy`,依「明確 input → DialogConfig → projected title/description → 元件 fallback」決定關聯,並用 signal `contentChild` + effect 處理動態加入/移除 title,避免殘留舊 attribute;沒有 title 的 `alertdialog` 也一定有 accessible name。`sheet` 使用同一套 explicit relationship/projected title/fallback 邏輯,且 nested `sheet-header` 已有回歸測試。`hover-card` 讓 trigger 與 content 透過 `aria-controls`/`aria-expanded`/穩定 id 建立關聯,有名稱時 content 具 `region` 語意。`sidebar` root 預設 `complementary` 與可覆寫 label;menu button/action 保留寬 selector相容性,同時讓非原生元素取得 `role="button"`、tab stop、Enter/Space activation、disabled guard,並忽略 key-repeat,無 `href` 的 anchor 也不再被誤認成原生互動元素。 + +**Combobox、Date Picker、Context Menu 焦點與鍵盤操作**:`combobox` 的 `inputId`/`listId` 可覆寫;Escape 與單選完成會在 panel 移除後回到 trigger/input,但 outside pointer close 不再排程回焦,因此不會搶走使用者剛點擊的外部 control。`date-picker` 補 `ariaLabel`/`ariaLabelledBy`,`disabled` 同時接受既有 matcher/matcher array 與整體 boolean(含裸 `disabled` attribute、`"false"` coercion),整體停用時 selection guard 與 ARIA 狀態一致。`context-menu` 改為每層 menu 只有一個 roving tab stop,方向鍵跳過 disabled item;Tab/Shift+Tab 會關閉完整 menu tree,並以 logical trigger 為基準移到文件中前/後相鄰的 control,不受 CDK overlay 被 portal 到 `` 尾端影響,root 與 submenu path 都有 activeElement 回歸測試。 + +**Transfer 與 Tree**:`transfer-item` 移除「整列 click + 巢狀 checkbox」的雙重 toggle source,改由整列本身承擔 `role="checkbox"`、`aria-checked`、`aria-disabled`、roving tabindex、click/Space;panel 支援 ArrowUp/ArrowDown/Home/End 且跳過 disabled item。`tree` root 補 `ariaLabel`/`ariaLabelledBy`,node 補 `disabled`、ARIA/data state 與 expand/select guard;children lookup 改為一次建立 parent map,避免每個 node 都掃全部 descendants 的 O(n²) 路徑。兩者 Docs keyboard/a11y/API 與 live examples 同步更新。 + +**刻意保留的 Dropdown Menu 分岔**:沒有為了表面共用而改寫。它使用 `@angular/aria/menu` 的永久 `DomPortal` attach-once 模型;`MenuOverlayController` 則負責自行 create/attach/detach overlay,直接重用會造成生命週期責任衝突。現況已有檔內說明且不是 correctness 缺口,結論是保留此 deliberate divergence。 + +**交叉 review 額外收掉的邊界**:修正 `hover-card-content` package/registry 四個 template handler 的 `protected` 可見性 drift;刪除 registry combobox 與 `SanringCvaBase` 完全重複的 `onFocus`/`onBlur`;補 Dialog title/description 放在 nested `DialogHeader` 時的 ARIA 測試,以及 Sheet 開啟後 panel focus 測試。Transfer 使用錯誤的 `--sanring-primary-foreground` token 也改回既有 `--sanring-primary-fg`。 + +**驗證**:`pnpm exec ng test "@sanring/ui" --watch=false` 為 70 個 spec 檔、**421 個測試全過**;`pnpm exec ng build "@sanring/ui"` 成功;P30 修改範圍 ESLint、Prettier、`git diff --check` 通過;`pnpm exec ngc -p apps/docs/tsconfig.app.json --noEmit` 通過;`check-registry-sync.mjs`(52/52)與 `check-registry-parity.mjs`(52 個 shared component directories)皆綠燈;Docs production build 成功,initial bundle 435.37 kB。首次 Docs build 的 `SIGABRT` 由 macOS crash report 定位到 Angular 22 local build cache 的 native LMDB `ExtendedEnv`(不是編譯診斷);清除 `.angular/cache` 後仍可重現,該次驗證以 `CI=true` 停用 local persistent cache,並在允許 Google Fonts inline request 後完整通過。 diff --git a/TODOLIST.md b/TODOLIST.md index a8ba0d0c..1f9400fd 100644 --- a/TODOLIST.md +++ b/TODOLIST.md @@ -59,96 +59,6 @@ Phase 4 已解封:Playwright 截圖 + `Read` 工具可以實際檢視 home(light --- -## P27 — CLI command UX / robustness 補強 - -針對 `packages/cli/src/commands/*` 逐一掃描後列出的待補項目。現況不是缺少核心 command,而是部分 command 的責任邊界、失敗狀態與 CI/agent 可讀性還可以再收斂。 - -### 整體流程 - -- [ ] 重新定義 CLI 對外主流程並同步 help/README/docs:`init`/`add` 是安裝流程,`info`/`list`/`search` 是探索流程,`diff`/`migrate`/`update`/`doctor` 是維護流程,`build` 是 registry 發布流程,`mcp` 是 agent 入口;npm README 只展示主流程,完整 flags 移到 docs - **查證(2026-08-19)**:`apps/docs` 的 CLI 頁面(`cli.overview.body`)文案寫「exposes nine commands — init, add, remove, info, diff, update, list, search, and doctor」,但 `packages/cli/src/index.ts` 實際註冊 12 個 top-level command。`mcp`、`build` 各有自己的文件頁(mcp page、registry page)不算缺,但 **`migrate` 在整個 docs 站完全沒有出現**(`apps/docs/src/app` 底下 grep 不到任何段落介紹它)。此外個別 command 的文件內文已經跟實際 flag 脫節:`add` 沒提到 `--check`、`remove` 沒提到 `--dry-run`、`list` 沒提到 `--outdated`、`doctor` 沒提到 `--fix`/`--json`、`diff` 沒提到 `--json`/`--summary`。 -- [ ] 補 `--json` 給 CI/agent 常用 command:至少 `doctor`、`diff`、`build --dry-run/--check`、`list --outdated`,避免只能解析人類可讀輸出 - **查證(2026-08-19)**:`doctor`/`diff`/`search`/`info` 已有 `--json`。`build`(僅有 `--check` 純文字輸出,無機器可讀格式)與 `list`(`--outdated`/`--installed` 皆無 `--json`)仍缺,是剩餘缺口。 -- [ ] 把 registry 完整性檢查抽成共用工具:schema、componentDeps/sharedDeps dangling reference、registry file 是否存在/可 fetch、peer dependency version 是否可解析,供 `doctor`、`build`、CI 與 MCP 重用 - **查證(2026-08-19)**:schema 驗證(`registry.ts` 的 `validateRegistry`)已經共用,`fetchRegistry` 內部會自動套用。但 componentDeps/sharedDeps dangling reference 檢查(`build.ts` 的 `validateReferencedTargets`)只在 `build` 產生 registry.json 當下驗證**自己掃描到的來源**,`doctor`/`mcp` 沒有等價入口可以驗證一份**已存在、可能是手寫或第三方**的 registry.json 內部 componentDeps/sharedDeps 是否有 dangling reference——`doctor` 目前只檢查「使用者專案安裝狀態 vs registry」,不檢查「registry.json 自己內部的引用完整性」,這仍是兩條分開的邏輯。 -- [ ] 將 `fetchRegistry`/`fetchFile` 底層錯誤改成 throw typed error,由 command 層決定怎麼印訊息與 exit;目前 `registry.ts` 內部 `die()` 直接 `process.exit(1)`,對測試與 MCP server reuse 都不理想 - **查證(2026-08-19)**:`registry.ts:78-84` 的 `die()` 現狀不變,`fetchRegistry`/`fetchFileFromUrl` 仍直接呼叫它終止行程,此項仍為有效缺口。 - -### 各 command 弱點 - -- [x] `init`:重跑時目前只提示 `sanring.config.json already exists`,但仍會直接覆寫 config;補明確確認或 `--force` 語意,並確保 theme/dependency 失敗時不留下半初始化狀態 -- [x] `init`:base dependency install 失敗只 warning,後續仍顯示 Done;調整 summary,把「已寫檔但依賴未安裝」列成 warning 並給可複製的修復命令 -- [x] `add`:`--diff` 與 `diff` 的比較邏輯重疊,且目前逐檔 await fetch;抽成共用 diff engine,讓 `add --diff`、`diff`、`update` 使用一致分類與 bounded concurrency -- [x] `add`:`--dry-run` 主要看 registry metadata,沒有驗證 registry file 真的可 fetch;補 `--check` 或 dry-run fetch validation,避免實際安裝時才發現缺檔 -- [x] `add`:peer dependency install 失敗後檔案已寫入且 config 仍可能更新;補 transaction-like summary,或把「source files written / peer deps failed」狀態明確記錄並引導 `doctor` -- [x] `remove`:刪 component 目錄時會整個 `rm -r`,若使用者在該 component 目錄加入自有檔案也會被刪;改成只刪 registry-tracked files,或在刪除前列出 extra files 並要求確認 -- [x] `remove`:目前只 prune `installedHashes`,沒有同步移除 `installedVersions`;補清理邏輯,尤其要涵蓋 `alias:component` key -- [x] `remove`:補 `--dry-run`,先列出會刪的 component files、會保留的 shared files、被 dependents 擋下的項目 -- [x] `diff`:指定未知 component 時只印錯誤但最後可能 success exit;改成 unknown target 一律 exit 1,`--exit-code` 只控制「有差異」的 exit behavior -- [x] `diff`:補 summary-only / json mode,避免大型 component diff 在 CI 或 agent output 裡過度冗長 -- [x] `update`:指定未知或未安裝 component 時目前偏向 skip,需統一 exit code 規則;未知 target 應失敗,未安裝 target 可提示 `sanring add` -- [x] `update`:和 `diff` 共用 target/job 建立邏輯,避免 theme/shared/component file 集合規則日後分歧 -- [x] `doctor`:補 registry integrity checks、peer dependency missing/outdated checks、`defaultRegistry` 是否存在於 `registries`、legacy `installedVersions` key 是否需要 migration -- [x] `doctor`:補 `--fix` 或至少 `--json`;`--fix` 可先只處理安全項目,例如 backfill missing hashes、清理不存在檔案的 hash、migrate installedVersions key -- [x] `build`:補 `--check`,只掃描/驗證/檢查 peer deps/檢查輸出一致性,不寫檔,給 CI 使用 -- [x] `build`:目前輸出 shared description 為空且不支援 groups metadata;評估加入可選 manifest,讓第三方 registry 能補 description、groups、since、migrations 等人工 metadata -- [x] `build`:確認 nested files / 同名 basename 是否會碰撞;目前輸出路徑用 `basename(file)`,若 component 內有子目錄或同名檔案會有風險 -- [x] `info`:component 模式未支援 `alias:component` 語法,與 `add` 的 multi-registry 使用方式不一致;補 parse 或明確禁止並提示 `--registry` -- [x] `info`:`getCliVersion` 與 MCP 內部重複實作,改用 `utils.getCliVersion()` -- [x] `list`:`--registry` help 寫 `custom registry URL`,但實作可接 local path;統一為 `custom registry (URL or local path)` -- [x] `list --installed`:目前靠 component 目錄掃描,不看 `installedVersions` alias key;評估是否以 config 為主、目錄為輔,讓 multi-registry 狀態更準 -- [x] `search`:目前只有 substring ranking;元件變多後補 fuzzy/token ranking、category/tag filter 與 `--json` -- [x] `migrate`:目前用 `installedVersions` key 直接查 registry component name,`alias:component` 會被當成不存在;先 parse alias key,再對 bare component name 查 registry -- [x] `migrate`:`noData` result 型別目前未實際產生;補 legacy/no baseline 情境或移除 dead branch -- [x] `mcp`:registry cache 沒有 refresh/invalidate;長時間 agent session 可能看不到 registry 更新,補 refresh tool 或 TTL -- [x] `mcp`:目前 agent 只能 list/search/info/plan/add,缺 `diff`、`doctor`、`migrate`/`update` 的安全入口;補 read-only 檢查工具,再評估是否開放更新工具 -- [ ] `info`、`migrate`、`search` 三個 command 完全沒有對應的 `*.test.ts`(`commands/` 下其餘 9 個 command 都有);`search --json` 曾經有一個未被任何測試發現的 TDZ crash(見下方查證),凸顯沒測試的 command 風險較高,優先補齊這三個 - **查證(2026-08-19)**:`diff <(ls packages/cli/src/commands/*.ts | grep -v test) <(ls packages/cli/src/commands/*.test.ts)` 確認只有這三個 command 缺測試檔。 -- [ ] `remove`:混合 target(部分已安裝、部分未安裝)時,未安裝的目標會印出紅色 `✖` 錯誤訊息,但只要至少一個 target 成功移除,整個 process 仍以 exit code 0 結束——視覺上宣告失敗但退出碼宣告成功,跟 `diff`/`update` 對「未安裝視為軟性提示」或「未知一律 exit 1」的既有慣例都不一致,需要決定 remove 的未安裝目標到底該不該讓整體 exit code 非 0 - **查證(2026-08-19)**:`remove.ts:127-136`——只有當 `plan.toRemove.length === 0`(也就是全部都未安裝)才會 `process.exit(plan.notInstalled.length > 0 ? 1 : 0)`;只要有任何一個 target 可以移除,函式跑到底沒有再檢查 `plan.notInstalled`,隱含 exit 0。 - ---- - -## P28 — P14 CVA 重構未套用到 `packages/ui` - -- [ ] `packages/ui` 的 9 個表單元件(`checkbox`/`switch`/`radio-group`/`slider`/`otp-input`/`date-picker`/`calendar`/`file-upload`/`combobox`)補做 P14 第二批重構:改成 `extends SanringCvaBase`,跟 `registry/` 對齊,徹底消除架構分岔 - -**現況**:P14 第二批重構(`a9cb0fd`)只實際套用在 `registry/shared/cva-base.ts` + `registry/components/*`,`packages/ui` 的對應 9 個元件從未跟進,兩邊架構自此分岔——`packages/ui` 還是舊的手刻 `XxxFieldControlAdapter`。**根因(驗證缺口)已解決**:新增 `packages/cli/scripts/check-registry-parity.mjs`(已掛進 `.github/workflows/registry-sync-check.yml`),靜態比對 `packages/ui` 與 `registry/` 每個同名元件檔案的 `input()`/`output()`/`model()` 宣告 + a11y 相關 attribute binding 表達式(`aria-*`/`role`/`disabled`/`tabindex`/`id`)。跑起來後除了 `/audit-component` 已經抓到的 `switch`/`checkbox`/`radio-group` 三筆,又額外挖出四筆獨立的 registry-only regression 並修正:`button`(本次 session 自己 cherry-pick 時漏同步 `role="button"` 修復,外加 `rounded-lg`/硬編碼 destructive 色碼兩個更早的 design token 漂移)、`context-menu-sub-trigger`(漏 `aria-disabled`)、`resizable-handle` + `resizable-group`(整組 `aria-valuenow`/`min`/`max` keyboard-splitter 支援完全沒同步過去)。目前這個腳本是純靜態 regex 比對,不是真的執行 registry 程式碼(嘗試讓 `packages/ui` 的 TestBed 直接 import registry 元件失敗了——Angular 的 build 工具鏈假設單一 project rootDir,跨 project import 會讓 `extends SanringCvaBase` 的型別解析失敗,細節見下方腳本檔頭註解)。剩餘工作(改用 `SanringCvaBase`)是解決架構分岔本身,優先度較低,兩邊行為已經有腳本守著。 - ---- - -## P30 — `packages/ui` 52 個元件 headless 品質全庫掃描 - -**現況(2026-08-19)**:用 `/audit-sweep`(本次新增的自檢 skill)對 `packages/ui/src/lib/components/` 下全部 52 個元件做了一輪唯讀分批稽核,套用 `/audit-component` Phase 1–3 的檢查表(Angular 結構、a11y、props/API 設計)。這不是逐元件的完整 `/audit-component`(沒有跑 Phase 4-6 的 spec 補寫與 usage evidence),純粹是「這個元件夠不夠格當一個 headless library 元件」的靜態掃描,找到 15 個元件有明確缺口、另外約 20 個是次要/建議事項。`packages/ui`↔`registry/` 的同步(`check-registry-parity.mjs`/`check-registry-sync.mjs`)目前都是綠燈,本節找到的都是 `packages/ui`/`registry` **兩邊共有**的新缺口,不是既有的分岔問題。 - -**已修復(2026-08-19)**:`button`(`sanringBtn` 套在沒有 `href` 的 `` 上時)、`context-menu-trigger`、`sidebar-trigger` 這三個獨立元件都出現過同一種「`role="button"` 卻沒有同時給 `tabindex` 與鍵盤支援」的模式,已分別修好(`packages/ui`/`registry` 同步)。詳見 DEVLOG。三個各自的成因不同——`button` 補了 `tabindex`+`keydown.enter`/`.space`;`context-menu-trigger` 只補 `tabindex`(它的鍵盤等價操作是 Shift+F10/選單鍵,瀏覽器會自動轉成既有的 `contextmenu` 事件,不需要額外的 Enter/Space handler);`sidebar-trigger` 是把 selector 從 `[sanringSidebarTrigger]` 收緊成 `button[sanringSidebarTrigger]`(比照 `sidebar-rail.directive.ts` 既有的 `button[sanringSidebarRail]` 慣例,所有既有用法本來就都是 ` - + @@ -224,6 +244,7 @@ import { TreeComponent, TreeNodeComponent, TreeGroupComponent, TreeTriggerDirect export class ExampleComponent {}`, usageMain: `
-

DELIVERY MAP

What is shipped, next, and forming.

v0.23.3
+

DELIVERY MAP

What is shipped, next, and forming.

v0.24.0

SHIPPED

{{ shipped.length }}

available now

TIER 1

{{ tier1.length }}

next in line

TIER 2

{{ tier2.length }}

in planning

TIER 3+

{{ tier3.length + tier4.length }}

forming

diff --git a/packages/cli/README.md b/packages/cli/README.md index be501fa8..499e30d6 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -21,18 +21,29 @@ Angular CLI users can bootstrap with: ng add @sanring/cli ``` -Common commands: +Commands are grouped by what stage of your workflow they belong to: ```bash -npx @sanring/cli@latest list # browse available components -npx @sanring/cli@latest info date-picker # preview what would be installed -npx @sanring/cli@latest add date-picker # copy component into your project -npx @sanring/cli@latest diff date-picker # compare local files against registry -npx @sanring/cli@latest update date-picker # apply registry changes -npx @sanring/cli@latest remove date-picker # remove a component +# Install +npx @sanring/cli@latest init # set up sanring.config.json + theme +npx @sanring/cli@latest add date-picker # copy a component into your project +npx @sanring/cli@latest remove date-picker # remove an installed component + +# Explore +npx @sanring/cli@latest list # browse available components +npx @sanring/cli@latest search date # search by name or description +npx @sanring/cli@latest info date-picker # preview what would be installed + +# Maintain +npx @sanring/cli@latest diff date-picker # compare local files against registry +npx @sanring/cli@latest migrate date-picker # check for breaking-change migration steps +npx @sanring/cli@latest update date-picker # apply registry changes +npx @sanring/cli@latest doctor # check environment + project health ``` -Every command accepts `--registry ` to point at a custom registry. `sanring.config.json` also supports `registries` and `defaultRegistry` for permanent alias configuration. Run any command with `--help` for the full flag list. +`build` generates a `registry.json` for publishing your own registry, and `mcp` starts an MCP server for AI coding agents — see below. + +Every command accepts `--registry ` to point at a custom registry. `sanring.config.json` also supports `registries` and `defaultRegistry` for permanent alias configuration. Run any command with `--help` for the full flag list, or see [ui.sanring.dev/cli](https://ui.sanring.dev/cli) for complete documentation. ## Notes diff --git a/packages/cli/scripts/sync-registry.mjs b/packages/cli/scripts/sync-registry.mjs index 3f48e71d..59361c92 100644 --- a/packages/cli/scripts/sync-registry.mjs +++ b/packages/cli/scripts/sync-registry.mjs @@ -7,7 +7,7 @@ // Root cause this replaces: `packages/cli/registry` used to be populated // by hand and had no build step keeping it in sync with root `registry/`, // so published CLI versions could silently ship stale component code. -import { existsSync, mkdirSync, readFileSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, readFileSync, renameSync, rmSync } from 'node:fs'; import { cp } from 'node:fs/promises'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -15,16 +15,27 @@ import { fileURLToPath } from 'node:url'; const __dirname = dirname(fileURLToPath(import.meta.url)); const SOURCE_DIR = join(__dirname, '../../../registry'); const DEST_DIR = join(__dirname, '../registry'); +const TMP_DIR = join(__dirname, '../registry.sync-tmp'); +// Copies into a sibling temp dir, then swaps it into place with a single +// rm+rename — `packages/cli/src/registry.test.ts` (and anything else that +// reads the bundled local registry at DEST_DIR) can otherwise observe a +// window where DEST_DIR has just been rm'd but the multi-file async `cp` +// hasn't finished yet if this script runs concurrently with the test suite +// (e.g. `mcp.e2e.test.ts` shells out to `npm run build`, which calls this). +// rm+rename is not atomic as a pair, but shrinks that window from "however +// long the copy takes" to a single synchronous syscall each. async function sync() { if (!existsSync(SOURCE_DIR)) { console.error(`✖ Source registry not found at ${SOURCE_DIR}`); process.exit(1); } + rmSync(TMP_DIR, { recursive: true, force: true }); + mkdirSync(TMP_DIR, { recursive: true }); + await cp(SOURCE_DIR, TMP_DIR, { recursive: true }); rmSync(DEST_DIR, { recursive: true, force: true }); - mkdirSync(DEST_DIR, { recursive: true }); - await cp(SOURCE_DIR, DEST_DIR, { recursive: true }); + renameSync(TMP_DIR, DEST_DIR); console.log(`✔ Synced registry: ${SOURCE_DIR} -> ${DEST_DIR}`); } diff --git a/packages/cli/src/commands/add.test.ts b/packages/cli/src/commands/add.test.ts index 78a0cbef..50ec5301 100644 --- a/packages/cli/src/commands/add.test.ts +++ b/packages/cli/src/commands/add.test.ts @@ -208,6 +208,27 @@ describe('addCommand (integration)', () => { ); }); + it('exits 1 with a clean error instead of an unhandled rejection when the registry is unreachable', async () => { + const errors: string[] = []; + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.join(' ')); + }); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit'); + }); + const brokenRegistryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-add-broken-registry-')); + + await expect( + addCommand.parseAsync(['widget', '--registry', brokenRegistryDir], { from: 'user' }), + ).rejects.toThrow('process.exit'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errors.some((e) => e.includes('Cannot read registry at'))).toBe(true); + + rmSync(brokenRegistryDir, { recursive: true, force: true }); + exitSpy.mockRestore(); + }); + it('preserves registries/defaultRegistry from the existing config on write', async () => { writeConfig(projectDir, { componentPath: 'src/app/components/ui', diff --git a/packages/cli/src/commands/add.ts b/packages/cli/src/commands/add.ts index 67fb16f4..60b06f46 100644 --- a/packages/cli/src/commands/add.ts +++ b/packages/cli/src/commands/add.ts @@ -23,6 +23,7 @@ import { hashContent, fetchTextTargetsConcurrent, readConfig, + reportRegistryFetchError, requireAngularProject, resolveComponentPath, resolveRegistrySource, @@ -258,7 +259,13 @@ export const addCommand = new Command('add') // Fetch registry const registrySpinner = ora('Loading registry...').start(); - const registry = await fetchRegistry(registrySource); + let registry; + try { + registry = await fetchRegistry(registrySource); + } catch (e) { + registrySpinner.stop(); + reportRegistryFetchError(e); + } const registryIndex = createRegistryIndex(registry); registrySpinner.stop(); diff --git a/packages/cli/src/commands/build.test.ts b/packages/cli/src/commands/build.test.ts index 51a0966b..70ad9009 100644 --- a/packages/cli/src/commands/build.test.ts +++ b/packages/cli/src/commands/build.test.ts @@ -17,13 +17,17 @@ describe('buildCommand (integration)', () => { let outDir: string; let originalCwd: string; let exitSpy: ReturnType; + let logs: string[]; beforeEach(() => { originalCwd = process.cwd(); cwd = mkdtempSync(join(tmpdir(), 'sanring-build-cwd-')); outDir = join(cwd, 'dist-registry'); process.chdir(cwd); - vi.spyOn(console, 'log').mockImplementation(() => {}); + logs = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')); + }); vi.spyOn(console, 'warn').mockImplementation(() => {}); vi.spyOn(console, 'error').mockImplementation(() => {}); exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); @@ -39,6 +43,8 @@ describe('buildCommand (integration)', () => { buildCommand.setOptionValue('out', './dist-registry'); buildCommand.setOptionValue('name', undefined); buildCommand.setOptionValue('dryRun', false); + buildCommand.setOptionValue('check', false); + buildCommand.setOptionValue('json', false); }); afterEach(() => { @@ -113,6 +119,88 @@ describe('buildCommand (integration)', () => { expect(existsSync(outDir)).toBe(false); }); + it('flags a manifest group referencing an unknown component as a warning, not a build failure', async () => { + const sourceDir = writeMinimalSource(); + writeFileSync( + join(cwd, 'package.json'), + JSON.stringify({ name: 'test-registry', dependencies: { '@lucide/angular': '^1.18.0' } }), + 'utf-8', + ); + writeFileSync( + join(cwd, 'registry.manifest.json'), + JSON.stringify({ groups: [{ id: 'forms', title: 'Forms', components: ['widget', 'does-not-exist'] }] }), + 'utf-8', + ); + + await buildCommand.parseAsync(['--source', sourceDir, '--out', outDir, '--check', '--json'], { + from: 'user', + }); + + expect(exitSpy).not.toHaveBeenCalled(); + const report = JSON.parse(logs.join('')) as { ok: boolean; warnings: Array<{ message: string }> }; + expect(report.ok).toBe(true); + expect(report.warnings.some((w) => w.message.includes('does-not-exist'))).toBe(true); + }); + + it('--check --json prints a machine-readable summary and writes nothing', async () => { + const sourceDir = writeMinimalSource(); + writeFileSync( + join(cwd, 'package.json'), + JSON.stringify({ name: 'test-registry', dependencies: { '@lucide/angular': '^1.18.0' } }), + 'utf-8', + ); + + await buildCommand.parseAsync(['--source', sourceDir, '--out', outDir, '--check', '--json'], { + from: 'user', + }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(existsSync(outDir)).toBe(false); + const report = JSON.parse(logs.join('')) as { + ok: boolean; + registryName: string; + written: boolean; + components: Array<{ name: string; componentDeps: string[]; sharedDeps: string[]; peerDependencies: string[] }>; + }; + expect(report.ok).toBe(true); + expect(report.registryName).toBe('test-registry'); + expect(report.written).toBe(false); + expect(report.components).toEqual([ + { name: 'widget', componentDeps: [], sharedDeps: ['utils'], peerDependencies: ['@lucide/angular'] }, + ]); + }); + + it('--json (without --check) reports written:true and outDir after actually writing', async () => { + const sourceDir = writeMinimalSource(); + writeFileSync( + join(cwd, 'package.json'), + JSON.stringify({ name: 'test-registry', dependencies: { '@lucide/angular': '^1.18.0' } }), + 'utf-8', + ); + + await buildCommand.parseAsync(['--source', sourceDir, '--out', outDir, '--json'], { from: 'user' }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(existsSync(join(outDir, 'registry.json'))).toBe(true); + const report = JSON.parse(logs.join('')) as { written: boolean; outDir: string }; + expect(report.written).toBe(true); + expect(report.outDir).toBe(outDir); + }); + + it('--json reports unresolved peer dependencies as structured output, not colored text', async () => { + const sourceDir = writeMinimalSource(); + // No package.json at all -> nothing can resolve @lucide/angular's version. + + await buildCommand.parseAsync(['--source', sourceDir, '--out', outDir, '--name', 'test-registry', '--json'], { + from: 'user', + }); + + expect(exitSpy).toHaveBeenCalledWith(1); + const report = JSON.parse(logs.join('')) as { ok: boolean; unresolvedPeerDependencies: string[] }; + expect(report.ok).toBe(false); + expect(report.unresolvedPeerDependencies).toEqual(['@lucide/angular']); + }); + it('fails without writing when a peerDependency version cannot be resolved', async () => { const sourceDir = writeMinimalSource(); // No package.json at all -> nothing can resolve @lucide/angular's version. diff --git a/packages/cli/src/commands/build.ts b/packages/cli/src/commands/build.ts index ebeed0e2..e0055611 100644 --- a/packages/cli/src/commands/build.ts +++ b/packages/cli/src/commands/build.ts @@ -8,6 +8,7 @@ import { type RegistryShared, validateRegistry, } from '../registry.js'; +import { findRegistryReferenceIssues } from '../registry-integrity.js'; import { canonicalizePeerDependencies, classifyModuleSpecifiers, @@ -115,23 +116,38 @@ export const buildCommand = new Command('build') .option('--manifest ', 'optional metadata manifest (default: registry.manifest.json)') .option('--dry-run', 'preview without writing files', false) .option('--check', 'scan and validate without writing files (CI mode)', false) - .action(async (options: { source: string; out: string; name?: string; manifest?: string; dryRun: boolean; check: boolean }) => { + .option('--json', 'output machine-readable build results (useful for CI and coding agents)', false) + .action(async (options: { source: string; out: string; name?: string; manifest?: string; dryRun: boolean; check: boolean; json: boolean }) => { const cwd = process.cwd(); const sourceDir = resolve(cwd, options.source); const outDir = resolve(cwd, options.out); const cwdPackageJson = readCwdPackageJson(cwd); if (!existsSync(sourceDir)) { - console.error(pc.red(`✖ Source directory not found: ${sourceDir}`)); + if (options.json) { + console.log(JSON.stringify({ ok: false, error: `Source directory not found: ${sourceDir}` }, null, 2)); + } else { + console.error(pc.red(`✖ Source directory not found: ${sourceDir}`)); + } process.exit(1); return; } const registryName = options.name ?? cwdPackageJson?.name; if (!registryName) { - console.error( - pc.red('✖ No registry name. Pass --name , or add a "name" field to package.json.'), - ); + if (options.json) { + console.log( + JSON.stringify( + { ok: false, error: 'No registry name. Pass --name , or add a "name" field to package.json.' }, + null, + 2, + ), + ); + } else { + console.error( + pc.red('✖ No registry name. Pass --name , or add a "name" field to package.json.'), + ); + } process.exit(1); return; } @@ -159,16 +175,24 @@ export const buildCommand = new Command('build') ]), ); + const warnings: Array<{ component: string; message: string }> = []; + for (const raw of rawComponents) { const v = validated.get(raw.name)!; for (const dropped of v.droppedComponentDeps) { - console.warn(pc.yellow(`⚠ ${raw.name}: dropping componentDep "${dropped}" — no matching component found`)); + const message = `dropping componentDep "${dropped}" — no matching component found`; + warnings.push({ component: raw.name, message }); + if (!options.json) console.warn(pc.yellow(`⚠ ${raw.name}: ${message}`)); } for (const dropped of v.droppedSharedDeps) { - console.warn(pc.yellow(`⚠ ${raw.name}: dropping sharedDep "${dropped}" — no matching shared file found`)); + const message = `dropping sharedDep "${dropped}" — no matching shared file found`; + warnings.push({ component: raw.name, message }); + if (!options.json) console.warn(pc.yellow(`⚠ ${raw.name}: ${message}`)); } if (!raw.description) { - console.warn(pc.yellow(`⚠ ${raw.name}: no description found, edit registry.json manually`)); + const message = 'no description found, edit registry.json manually'; + warnings.push({ component: raw.name, message }); + if (!options.json) console.warn(pc.yellow(`⚠ ${raw.name}: ${message}`)); } } @@ -204,15 +228,25 @@ export const buildCommand = new Command('build') }); if (unresolved.size > 0) { - console.error( - pc.red( - `✖ Could not resolve a version for ${unresolved.size} peer dependenc${unresolved.size > 1 ? 'ies' : 'y'}:`, - ), - ); - for (const name of [...unresolved].sort()) console.error(pc.dim(` - ${name}`)); - console.error( - pc.dim(' Add them to dependencies/devDependencies/peerDependencies in package.json and re-run.'), - ); + if (options.json) { + console.log( + JSON.stringify( + { ok: false, error: 'unresolved-peer-dependencies', unresolvedPeerDependencies: [...unresolved].sort() }, + null, + 2, + ), + ); + } else { + console.error( + pc.red( + `✖ Could not resolve a version for ${unresolved.size} peer dependenc${unresolved.size > 1 ? 'ies' : 'y'}:`, + ), + ); + for (const name of [...unresolved].sort()) console.error(pc.dim(` - ${name}`)); + console.error( + pc.dim(' Add them to dependencies/devDependencies/peerDependencies in package.json and re-run.'), + ); + } process.exit(1); return; } @@ -227,25 +261,66 @@ export const buildCommand = new Command('build') try { validatedRegistry = validateRegistry(registry); } catch (e) { - console.error(pc.red('✖ Generated registry failed validation:')); - console.error(pc.dim(` ${e instanceof Error ? e.message : String(e)}`)); + const message = e instanceof Error ? e.message : String(e); + if (options.json) { + console.log(JSON.stringify({ ok: false, error: 'validation-failed', message }, null, 2)); + } else { + console.error(pc.red('✖ Generated registry failed validation:')); + console.error(pc.dim(` ${message}`)); + } process.exit(1); return; } - console.log(pc.cyan(`\n${pc.bold(registryName)}`) + pc.dim(` — ${components.length} component(s), ${shared.length} shared file(s)\n`)); - for (const component of components) { - const deps: string[] = []; - if (component.componentDeps?.length) deps.push(`componentDeps: ${component.componentDeps.join(', ')}`); - if (component.sharedDeps?.length) deps.push(`sharedDeps: ${component.sharedDeps.join(', ')}`); - if (component.peerDependencies && Object.keys(component.peerDependencies).length) { - deps.push(`peerDependencies: ${Object.keys(component.peerDependencies).join(', ')}`); + // Component/sharedDep candidates were already validated against + // discovered names during scanning (see `validated` above), so this + // mainly catches what scanning can't: a manifest-supplied `groups` entry + // referencing an unknown component, or a garbage peer dependency version + // string pulled verbatim from package.json. + for (const issue of findRegistryReferenceIssues(validatedRegistry)) { + warnings.push({ component: '(registry)', message: issue.message }); + if (!options.json) console.warn(pc.yellow(`⚠ ${issue.message}`)); + } + + const componentSummaries = components.map((component) => ({ + name: component.name, + componentDeps: component.componentDeps ?? [], + sharedDeps: component.sharedDeps ?? [], + peerDependencies: component.peerDependencies ? Object.keys(component.peerDependencies) : [], + })); + + if (!options.json) { + console.log(pc.cyan(`\n${pc.bold(registryName)}`) + pc.dim(` — ${components.length} component(s), ${shared.length} shared file(s)\n`)); + for (const component of components) { + const deps: string[] = []; + if (component.componentDeps?.length) deps.push(`componentDeps: ${component.componentDeps.join(', ')}`); + if (component.sharedDeps?.length) deps.push(`sharedDeps: ${component.sharedDeps.join(', ')}`); + if (component.peerDependencies && Object.keys(component.peerDependencies).length) { + deps.push(`peerDependencies: ${Object.keys(component.peerDependencies).join(', ')}`); + } + console.log(` ${pc.bold(component.name)}${deps.length ? pc.dim(` (${deps.join('; ')})`) : ''}`); } - console.log(` ${pc.bold(component.name)}${deps.length ? pc.dim(` (${deps.join('; ')})`) : ''}`); } if (options.dryRun || options.check) { - console.log(pc.dim(`\n ${options.check ? 'Check' : 'Dry run'} — nothing written${options.check ? '.' : `; run without --dry-run to write to ${outDir}.`}\n`)); + if (options.json) { + console.log( + JSON.stringify( + { + ok: true, + registryName, + components: componentSummaries, + sharedCount: shared.length, + warnings, + written: false, + }, + null, + 2, + ), + ); + } else { + console.log(pc.dim(`\n ${options.check ? 'Check' : 'Dry run'} — nothing written${options.check ? '.' : `; run without --dry-run to write to ${outDir}.`}\n`)); + } return; } @@ -273,5 +348,23 @@ export const buildCommand = new Command('build') } } - console.log(pc.green(`\n✔ Wrote ${outDir}\n`)); + if (options.json) { + console.log( + JSON.stringify( + { + ok: true, + registryName, + components: componentSummaries, + sharedCount: shared.length, + warnings, + written: true, + outDir, + }, + null, + 2, + ), + ); + } else { + console.log(pc.green(`\n✔ Wrote ${outDir}\n`)); + } }); diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index 71703101..dc88fa9c 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -15,6 +15,7 @@ import { fetchTextTargetsConcurrent, isUntouchedSinceInstall, readConfig, + reportRegistryFetchError, requireAngularProject, resolveComponentBasePath, resolveRegistrySource, @@ -181,7 +182,12 @@ export const diffCommand = new Command('diff') ? resolve(cwd, config.sharedPath) : join(componentBasePath, 'shared'); - const registry = await fetchRegistry(registrySource); + let registry; + try { + registry = await fetchRegistry(registrySource); + } catch (e) { + reportRegistryFetchError(e, { json: options.json }); + } const registryIndex = createRegistryIndex(registry); const { components, missing, notInstalled } = resolveDiffTargets( componentNames, diff --git a/packages/cli/src/commands/doctor.test.ts b/packages/cli/src/commands/doctor.test.ts index ad42624e..3668a622 100644 --- a/packages/cli/src/commands/doctor.test.ts +++ b/packages/cli/src/commands/doctor.test.ts @@ -88,6 +88,42 @@ describe('doctorCommand (integration)', () => { exitSpy.mockRestore(); }); + it('reports an unreachable registry as a failed check instead of crashing (regression: registry.ts used to process.exit directly)', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => {}) as () => never); + const brokenRegistryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-doctor-broken-registry-')); + + doctorCommand.setOptionValue('offline', false); + // Resolving does not throw/reject — this is the actual regression: before + // registry.ts threw a typed error instead of calling process.exit(1) + // itself, this awaited call never returned control to doctor.ts at all, + // so the `catch { fail('Unreachable...') }` below could never run. + await doctorCommand.parseAsync(['--registry', brokenRegistryDir], { from: 'user' }); + + const output = logs.join('\n'); + expect(output).toMatch(/Unreachable/); + expect(exitSpy).toHaveBeenCalledWith(1); + + rmSync(brokenRegistryDir, { recursive: true, force: true }); + exitSpy.mockRestore(); + }); + + it('reports a dangling componentDep in the registry itself as a warning', async () => { + const registryPath = join(registryDir, 'registry.json'); + const registry = JSON.parse(readFileSync(registryPath, 'utf-8')) as { + components: Array<{ name: string; componentDeps?: string[] }>; + }; + registry.components.find((c) => c.name === 'widget')!.componentDeps = ['does-not-exist']; + writeFileSync(registryPath, JSON.stringify(registry, null, 2), 'utf-8'); + + doctorCommand.setOptionValue('offline', false); + doctorCommand.setOptionValue('json', false); + await doctorCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + const output = logs.join('\n'); + expect(output).toMatch(/Registry integrity/); + expect(output).toMatch(/does-not-exist/); + }); + it('reports JSON checks and backfills missing hashes with --fix', async () => { const configPath = join(projectDir, 'sanring.config.json'); const config = JSON.parse(readFileSync(configPath, 'utf-8')) as { diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 74fc53f8..649a83af 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -3,6 +3,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import pc from 'picocolors'; import { createRegistryIndex, fetchRegistry } from '../registry.js'; +import { findRegistryReferenceIssues } from '../registry-integrity.js'; import { getInstalledPackageSpecs, hashContent, @@ -132,6 +133,12 @@ export const doctorCommand = new Command('doctor') const registry = await fetchRegistry(resolveRegistrySource(undefined, config, options.registry)); const index = createRegistryIndex(registry); ok('Reachable'); + // Internal referential integrity of the fetched registry.json itself + // (dangling componentDeps/sharedDeps/group references, unparseable + // peer dependency versions) — distinct from the checks below, which + // compare this project's installed state against the registry. + const integrityIssues = findRegistryReferenceIssues(registry); + for (const issue of integrityIssues) warn(`Registry integrity: ${issue.message}`); if (config?.defaultRegistry && !config.registries?.[config.defaultRegistry]) { warn(`defaultRegistry "${config.defaultRegistry}" is missing from registries`); } diff --git a/packages/cli/src/commands/info.test.ts b/packages/cli/src/commands/info.test.ts new file mode 100644 index 00000000..58ed85c3 --- /dev/null +++ b/packages/cli/src/commands/info.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { getCliVersion, readConfig, writeConfig } from '../utils.js'; +import { writeRegistryFixture } from '../__tests__/registry-fixture.js'; +import { addCommand } from './add.js'; +import { infoCommand } from './info.js'; + +describe('infoCommand (integration)', () => { + let projectDir: string; + let registryDir: string; + let originalCwd: string; + let logs: string[]; + let errors: string[]; + let stdout: string[]; + + beforeEach(() => { + originalCwd = process.cwd(); + projectDir = mkdtempSync(join(tmpdir(), 'sanring-cli-info-project-')); + registryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-info-registry-')); + writeFileSync(join(projectDir, 'angular.json'), '{}', 'utf-8'); + writeRegistryFixture(registryDir, { + utils: 'export function cn() {}\n', + utilsPeerDependencies: { clsx: '^2.0.0' }, + widget: 'export const widget = 1;\n', + }); + process.chdir(projectDir); + + // Commander reuses this module-level Command instance across every + // parseAsync() call in this file and never resets boolean flags back to + // their default on its own (see doctor.test.ts for the same workaround) + // — without this, a `--json` run earlier in the file would leak `true` + // into a later human-readable-output test that never passes `--json`. + infoCommand.setOptionValue('json', false); + infoCommand.setOptionValue('registry', undefined); + infoCommand.setOptionValue('path', undefined); + + logs = []; + errors = []; + stdout = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')); + }); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.join(' ')); + }); + // --json output goes through process.stdout.write, not console.log. + vi.spyOn(process.stdout, 'write').mockImplementation((chunk: unknown) => { + stdout.push(String(chunk)); + return true; + }); + }); + + afterEach(() => { + process.chdir(originalCwd); + rmSync(projectDir, { recursive: true, force: true }); + rmSync(registryDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + describe('project info mode (no argument)', () => { + it('reports --json project info with no components installed', async () => { + await infoCommand.parseAsync(['--json', '--registry', registryDir], { from: 'user' }); + + const report = JSON.parse(stdout.join('')) as { + cli: string; + angular: unknown; + config: unknown; + theme: { present: boolean }; + installed: string[]; + }; + expect(report.cli).toBe(getCliVersion()); + expect(report.angular).not.toBe(false); + expect(report.config).toBeNull(); + expect(report.theme.present).toBe(false); + expect(report.installed).toEqual([]); + }); + + it('reports human-readable project info reflecting installed components', async () => { + await addCommand.parseAsync(['widget', '--registry', registryDir], { from: 'user' }); + logs = []; + + await infoCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + const output = logs.join('\n'); + expect(output).toMatch(/Sanring UI — project info/); + expect(output).toMatch(/Angular.*✔/); + expect(output).toMatch(/sanring\.config\.json/); + expect(output).toMatch(/Installed \(1\).*widget/); + }); + + it('reports --json project info reflecting installed components and config', async () => { + await addCommand.parseAsync(['widget', '--registry', registryDir], { from: 'user' }); + logs = []; + + await infoCommand.parseAsync(['--json', '--registry', registryDir], { from: 'user' }); + + const report = JSON.parse(stdout.join('')) as { installed: string[]; config: { componentPath: string } }; + expect(report.installed).toEqual(['widget']); + expect(report.config.componentPath).toBeTruthy(); + }); + }); + + describe('component info mode', () => { + it('reports --json details for an available, not-yet-installed component', async () => { + await infoCommand.parseAsync(['widget', '--json', '--registry', registryDir], { from: 'user' }); + + const report = JSON.parse(stdout.join('')) as { + name: string; + installed: boolean; + files: string[]; + sharedDeps: string[]; + peerDependencies: Record; + }; + expect(report.name).toBe('widget'); + expect(report.installed).toBe(false); + expect(report.files).toEqual(['widget/index.ts']); + expect(report.sharedDeps).toEqual(['utils']); + // Peer deps roll up transitively from the shared dep. + expect(report.peerDependencies).toEqual({ clsx: '^2.0.0' }); + }); + + it('reports human-readable details and reflects already-installed status', async () => { + await addCommand.parseAsync(['widget', '--registry', registryDir], { from: 'user' }); + logs = []; + + await infoCommand.parseAsync(['widget', '--registry', registryDir], { from: 'user' }); + + const output = logs.join('\n'); + expect(output).toMatch(/widget/); + expect(output).toMatch(/Already installed/); + }); + + it('exits 1 with the list of available components for an unknown component', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit'); + }); + + await expect( + infoCommand.parseAsync(['does-not-exist', '--registry', registryDir], { from: 'user' }), + ).rejects.toThrow('process.exit'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errors.some((e) => e.includes('Component not found: does-not-exist'))).toBe(true); + expect(errors.some((e) => e.includes('widget'))).toBe(true); + + exitSpy.mockRestore(); + }); + + it('resolves an alias:component reference against the aliased registry, not the default', async () => { + const otherRegistryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-info-registry-other-')); + writeRegistryFixture(otherRegistryDir, { widget: 'export const widget = "other";\n' }); + + // No --registry flag override here — resolution must come purely from + // the alias prefix against sanring.config.json's `registries` map. + writeConfig(projectDir, { + componentPath: readConfig(projectDir)?.componentPath ?? 'src/app/components/ui', + registries: { mine: registryDir, other: otherRegistryDir }, + defaultRegistry: 'mine', + }); + + await infoCommand.parseAsync(['other:widget', '--json'], { from: 'user' }); + + const report = JSON.parse(stdout.join('')) as { name: string; sharedDeps: string[] }; + expect(report.name).toBe('widget'); + // The 'other' fixture has no `utils` shared file, unlike the default registry. + expect(report.sharedDeps).toEqual([]); + + rmSync(otherRegistryDir, { recursive: true, force: true }); + }); + }); +}); diff --git a/packages/cli/src/commands/info.ts b/packages/cli/src/commands/info.ts index 412135e4..fbacb740 100644 --- a/packages/cli/src/commands/info.ts +++ b/packages/cli/src/commands/info.ts @@ -9,6 +9,7 @@ import { isAngularProject, getCliVersion, readConfig, + reportRegistryFetchError, resolveComponentBasePath, resolveRegistrySource, } from '../utils.js'; @@ -120,7 +121,13 @@ export const infoCommand = new Command('info') const parsedRef = parseComponentRef(componentName); const bareComponentName = parsedRef.name; const registrySpinner = ora('Loading registry...').start(); - const registry = await fetchRegistry(resolveRegistrySource(parsedRef.alias, config, options.registry)); + let registry; + try { + registry = await fetchRegistry(resolveRegistrySource(parsedRef.alias, config, options.registry)); + } catch (e) { + registrySpinner.stop(); + reportRegistryFetchError(e, { json: options.json }); + } const registryIndex = createRegistryIndex(registry); registrySpinner.stop(); @@ -134,7 +141,7 @@ export const infoCommand = new Command('info') return; } - const component = toInstall.find((c) => c.name === componentName)!; + const component = toInstall.find((c) => c.name === bareComponentName)!; const componentBasePath = resolveComponentBasePath(cwd, options.path, config); const alreadyInstalled = isAngularProject(cwd) && existsSync(join(componentBasePath, component.name)); diff --git a/packages/cli/src/commands/list.test.ts b/packages/cli/src/commands/list.test.ts index 5dfff986..82f03b15 100644 --- a/packages/cli/src/commands/list.test.ts +++ b/packages/cli/src/commands/list.test.ts @@ -26,6 +26,16 @@ describe('listCommand --outdated', () => { await addCommand.parseAsync(['widget', '--registry', registryDir], { from: 'user' }); + // Commander reuses this module-level Command instance across every + // parseAsync() call in this file and never resets boolean/string option + // state back to its default between calls (see doctor.test.ts for the + // same pattern) — without this, e.g. a prior test's `--outdated` or + // `--json` would leak into a later call that doesn't pass those flags. + listCommand.setOptionValue('installed', false); + listCommand.setOptionValue('outdated', false); + listCommand.setOptionValue('json', false); + listCommand.setOptionValue('path', undefined); + logs = []; vi.mocked(console.log).mockImplementation((...args: unknown[]) => { logs.push(args.join(' ')); @@ -86,4 +96,29 @@ describe('listCommand --outdated', () => { true, ); }); + + it('--outdated --json reports the same status as structured output', async () => { + writeRegistryFixture(registryDir, { + utils: 'export function cn() {}\n', + widget: 'export const widget = 2;\n', + }); + + await listCommand.parseAsync(['--outdated', '--json', '--registry', registryDir], { from: 'user' }); + + const report = JSON.parse(logs.join('')) as { + components: Array<{ name: string; status: string }>; + upToDate: number; + outdated: number; + conflicts: number; + }; + expect(report.components).toEqual([expect.objectContaining({ name: 'widget', status: 'outdated' })]); + expect(report).toMatchObject({ upToDate: 0, outdated: 1, conflicts: 0 }); + }); + + it('plain --json listing (no --outdated) reports the available components', async () => { + await listCommand.parseAsync(['--json', '--registry', registryDir], { from: 'user' }); + + const report = JSON.parse(logs.join('')) as { components: Array<{ name: string }> }; + expect(report.components.map((c) => c.name)).toEqual(['widget']); + }); }); diff --git a/packages/cli/src/commands/list.ts b/packages/cli/src/commands/list.ts index 56ce94d8..946bb07b 100644 --- a/packages/cli/src/commands/list.ts +++ b/packages/cli/src/commands/list.ts @@ -14,6 +14,7 @@ import { import { fetchTextTargetsConcurrent, readConfig, + reportRegistryFetchError, requireAngularProject, resolveComponentBasePath, resolveRegistrySource, @@ -188,11 +189,18 @@ export const listCommand = new Command('list') .option('--outdated', 'show installed component update status against the current registry', false) .option('-p, --path ', 'component path relative to cwd (used with --installed)') .option('--registry ', 'custom registry (URL or local path)') - .action(async (options: { installed: boolean; outdated: boolean; path?: string; registry?: string }) => { + .option('--json', 'output machine-readable results (useful for CI and coding agents)', false) + .action(async (options: { installed: boolean; outdated: boolean; path?: string; registry?: string; json: boolean }) => { const config = readConfig(process.cwd()); const registrySource = resolveRegistrySource(undefined, config, options.registry); const spinner = ora('Loading components...').start(); - const registry = await fetchRegistry(registrySource); + let registry; + try { + registry = await fetchRegistry(registrySource); + } catch (e) { + spinner.stop(); + reportRegistryFetchError(e, { json: options.json }); + } const registryIndex = createRegistryIndex(registry); spinner.stop(); @@ -213,10 +221,14 @@ export const listCommand = new Command('list') ? resolve(process.cwd(), config.sharedPath) : join(componentBasePath, 'shared'); - console.log(pc.cyan(`\nInstalled component status`) + pc.dim(` (${components.length})\n`)); + if (!options.json) console.log(pc.cyan(`\nInstalled component status`) + pc.dim(` (${components.length})\n`)); if (components.length === 0) { - console.log(pc.dim(' None installed yet. Run `sanring add ` to get started.\n')); + if (options.json) { + console.log(JSON.stringify({ components: [], upToDate: 0, outdated: 0, conflicts: 0 }, null, 2)); + } else { + console.log(pc.dim(' None installed yet. Run `sanring add ` to get started.\n')); + } return; } @@ -228,11 +240,16 @@ export const listCommand = new Command('list') installedHashes: config?.installedHashes, }); - printOutdatedSummaries(summaries); - const upToDate = summaries.filter((summary) => summary.status === 'up-to-date').length; const outdated = summaries.filter((summary) => summary.status === 'outdated').length; const conflicts = summaries.filter((summary) => summary.status === 'has-conflicts').length; + + if (options.json) { + console.log(JSON.stringify({ components: summaries, upToDate, outdated, conflicts }, null, 2)); + return; + } + + printOutdatedSummaries(summaries); console.log( pc.dim( `\n ${upToDate} up-to-date, ${outdated} outdated, ${conflicts} with conflicts.`, @@ -243,6 +260,11 @@ export const listCommand = new Command('list') } } + if (options.json) { + console.log(JSON.stringify({ components }, null, 2)); + return; + } + const title = options.installed ? 'Installed components' : 'Available components'; console.log(pc.cyan(`\n${title}`) + pc.dim(` (${components.length}${options.installed ? '' : ' total'})\n`)); diff --git a/packages/cli/src/commands/mcp.test.ts b/packages/cli/src/commands/mcp.test.ts index a3a13ab9..05a36a79 100644 --- a/packages/cli/src/commands/mcp.test.ts +++ b/packages/cli/src/commands/mcp.test.ts @@ -1,6 +1,6 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -106,6 +106,55 @@ describe('mcp server', () => { expect(textContent(detailResult)).toContain('clsx@^2.0.0'); }); + it('returns a tool error instead of killing the server when the registry is unreachable (regression: registry.ts used to process.exit directly)', async () => { + const brokenRegistryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-mcp-broken-registry-')); + const server = createMcpServer({ registryUrl: brokenRegistryDir }); + client = new Client({ name: 'sanring-cli-mcp-test', version: '0.0.0' }, { capabilities: {} }); + [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + + // Before this fix, fetchRegistry() called process.exit(1) directly on + // failure, killing this whole long-running MCP server process. Now the + // thrown RegistryFetchError propagates through the SDK's own request + // handler, which converts it into a normal JSON-RPC error response (the + // client surfaces that as a rejected callTool()) instead of the + // connection simply dying with no response at all. + await expect(client.callTool({ name: 'list_components', arguments: {} })).rejects.toThrow( + /Cannot read registry at/, + ); + + // The server process is still alive and can serve a subsequent call — + // the registry fetch failure didn't take the whole process down with it. + // (This second call fails the same way since the registry is still + // broken; what matters is that it gets a clean response at all instead + // of the connection dying with no response, which is what a killed + // server process would look like.) + await expect(client.callTool({ name: 'search_components', arguments: { query: 'x' } })).rejects.toThrow( + /Cannot read registry at/, + ); + + rmSync(brokenRegistryDir, { recursive: true, force: true }); + }); + + it('doctor_project surfaces a dangling componentDep in the registry itself', async () => { + const registryPath = join(registryDir, 'registry.json'); + const registry = JSON.parse(readFileSync(registryPath, 'utf-8')) as { + components: Array<{ name: string; componentDeps?: string[] }>; + }; + registry.components.find((c) => c.name === 'widget')!.componentDeps = ['does-not-exist']; + writeFileSync(registryPath, JSON.stringify(registry, null, 2), 'utf-8'); + + const testClient = await connect(); + const result = await testClient.callTool({ + name: 'doctor_project', + arguments: { cwd: projectDir }, + }); + + const text = textContent(result); + expect(text).toContain('Registry integrity: 1 issue'); + expect(text).toContain('does-not-exist'); + }); + it('plan_component_install returns files and peer deps without modifying project', async () => { const testClient = await connect(); diff --git a/packages/cli/src/commands/mcp.ts b/packages/cli/src/commands/mcp.ts index 1081384c..1788a822 100644 --- a/packages/cli/src/commands/mcp.ts +++ b/packages/cli/src/commands/mcp.ts @@ -17,6 +17,7 @@ import { type Registry, type RegistryComponent, } from '../registry.js'; +import { findRegistryReferenceIssues } from '../registry-integrity.js'; import { isAngularProject, readConfig, resolveComponentBasePath, resolveRegistrySource, semverLte } from '../utils.js'; import { resolveInstallSet, collectPeerDeps } from './add.js'; import { registryRelativePath } from './diff.js'; @@ -342,6 +343,13 @@ export function createMcpServer(options: CreateMcpServerOptions = {}): Server { try { const registry = await getRegistry(); lines.push(`Registry: reachable (${registry.components.length} components)`); + const integrityIssues = findRegistryReferenceIssues(registry); + if (integrityIssues.length > 0) { + lines.push(`Registry integrity: ${integrityIssues.length} issue${integrityIssues.length > 1 ? 's' : ''}:`); + for (const issue of integrityIssues) lines.push(` - ${issue.message}`); + } else { + lines.push('Registry integrity: no dangling references or unparseable peer versions found'); + } } catch (error) { lines.push(`Registry: unreachable (${error instanceof Error ? error.message : String(error)})`); } diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts new file mode 100644 index 00000000..4ecdd4bb --- /dev/null +++ b/packages/cli/src/commands/migrate.test.ts @@ -0,0 +1,198 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Registry } from '../registry.js'; +import { readConfig, writeConfig } from '../utils.js'; +import { writeRegistryFixture } from '../__tests__/registry-fixture.js'; +import { addCommand } from './add.js'; +import { migrateCommand } from './migrate.js'; + +describe('migrateCommand (integration)', () => { + let projectDir: string; + let registryDir: string; + let originalCwd: string; + let logs: string[]; + let errors: string[]; + + beforeEach(async () => { + originalCwd = process.cwd(); + projectDir = mkdtempSync(join(tmpdir(), 'sanring-cli-migrate-project-')); + registryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-migrate-registry-')); + writeFileSync(join(projectDir, 'angular.json'), '{}', 'utf-8'); + writeRegistryFixture(registryDir, { + utils: 'export function cn() {}\n', + widget: 'export const widget = 1;\n', + }); + process.chdir(projectDir); + + logs = []; + errors = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')); + }); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.join(' ')); + }); + + await addCommand.parseAsync(['widget', '--registry', registryDir], { from: 'user' }); + + // Commander reuses this module-level Command instance across every + // parseAsync() call in this file and doesn't reset boolean flags back to + // their default between calls (see doctor.test.ts for the same pattern). + migrateCommand.setOptionValue('check', false); + migrateCommand.setOptionValue('registry', undefined); + + logs = []; + errors = []; + }); + + afterEach(() => { + process.chdir(originalCwd); + rmSync(projectDir, { recursive: true, force: true }); + rmSync(registryDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + function addMigration(fromVersion: string, breaking: boolean, steps: string[]) { + const registryPath = join(registryDir, 'registry.json'); + const registry: Registry = JSON.parse(readFileSync(registryPath, 'utf-8')); + const widget = registry.components.find((c) => c.name === 'widget')!; + widget.migrations = [...(widget.migrations ?? []), { fromVersion, breaking, steps }]; + writeFileSync(registryPath, JSON.stringify(registry, null, 2), 'utf-8'); + } + + it('reports up to date when the installed component has no migrations ahead of it', async () => { + await migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + expect(logs.some((line) => line.includes('up to date'))).toBe(true); + }); + + it('prints migration steps for an installed component behind a breaking migration', async () => { + addMigration('0.1.0', true, ['Rename `foo` input to `bar`.']); + // installedVersions records the CLI version at install time; the fixture + // registry has no version info, so this asserts against whatever the + // real add command recorded, read back from disk. + const config = readConfig(projectDir)!; + writeConfig(projectDir, { + ...config, + installedVersions: { ...config.installedVersions, widget: '0.1.0' }, + }); + + await migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + const output = logs.join('\n'); + expect(output).toMatch(/BREAKING/); + expect(output).toMatch(/Rename `foo` input to `bar`\./); + expect(output).toMatch(/sanring update widget/); + }); + + it('does not surface a migration whose fromVersion is behind the installed version', async () => { + addMigration('0.1.0', true, ['Rename `foo` input to `bar`.']); + const config = readConfig(projectDir)!; + writeConfig(projectDir, { + ...config, + installedVersions: { ...config.installedVersions, widget: '0.2.0' }, + }); + + await migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + expect(logs.some((line) => line.includes('up to date'))).toBe(true); + expect(logs.some((line) => line.includes('BREAKING'))).toBe(false); + }); + + it('--check exits 1 without printing steps when a migration is needed', async () => { + addMigration('0.1.0', true, ['Rename `foo` input to `bar`.']); + const config = readConfig(projectDir)!; + writeConfig(projectDir, { + ...config, + installedVersions: { ...config.installedVersions, widget: '0.1.0' }, + }); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit'); + }); + + await expect( + migrateCommand.parseAsync(['--check', '--registry', registryDir], { from: 'user' }), + ).rejects.toThrow('process.exit'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(logs.some((line) => line.includes('Rename `foo`'))).toBe(false); + + exitSpy.mockRestore(); + }); + + it('--check exits cleanly (no process.exit call) when nothing needs migration', async () => { + const exitSpy = vi.spyOn(process, 'exit'); + + await migrateCommand.parseAsync(['--check', '--registry', registryDir], { from: 'user' }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(logs.some((line) => line.includes('up to date'))).toBe(true); + + exitSpy.mockRestore(); + }); + + it('reports a component no longer present in the registry as not-found and skips it', async () => { + const config = readConfig(projectDir)!; + writeConfig(projectDir, { + ...config, + installedVersions: { ...config.installedVersions, 'removed-component': '0.1.0' }, + }); + + await migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + expect(logs.some((line) => line.includes('removed-component') && line.includes('not found in registry'))).toBe( + true, + ); + }); + + it('resolves an alias:component installedVersions key against the bare registry component name', async () => { + const config = readConfig(projectDir)!; + // Simulate a multi-registry config where installedVersions keys carry an + // alias prefix — migrate must strip it before looking the name up in the + // registry, not treat the whole "alias:name" string as the component name. + writeConfig(projectDir, { + ...config, + registries: { mine: registryDir }, + defaultRegistry: 'mine', + installedVersions: { 'mine:widget': '0.0.0' }, + }); + addMigration('0.1.0', false, ['Some non-breaking cleanup.']); + + await migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + const output = logs.join('\n'); + expect(output).toMatch(/widget/); + expect(output).toMatch(/Some non-breaking cleanup\./); + }); + + it('reports a no-baseline component distinctly from a needs-migration one', async () => { + const config = readConfig(projectDir)!; + const restVersions = { ...config.installedVersions }; + delete restVersions.widget; + writeConfig(projectDir, { ...config, installedVersions: restVersions }); + + await migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }); + + expect(logs.some((line) => line.includes('widget') && line.includes('no installed version baseline'))).toBe( + true, + ); + }); + + it('errors out when sanring.config.json does not exist', async () => { + rmSync(join(projectDir, 'sanring.config.json')); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit'); + }); + + await expect( + migrateCommand.parseAsync(['--registry', registryDir], { from: 'user' }), + ).rejects.toThrow('process.exit'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errors.some((e) => e.includes('sanring.config.json not found'))).toBe(true); + + exitSpy.mockRestore(); + }); +}); diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index 6e838529..12b0fef7 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -5,6 +5,7 @@ import { parseComponentRef } from './add.js'; import { getCliVersion, readConfig, + reportRegistryFetchError, requireAngularProject, resolveRegistrySource, semverLte, @@ -39,7 +40,12 @@ export const migrateCommand = new Command('migrate') return; } - const registry = await fetchRegistry(resolveRegistrySource(undefined, config, options.registry)); + let registry; + try { + registry = await fetchRegistry(resolveRegistrySource(undefined, config, options.registry)); + } catch (e) { + reportRegistryFetchError(e); + } const registryIndex = createRegistryIndex(registry); const currentCliVersion = getCliVersion(); diff --git a/packages/cli/src/commands/remove.test.ts b/packages/cli/src/commands/remove.test.ts index 57a66031..f47a0e8a 100644 --- a/packages/cli/src/commands/remove.test.ts +++ b/packages/cli/src/commands/remove.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import type { Registry, RegistryComponent } from '../registry.js'; @@ -35,9 +35,17 @@ describe('planRemoval', () => { }); it('reports requested-but-not-installed components separately', () => { + const plan = planRemoval(['button', 'combobox'], ['button'], registry); + expect(plan.toRemove).toEqual(['button']); + expect(plan.notInstalled).toEqual(['combobox']); + expect(plan.unknown).toEqual([]); + }); + + it('reports components not present in the registry as unknown, not notInstalled', () => { const plan = planRemoval(['button', 'select'], ['button'], registry); expect(plan.toRemove).toEqual(['button']); - expect(plan.notInstalled).toEqual(['select']); + expect(plan.notInstalled).toEqual([]); + expect(plan.unknown).toEqual(['select']); }); it('blocks removal when a remaining installed component still depends on it', () => { @@ -100,6 +108,44 @@ describe('removeCommand (integration)', () => { expect(config?.installedHashes?.['shared/utils.ts']).toBeDefined(); }); + it('exits 0 when a mixed batch has a known-but-not-installed target alongside a removable one', async () => { + // Register a second component in the registry that's never installed in + // this project, so it's a real (known) target that's just not present. + const registryPath = join(registryDir, 'registry.json'); + const fixtureRegistry: Registry = JSON.parse(readFileSync(registryPath, 'utf-8')); + fixtureRegistry.components.push({ name: 'gizmo', description: '', files: ['gizmo/index.ts'] }); + writeFileSync(registryPath, JSON.stringify(fixtureRegistry, null, 2), 'utf-8'); + + await removeCommand.parseAsync(['widget', 'gizmo', '--registry', registryDir, '--yes'], { + from: 'user', + }); + + expect(readConfig(projectDir)?.installedHashes?.['widget/index.ts']).toBeUndefined(); + }); + + it('exits 1 and removes nothing when the batch includes an unknown component', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit'); + }); + const errors: string[] = []; + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.join(' ')); + }); + + await expect( + removeCommand.parseAsync(['widget', 'does-not-exist', '--registry', registryDir, '--yes'], { + from: 'user', + }), + ).rejects.toThrow('process.exit'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errors.some((e) => e.includes('Unknown component: does-not-exist'))).toBe(true); + // Nothing should have been removed — the unknown-target check runs before any deletion. + expect(readConfig(projectDir)?.installedHashes?.['widget/index.ts']).toBeDefined(); + + exitSpy.mockRestore(); + }); + it('preserves registries/defaultRegistry from the existing config on write', async () => { const existing = readConfig(projectDir)!; writeConfig(projectDir, { diff --git a/packages/cli/src/commands/remove.ts b/packages/cli/src/commands/remove.ts index 50b0d4ab..5c306a7f 100644 --- a/packages/cli/src/commands/remove.ts +++ b/packages/cli/src/commands/remove.ts @@ -12,6 +12,7 @@ import { confirmPrompt, DEFAULT_COMPONENT_PATH, readConfig, + reportRegistryFetchError, requireAngularProject, resolveComponentBasePath, resolveRegistrySource, @@ -22,7 +23,10 @@ import { parseComponentRef } from './add.js'; export interface RemovalPlan { toRemove: string[]; + /** known in the registry, but not currently installed — soft skip */ notInstalled: string[]; + /** not present in the registry at all — hard input error */ + unknown: string[]; /** name being removed -> still-installed component names that depend on it */ blockedBy: Map; /** shared dep names used only by the components being removed */ @@ -56,7 +60,9 @@ export function planRemoval( : createRegistryIndex(registry).componentsByName; const toRemove = requestedNames.filter((n) => installedSet.has(n)); - const notInstalled = requestedNames.filter((n) => !installedSet.has(n)); + const notRequestedInstalled = requestedNames.filter((n) => !installedSet.has(n)); + const notInstalled = notRequestedInstalled.filter((n) => byName.has(n)); + const unknown = notRequestedInstalled.filter((n) => !byName.has(n)); const remaining = installedNames.filter((n) => !toRemove.includes(n)); const blockedBy = new Map(); @@ -81,7 +87,7 @@ export function planRemoval( (dep) => !sharedStillNeeded.has(dep), ); - return { toRemove, notInstalled, blockedBy, possiblyUnusedShared }; + return { toRemove, notInstalled, unknown, blockedBy, possiblyUnusedShared }; } function confirmRemoval(names: string[], yes: boolean): Promise { @@ -117,21 +123,33 @@ export const removeCommand = new Command('remove') const registrySource = resolveRegistrySource(undefined, config, options.registry); const componentBasePath = resolveComponentBasePath(cwd, options.path, config); - const registry = await fetchRegistry(registrySource); + let registry; + try { + registry = await fetchRegistry(registrySource); + } catch (e) { + reportRegistryFetchError(e); + } const registryIndex = createRegistryIndex(registry); const installed = listInstalledComponentNames(componentBasePath, registryIndex); const parsed = componentNames.map(parseComponentRef); const requestedNames = parsed.map((ref) => ref.name); const plan = planRemoval(requestedNames, installed, registryIndex); - if (plan.notInstalled.length > 0) { + if (plan.unknown.length > 0) { console.error( - pc.red(`✖ Not installed, nothing to remove: ${plan.notInstalled.join(', ')}`), + pc.red(`✖ Unknown component${plan.unknown.length > 1 ? 's' : ''}: ${plan.unknown.join(', ')}`), ); + // Unknown targets are input errors, independent of how the rest of the + // batch turns out — matches diff/update's "unknown target always fails". + process.exit(1); + return; + } + + if (plan.notInstalled.length > 0) { + console.log(pc.dim(` Not installed, nothing to remove: ${plan.notInstalled.join(', ')}`)); } if (plan.toRemove.length === 0) { - process.exit(plan.notInstalled.length > 0 ? 1 : 0); return; } diff --git a/packages/cli/src/commands/search.test.ts b/packages/cli/src/commands/search.test.ts new file mode 100644 index 00000000..0fb35dc9 --- /dev/null +++ b/packages/cli/src/commands/search.test.ts @@ -0,0 +1,125 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Registry } from '../registry.js'; +import { writeRegistryFixture } from '../__tests__/registry-fixture.js'; +import { addCommand } from './add.js'; +import { searchCommand } from './search.js'; + +describe('searchCommand (integration)', () => { + let projectDir: string; + let registryDir: string; + let originalCwd: string; + let logs: string[]; + + beforeEach(() => { + originalCwd = process.cwd(); + projectDir = mkdtempSync(join(tmpdir(), 'sanring-cli-search-project-')); + registryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-search-registry-')); + writeFileSync(join(projectDir, 'angular.json'), '{}', 'utf-8'); + writeRegistryFixture(registryDir, { + utils: 'export function cn() {}\n', + widget: 'export const widget = 1;\n', + }); + + // Extend the fixture with more components so ranking/group/tag filters + // have something real to differentiate — the base fixture only has one. + for (const name of ['button', 'buttons-group', 'tooltip']) { + mkdirSync(join(registryDir, 'components', name), { recursive: true }); + writeFileSync(join(registryDir, 'components', name, 'index.ts'), `export const ${name.replace(/-/g, '_')} = 1;\n`, 'utf-8'); + } + const registryPath = join(registryDir, 'registry.json'); + const registry: Registry = JSON.parse(readFileSync(registryPath, 'utf-8')); + registry.components.push( + { name: 'button', description: 'A clickable button', files: ['button/index.ts'], tags: ['form'] }, + { name: 'buttons-group', description: 'Groups multiple buttons', files: ['buttons-group/index.ts'], tags: ['form', 'layout'] }, + { name: 'tooltip', description: 'Shows a hint on hover', files: ['tooltip/index.ts'], tags: ['overlay'] }, + ); + registry.groups = [{ id: 'forms', title: 'Forms', components: ['button', 'buttons-group'] }]; + writeFileSync(registryPath, JSON.stringify(registry, null, 2), 'utf-8'); + + process.chdir(projectDir); + + // Commander reuses this module-level Command instance across every + // parseAsync() call in this file and doesn't reset option state back to + // its default between calls (see doctor.test.ts for the same pattern). + searchCommand.setOptionValue('json', false); + searchCommand.setOptionValue('registry', undefined); + searchCommand.setOptionValue('group', undefined); + searchCommand.setOptionValue('tag', undefined); + searchCommand.setOptionValue('path', undefined); + + logs = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')); + }); + }); + + afterEach(() => { + process.chdir(originalCwd); + rmSync(projectDir, { recursive: true, force: true }); + rmSync(registryDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('ranks an exact name match above a substring match', async () => { + await searchCommand.parseAsync(['button', '--registry', registryDir], { from: 'user' }); + + const output = logs.join('\n'); + const buttonIdx = output.indexOf('button'); + const groupIdx = output.indexOf('buttons-group'); + expect(buttonIdx).toBeGreaterThanOrEqual(0); + expect(groupIdx).toBeGreaterThan(buttonIdx); + }); + + it('reports no matches for a query with no hits', async () => { + await searchCommand.parseAsync(['zzz-nonexistent', '--registry', registryDir], { from: 'user' }); + + expect(logs.some((line) => line.includes('No components matching'))).toBe(true); + }); + + it('--json reports no matches as an empty results array', async () => { + await searchCommand.parseAsync(['zzz-nonexistent', '--json', '--registry', registryDir], { + from: 'user', + }); + + const report = JSON.parse(logs.join('')) as { query: string; results: unknown[] }; + expect(report.query).toBe('zzz-nonexistent'); + expect(report.results).toEqual([]); + }); + + it('--json includes an `installed` flag reflecting the current project state (regression: TDZ crash on installedNames)', async () => { + await addCommand.parseAsync(['button', '--registry', registryDir], { from: 'user' }); + logs = []; + + await searchCommand.parseAsync(['button', '--json', '--registry', registryDir], { from: 'user' }); + + const report = JSON.parse(logs.join('')) as { + results: Array<{ name: string; installed: boolean }>; + }; + const button = report.results.find((r) => r.name === 'button'); + const tooltip = report.results.find((r) => r.name === 'buttons-group'); + expect(button?.installed).toBe(true); + expect(tooltip?.installed).toBe(false); + }); + + it('--group filters results to only the components listed in that group', async () => { + await searchCommand.parseAsync(['e', '--group', 'forms', '--json', '--registry', registryDir], { + from: 'user', + }); + + const report = JSON.parse(logs.join('')) as { results: Array<{ name: string }> }; + const names = report.results.map((r) => r.name).sort(); + expect(names).toEqual(['button', 'buttons-group']); + }); + + it('--tag filters results to components carrying that tag', async () => { + await searchCommand.parseAsync(['e', '--tag', 'overlay', '--json', '--registry', registryDir], { + from: 'user', + }); + + const report = JSON.parse(logs.join('')) as { results: Array<{ name: string }> }; + expect(report.results.map((r) => r.name)).toEqual(['tooltip']); + }); +}); diff --git a/packages/cli/src/commands/search.ts b/packages/cli/src/commands/search.ts index 832ed4a9..47180239 100644 --- a/packages/cli/src/commands/search.ts +++ b/packages/cli/src/commands/search.ts @@ -5,6 +5,7 @@ import { fetchRegistry } from '../registry.js'; import { isAngularProject, readConfig, + reportRegistryFetchError, resolveComponentBasePath, resolveRegistrySource, } from '../utils.js'; @@ -31,7 +32,13 @@ export const searchCommand = new Command('search') .action(async (query: string, options: { path?: string; registry?: string; group?: string; tag?: string; json: boolean }) => { const config = readConfig(process.cwd()); const spinner = ora('Loading components...').start(); - const registry = await fetchRegistry(resolveRegistrySource(undefined, config, options.registry)); + let registry; + try { + registry = await fetchRegistry(resolveRegistrySource(undefined, config, options.registry)); + } catch (e) { + spinner.stop(); + reportRegistryFetchError(e, { json: options.json }); + } spinner.stop(); const q = query.toLowerCase(); diff --git a/packages/cli/src/commands/update.ts b/packages/cli/src/commands/update.ts index 23a61263..8bf100da 100644 --- a/packages/cli/src/commands/update.ts +++ b/packages/cli/src/commands/update.ts @@ -10,6 +10,7 @@ import { isUntouchedSinceInstall, fetchTextTargetsConcurrent, readConfig, + reportRegistryFetchError, requireAngularProject, resolveComponentPath, resolveRegistrySource, @@ -93,7 +94,12 @@ export const updateCommand = new Command('update') ? resolve(cwd, config.sharedPath) : join(componentBasePath, 'shared'); - const registry = await fetchRegistry(registrySource); + let registry; + try { + registry = await fetchRegistry(registrySource); + } catch (e) { + reportRegistryFetchError(e); + } const registryIndex = createRegistryIndex(registry); const { components, missing, notInstalled } = resolveDiffTargets( componentNames, diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 0e3f2abd..7ad468fd 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -35,22 +35,31 @@ ${pc.bold('Quick start')} $ npx @sanring/cli@latest init $ npx @sanring/cli@latest add button +${pc.bold('Command groups')} + ${pc.dim('Install ')} init, add, remove + ${pc.dim('Explore ')} info, list, search + ${pc.dim('Maintain ')} diff, migrate, update, doctor + ${pc.dim('Publish ')} build ${pc.dim('(for registry authors)')} + ${pc.dim('Agent ')} mcp ${pc.dim('(for AI coding agents)')} + ${pc.dim('No installation required — components are copied into your project as source,')} ${pc.dim('not installed as an npm package. Docs: https://ui.sanring.dev')} `, ); +// Registration order matches the "Command groups" summary above and drives +// the order commands are listed in the auto-generated help output. program.addCommand(initCommand); -program.addCommand(listCommand); -program.addCommand(searchCommand); -program.addCommand(infoCommand); program.addCommand(addCommand); program.addCommand(removeCommand); +program.addCommand(infoCommand); +program.addCommand(listCommand); +program.addCommand(searchCommand); program.addCommand(diffCommand); +program.addCommand(migrateCommand); program.addCommand(updateCommand); program.addCommand(doctorCommand); -program.addCommand(mcpCommand); -program.addCommand(migrateCommand); program.addCommand(buildCommand); +program.addCommand(mcpCommand); program.parse(); diff --git a/packages/cli/src/registry-integrity.test.ts b/packages/cli/src/registry-integrity.test.ts new file mode 100644 index 00000000..8d7c45b7 --- /dev/null +++ b/packages/cli/src/registry-integrity.test.ts @@ -0,0 +1,187 @@ +import { describe, expect, it } from 'vitest'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Registry } from './registry.js'; +import { + checkRegistryFilesFetchable, + checkRegistryIntegrity, + findRegistryReferenceIssues, + isParseableVersionRange, +} from './registry-integrity.js'; + +function baseRegistry(overrides: Partial = {}): Registry { + return { + name: 'test', + shared: [{ name: 'utils', description: '', file: 'shared/utils.ts' }], + components: [ + { name: 'badge', description: '', files: ['badge/index.ts'], sharedDeps: ['utils'] }, + { name: 'tag', description: '', files: ['tag/index.ts'], componentDeps: ['badge'] }, + ], + ...overrides, + }; +} + +describe('isParseableVersionRange', () => { + it.each([ + '1.2.3', + '^1.2.3', + '~2.0.0', + '>=1.0.0', + '>=1.0.0 <2.0.0', + '1.0.0 - 2.0.0', + '1.2.3-beta.1', + '1.x', + '*', + 'latest', + 'workspace:*', + 'workspace:^1.0.0', + '1.2.3 || 2.0.0', + ])('accepts %s', (spec) => { + expect(isParseableVersionRange(spec)).toBe(true); + }); + + it.each(['', ' ', 'not-a-version', '1.2.3.4.5', 'abc.def.ghi'])('rejects %s', (spec) => { + expect(isParseableVersionRange(spec)).toBe(false); + }); +}); + +describe('findRegistryReferenceIssues', () => { + it('reports no issues for an internally-consistent registry', () => { + expect(findRegistryReferenceIssues(baseRegistry())).toEqual([]); + }); + + it('flags a componentDep referencing an unknown component', () => { + const registry = baseRegistry({ + components: [{ name: 'tag', description: '', files: ['tag/index.ts'], componentDeps: ['does-not-exist'] }], + }); + const issues = findRegistryReferenceIssues(registry); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'dangling-component-dep', message: expect.stringContaining('does-not-exist') }), + ]); + }); + + it('flags a sharedDep referencing an unknown shared file', () => { + const registry = baseRegistry({ + shared: [], + components: [{ name: 'badge', description: '', files: ['badge/index.ts'], sharedDeps: ['ghost-util'] }], + }); + const issues = findRegistryReferenceIssues(registry); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'dangling-shared-dep', message: expect.stringContaining('ghost-util') }), + ]); + }); + + it('flags a group referencing an unknown component', () => { + const registry = baseRegistry({ + groups: [{ id: 'forms', title: 'Forms', components: ['badge', 'not-real'] }], + }); + const issues = findRegistryReferenceIssues(registry); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'dangling-group-component', message: expect.stringContaining('not-real') }), + ]); + }); + + it('flags an unparseable peer dependency version on a component', () => { + const registry = baseRegistry({ + components: [ + { + name: 'badge', + description: '', + files: ['badge/index.ts'], + peerDependencies: { clsx: 'not-a-version' }, + }, + ], + }); + const issues = findRegistryReferenceIssues(registry); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'unparseable-peer-version', message: expect.stringContaining('clsx') }), + ]); + }); + + it('flags an unparseable peer dependency version on a shared file', () => { + const registry = baseRegistry({ + shared: [{ name: 'utils', description: '', file: 'shared/utils.ts', peerDependencies: { clsx: 'garbage' } }], + }); + const issues = findRegistryReferenceIssues(registry); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'unparseable-peer-version', message: expect.stringContaining('utils') }), + ]); + }); +}); + +describe('checkRegistryFilesFetchable', () => { + let registryDir: string; + + function setup() { + registryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-integrity-')); + mkdirSync(join(registryDir, 'components', 'badge'), { recursive: true }); + mkdirSync(join(registryDir, 'components', 'tag'), { recursive: true }); + mkdirSync(join(registryDir, 'shared'), { recursive: true }); + writeFileSync(join(registryDir, 'components', 'badge', 'index.ts'), 'export const badge = 1;\n', 'utf-8'); + writeFileSync(join(registryDir, 'components', 'tag', 'index.ts'), 'export const tag = 1;\n', 'utf-8'); + writeFileSync(join(registryDir, 'shared', 'utils.ts'), 'export function cn() {}\n', 'utf-8'); + } + + it('reports no issues when every declared file exists at the source', async () => { + setup(); + const issues = await checkRegistryFilesFetchable(baseRegistry(), registryDir); + expect(issues).toEqual([]); + rmSync(registryDir, { recursive: true, force: true }); + }); + + it('flags a component file that is declared but missing from the source', async () => { + setup(); + const registry = baseRegistry({ + components: [{ name: 'badge', description: '', files: ['badge/index.ts', 'badge/missing.ts'] }], + }); + const issues = await checkRegistryFilesFetchable(registry, registryDir); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'unfetchable-file', message: expect.stringContaining('badge/missing.ts') }), + ]); + rmSync(registryDir, { recursive: true, force: true }); + }); + + it('flags a shared file that is declared but missing from the source', async () => { + setup(); + const registry = baseRegistry({ + shared: [{ name: 'ghost', description: '', file: 'shared/ghost.ts' }], + components: [{ name: 'badge', description: '', files: ['badge/index.ts'] }], + }); + const issues = await checkRegistryFilesFetchable(registry, registryDir); + expect(issues).toEqual([ + expect.objectContaining({ kind: 'unfetchable-file', message: expect.stringContaining('shared/ghost') }), + ]); + rmSync(registryDir, { recursive: true, force: true }); + }); +}); + +describe('checkRegistryIntegrity', () => { + it('runs only the sync reference checks when checkFiles is not set', async () => { + const registry = baseRegistry({ + components: [{ name: 'tag', description: '', files: ['tag/nonexistent.ts'], componentDeps: ['ghost'] }], + }); + const issues = await checkRegistryIntegrity(registry); + expect(issues).toEqual([expect.objectContaining({ kind: 'dangling-component-dep' })]); + }); + + it('combines sync reference checks with file-fetchability when checkFiles is set', async () => { + const registryDir = mkdtempSync(join(tmpdir(), 'sanring-cli-integrity-combined-')); + mkdirSync(join(registryDir, 'components', 'badge'), { recursive: true }); + writeFileSync(join(registryDir, 'components', 'badge', 'index.ts'), 'export const badge = 1;\n', 'utf-8'); + + const registry = baseRegistry({ + shared: [], + components: [ + { name: 'badge', description: '', files: ['badge/index.ts'] }, + { name: 'tag', description: '', files: ['tag/missing.ts'], componentDeps: ['ghost'] }, + ], + }); + + const issues = await checkRegistryIntegrity(registry, { source: registryDir, checkFiles: true }); + const kinds = issues.map((issue) => issue.kind).sort(); + expect(kinds).toEqual(['dangling-component-dep', 'unfetchable-file']); + + rmSync(registryDir, { recursive: true, force: true }); + }); +}); diff --git a/packages/cli/src/registry-integrity.ts b/packages/cli/src/registry-integrity.ts new file mode 100644 index 00000000..c132c6cd --- /dev/null +++ b/packages/cli/src/registry-integrity.ts @@ -0,0 +1,139 @@ +// Shared registry-integrity checks, factored out so `doctor`, `build --check`, +// CI, and the MCP server can all validate an *already-parsed* registry.json +// the same way instead of re-implementing overlapping logic. This is +// distinct from `validateRegistry` (schema shape) and `registry-scan.ts` +// (scanning a local source tree during `build`) — this module only looks at +// a `Registry` object's internal references and, optionally, whether the +// files it declares are actually fetchable from its source. +import { fetchFile, type Registry } from './registry.js'; +import { fetchTextTargetsConcurrent } from './utils.js'; + +const FILE_FETCH_CONCURRENCY = 6; + +export interface RegistryIntegrityIssue { + kind: 'dangling-component-dep' | 'dangling-shared-dep' | 'dangling-group-component' | 'unparseable-peer-version' | 'unfetchable-file'; + message: string; +} + +// Deliberately not a full semver-range parser (no bundled semver +// dependency) — this exists to catch obviously-broken strings in a +// hand-written or third-party registry.json (typos, copy-paste mistakes), +// not to validate npm's full range grammar. Hyphen ranges ("1.0.0 - 2.0.0") +// require surrounding whitespace per npm's own syntax, which is what lets +// us tell them apart from a pre-release suffix ("1.0.0-beta.1") that never +// has surrounding whitespace. +const VERSION_TOKEN_RE = + /^(>=|<=|>|<|=|\^|~)?\s*v?(\d+|[xX]|\*)(\.(\d+|[xX]|\*))?(\.(\d+|[xX]|\*))?(-[0-9A-Za-z.]+)?(\+[0-9A-Za-z.]+)?$/; + +export function isParseableVersionRange(spec: string): boolean { + const trimmed = spec.trim(); + if (!trimmed) return false; + if (trimmed === '*' || trimmed === 'latest' || trimmed.startsWith('workspace:')) return true; + return trimmed.split('||').every((alt) => { + const sides = alt.trim().split(/\s+-\s+/); + if (sides.length === 2) return sides.every((side) => VERSION_TOKEN_RE.test(side.trim())); + return alt + .trim() + .split(/\s+/) + .filter(Boolean) + .every((token) => VERSION_TOKEN_RE.test(token)); + }); +} + +// Synchronous checks: dangling componentDeps/sharedDeps/group references and +// unparseable peer dependency version specs. Cheap enough to run on every +// `doctor` invocation and every MCP `doctor_project` call, unlike the +// file-fetchability check below. +export function findRegistryReferenceIssues(registry: Registry): RegistryIntegrityIssue[] { + const issues: RegistryIntegrityIssue[] = []; + const knownComponentNames = new Set(registry.components.map((c) => c.name)); + const knownSharedNames = new Set(registry.shared.map((s) => s.name)); + + for (const component of registry.components) { + for (const dep of component.componentDeps ?? []) { + if (!knownComponentNames.has(dep)) { + issues.push({ + kind: 'dangling-component-dep', + message: `${component.name}: componentDeps references unknown component "${dep}"`, + }); + } + } + for (const dep of component.sharedDeps ?? []) { + if (!knownSharedNames.has(dep)) { + issues.push({ + kind: 'dangling-shared-dep', + message: `${component.name}: sharedDeps references unknown shared file "${dep}"`, + }); + } + } + for (const [pkg, spec] of Object.entries(component.peerDependencies ?? {})) { + if (!isParseableVersionRange(spec)) { + issues.push({ + kind: 'unparseable-peer-version', + message: `${component.name}: peerDependencies["${pkg}"] = "${spec}" is not a parseable version range`, + }); + } + } + } + + for (const shared of registry.shared) { + for (const [pkg, spec] of Object.entries(shared.peerDependencies ?? {})) { + if (!isParseableVersionRange(spec)) { + issues.push({ + kind: 'unparseable-peer-version', + message: `shared/${shared.name}: peerDependencies["${pkg}"] = "${spec}" is not a parseable version range`, + }); + } + } + } + + for (const group of registry.groups ?? []) { + for (const name of group.components) { + if (!knownComponentNames.has(name)) { + issues.push({ + kind: 'dangling-group-component', + message: `group "${group.id}": references unknown component "${name}"`, + }); + } + } + } + + return issues; +} + +// Async, network/disk-bound: confirms every file a registry declares is +// actually fetchable from `source`. Opt-in for callers (e.g. `doctor +// --offline` skips this) since it costs one fetch per declared file. +export async function checkRegistryFilesFetchable( + registry: Registry, + source?: string, +): Promise { + const targets = [ + ...registry.components.flatMap((component) => + component.files.map((file) => ({ label: `${component.name}/${file}`, remotePath: `components/${file}` })), + ), + ...registry.shared.map((shared) => ({ label: `shared/${shared.name}`, remotePath: shared.file })), + ]; + + const results = await fetchTextTargetsConcurrent(targets, FILE_FETCH_CONCURRENCY, (remotePath) => + fetchFile(remotePath, source), + ); + + return results + .filter((result): result is typeof result & { ok: false; error: unknown } => !result.ok) + .map((result) => ({ + kind: 'unfetchable-file' as const, + message: `${result.label}: could not fetch "${result.remotePath}" (${result.error instanceof Error ? result.error.message : String(result.error)})`, + })); +} + +export async function checkRegistryIntegrity( + registry: Registry, + options: { source?: string; checkFiles?: boolean } = {}, +): Promise { + const issues = findRegistryReferenceIssues(registry); + if (options.checkFiles) { + issues.push(...(await checkRegistryFilesFetchable(registry, options.source))); + } + return issues; +} diff --git a/packages/cli/src/registry.test.ts b/packages/cli/src/registry.test.ts index a5618307..d092c1dc 100644 --- a/packages/cli/src/registry.test.ts +++ b/packages/cli/src/registry.test.ts @@ -8,6 +8,7 @@ import { fetchRegistry, installCommand, installCommandParts, + RegistryFetchError, validateRegistry, } from './registry.js'; @@ -149,13 +150,10 @@ describe('fetchRegistry source resolution', () => { expect(fetchMock).not.toHaveBeenCalled(); }); - it('exits when the explicit local directory has no registry.json', async () => { - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('process.exit called'); - }) as never); + it('rejects with a RegistryFetchError when the explicit local directory has no registry.json', async () => { const dir = mkdtempSync(join(tmpdir(), 'sanring-cli-test-')); - await expect(fetchRegistry(dir)).rejects.toThrow('process.exit called'); - exitSpy.mockRestore(); + await expect(fetchRegistry(dir)).rejects.toThrow(RegistryFetchError); + await expect(fetchRegistry(dir)).rejects.toThrow(/Cannot read registry at/); rmSync(dir, { recursive: true, force: true }); }); @@ -173,29 +171,25 @@ describe('fetchRegistry source resolution', () => { expect(fetchMock).toHaveBeenCalledWith('https://example.com/registry/registry.json'); }); - it('exits when the explicit URL request fails', async () => { + it('rejects with a RegistryFetchError when the explicit URL request fails', async () => { fetchMock.mockResolvedValueOnce({ ok: false, status: 404 }); - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('process.exit called'); - }) as never); await expect(fetchRegistry('https://example.com/registry/registry.json')).rejects.toThrow( - 'process.exit called', + RegistryFetchError, + ); + fetchMock.mockResolvedValueOnce({ ok: false, status: 404 }); + await expect(fetchRegistry('https://example.com/registry/registry.json')).rejects.toThrow( + /Cannot fetch registry/, ); - exitSpy.mockRestore(); }); - it('exits when the explicit URL returns an invalid registry', async () => { + it('rejects with a RegistryFetchError when the explicit URL returns an invalid registry', async () => { fetchMock.mockResolvedValueOnce({ ok: true, json: async () => ({ name: 'bad', shared: [], components: [{ name: 'button' }] }), }); - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('process.exit called'); - }) as never); await expect(fetchRegistry('https://example.com/registry/registry.json')).rejects.toThrow( - 'process.exit called', + RegistryFetchError, ); - exitSpy.mockRestore(); }); it('priority 3: falls back to the bundled local registry when no source is given', async () => { diff --git a/packages/cli/src/registry.ts b/packages/cli/src/registry.ts index fa3bf224..22e34e91 100644 --- a/packages/cli/src/registry.ts +++ b/packages/cli/src/registry.ts @@ -75,12 +75,16 @@ function isUrl(s: string): boolean { return s.startsWith('http://') || s.startsWith('https://'); } -function die(message: string, detail?: unknown): never { - console.error(pc.red(`✖ ${message}`)); - if (detail !== undefined) { - console.error(pc.dim(` ${detail instanceof Error ? detail.message : String(detail)}`)); +// Thrown instead of printing + `process.exit(1)` directly, so callers (CLI +// commands, the MCP server) decide how to surface the failure — a one-shot +// CLI command prints and exits, but the long-running MCP server must not be +// killed by a single failed registry fetch, and tests should be able to +// assert against a rejected promise instead of spying on `process.exit`. +export class RegistryFetchError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = 'RegistryFetchError'; } - process.exit(1); } // --------------------------------------------------------------------------- @@ -288,7 +292,7 @@ export async function fetchRegistry(source?: string): Promise { try { return validateRegistry(JSON.parse(readFileSync(localJson, 'utf-8'))); } catch (e) { - die(`Cannot read registry at: ${localJson}`, e); + throw new RegistryFetchError(`Cannot read registry at: ${localJson}`, { cause: e }); } } @@ -321,7 +325,7 @@ async function fetchRegistryFromUrl(url: string): Promise { if (!res.ok) throw new Error(`HTTP ${res.status}`); return validateRegistry(await res.json()); } catch (e) { - die(`Cannot fetch registry: ${url}`, e); + throw new RegistryFetchError(`Cannot fetch registry: ${url}`, { cause: e }); } } diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index 96acca24..3e4140ec 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -99,6 +99,27 @@ export function resolveRegistrySource( return config?.registries?.[targetAlias]; } +// `fetchRegistry`/`fetchFile` throw (RegistryFetchError or a plain fs/fetch +// Error) instead of exiting the process themselves — this is the shared +// command-layer landing spot that turns that into the same "red message + +// exit 1" behavior every command already used to get for free. Only for +// one-shot CLI commands: the MCP server must not exit the whole process on +// one failed fetch, so it lets the SDK convert the thrown error into a +// normal tool-call error response instead of calling this. +export function reportRegistryFetchError(error: unknown, options: { json?: boolean } = {}): never { + const message = error instanceof Error ? error.message : String(error); + if (options.json) { + console.log(JSON.stringify({ ok: false, error: message }, null, 2)); + } else { + console.error(pc.red(`✖ ${message}`)); + const cause = error instanceof Error ? error.cause : undefined; + if (cause !== undefined) { + console.error(pc.dim(` ${cause instanceof Error ? cause.message : String(cause)}`)); + } + } + process.exit(1); +} + // Upgrades legacy `installedVersions` keys (bare component name, from before // multi-registry support) to the `alias:componentName` format, prefixing // with `defaultRegistry`. Not called automatically on every config read — diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 36eddf33..0105870f 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -3,6 +3,20 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { environment: 'node', + // picocolors treats any truthy `CI` env var as color support (it can't tell + // GitHub Actions' log viewer from a dumb pipe), so command output carries + // ANSI codes in CI but not locally — silently breaking literal-text + // assertions (e.g. `.includes('Installed (1)')`) only in CI. Force colors + // off so command output — and these tests — behave the same everywhere. + env: { NO_COLOR: '1' }, include: ['src/**/*.test.ts', 'schematics/**/*.test.ts'], + // Many command tests call process.chdir() to point commands at a temp + // project dir. process.chdir() is process-wide, not per-worker-thread, + // so under the default `threads` pool concurrently-running test files + // race on the real cwd — e.g. registry.test.ts's cwd-relative fixture + // reads intermittently fail when another file has chdir'd elsewhere at + // the same moment. `forks` runs each file in its own OS process, giving + // each an independent cwd and eliminating the race. + pool: 'forks', }, }); diff --git a/packages/ui/src/lib/components/alert-dialog/alert-dialog.component.spec.ts b/packages/ui/src/lib/components/alert-dialog/alert-dialog.component.spec.ts index b0034835..a22292b1 100644 --- a/packages/ui/src/lib/components/alert-dialog/alert-dialog.component.spec.ts +++ b/packages/ui/src/lib/components/alert-dialog/alert-dialog.component.spec.ts @@ -46,11 +46,18 @@ import { DialogTitleDirective } from '../dialog/dialog-title.directive'; + + + + + + `, }) class AlertDialogTestHost { @ViewChild('alertDialog') readonly alertDialog!: TemplateRef; @ViewChild('customResultAlertDialog') readonly customResultAlertDialog!: TemplateRef; + @ViewChild('untitledAlertDialog') readonly untitledAlertDialog!: TemplateRef; readonly alertDialogService = inject(AlertDialogService); } @@ -88,6 +95,23 @@ describe('AlertDialog', () => { expect(overlayElement.querySelector('button[aria-label="關閉對話框"]')).toBeNull(); }); + it('gives an untitled alert dialog a default accessible name', () => { + const fixture = TestBed.createComponent(AlertDialogTestHost); + fixture.detectChanges(); + + fixture.componentInstance.alertDialogService.open( + fixture.componentInstance.untitledAlertDialog, + ); + fixture.detectChanges(); + + const dialogContainer = overlayContainer + .getContainerElement() + .querySelector('cdk-dialog-container'); + + expect(dialogContainer?.getAttribute('aria-label')).toBe('Alert dialog'); + expect(dialogContainer?.hasAttribute('aria-labelledby')).toBe(false); + }); + it('opens via sanringAlertDialogTrigger and locks disableClose even if the trigger config tries to unset it', () => { const fixture = TestBed.createComponent(AlertDialogTestHost); fixture.detectChanges(); @@ -105,7 +129,9 @@ describe('AlertDialog', () => { backdrop.click(); fixture.detectChanges(); - expect(overlayContainer.getContainerElement().querySelector('cdk-dialog-container')).not.toBeNull(); + expect( + overlayContainer.getContainerElement().querySelector('cdk-dialog-container'), + ).not.toBeNull(); }); it('does not close on backdrop click or Escape', () => { @@ -127,7 +153,9 @@ describe('AlertDialog', () => { ?.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); fixture.detectChanges(); - expect(overlayContainer.getContainerElement().querySelector('cdk-dialog-container')).not.toBeNull(); + expect( + overlayContainer.getContainerElement().querySelector('cdk-dialog-container'), + ).not.toBeNull(); expect(ref.disableClose).toBe(true); }); @@ -158,9 +186,7 @@ describe('AlertDialog', () => { // 這裡是 property binding(非 bare attribute),Angular 不會把它反映成 DOM // attribute,所以改用文字內容找按鈕,而不是 attribute selector。 - const buttons = Array.from( - overlayContainer.getContainerElement().querySelectorAll('button'), - ); + const buttons = Array.from(overlayContainer.getContainerElement().querySelectorAll('button')); buttons.find((button) => button.textContent?.includes('Delete'))?.click(); fixture.detectChanges(); @@ -216,7 +242,9 @@ describe('AlertDialog', () => { fixture.componentInstance.alertDialogService.open(fixture.componentInstance.alertDialog); fixture.detectChanges(); - const content = overlayContainer.getContainerElement().querySelector('sanring-alert-dialog-content'); + const content = overlayContainer + .getContainerElement() + .querySelector('sanring-alert-dialog-content'); expect(content?.classList.contains('custom-alert-class')).toBe(true); }); @@ -229,7 +257,9 @@ describe('AlertDialog', () => { const trigger = fixture.nativeElement.querySelector('button') as HTMLElement; trigger.focus(); - const ref = fixture.componentInstance.alertDialogService.open(fixture.componentInstance.alertDialog); + const ref = fixture.componentInstance.alertDialogService.open( + fixture.componentInstance.alertDialog, + ); fixture.detectChanges(); await fixture.whenStable(); fixture.detectChanges(); diff --git a/packages/ui/src/lib/components/avatar/avatar-group-count.component.ts b/packages/ui/src/lib/components/avatar/avatar-group-count.component.ts index e2692b31..72ab4d9b 100644 --- a/packages/ui/src/lib/components/avatar/avatar-group-count.component.ts +++ b/packages/ui/src/lib/components/avatar/avatar-group-count.component.ts @@ -1,4 +1,11 @@ -import { ChangeDetectionStrategy, Component, computed, input, output } from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + booleanAttribute, + computed, + input, + output, +} from '@angular/core'; import { coerceNumberProperty } from '@angular/cdk/coercion'; import { cn } from '../../utils'; import { AvatarSize } from './avatar.types'; @@ -18,7 +25,9 @@ const SIZE_CLASSES: Record = { '[class]': 'countClass()', '[attr.aria-label]': 'ariaLabel()', '[attr.role]': 'clickable() ? "button" : null', - '[attr.tabindex]': 'clickable() ? "0" : null', + '[attr.aria-disabled]': 'clickable() && disabled() ? "true" : null', + '[attr.tabindex]': 'clickable() ? (disabled() ? "-1" : "0") : null', + '[attr.data-disabled]': 'disabled() || null', '(click)': 'handleClick()', '(keydown.enter)': 'handleKeyActivation($event)', '(keydown.space)': 'handleKeyActivation($event)', @@ -34,7 +43,8 @@ export class AvatarGroupCountComponent { }); readonly size = input('md'); readonly ariaLabel = input(); - readonly clickable = input(false); + readonly clickable = input(false, { transform: booleanAttribute }); + readonly disabled = input(false, { transform: booleanAttribute }); readonly clicked = output(); @@ -49,17 +59,18 @@ export class AvatarGroupCountComponent { 'bg-[var(--sanring-surface-strong)] text-[var(--sanring-foreground)]', 'ring-2 ring-[var(--sanring-background)]', SIZE_CLASSES[this.size()] ?? SIZE_CLASSES.md, - this.clickable() && 'cursor-pointer', + this.clickable() && !this.disabled() && 'cursor-pointer', + this.clickable() && this.disabled() && 'cursor-not-allowed opacity-50', this.class(), ), ); protected handleClick(): void { - if (this.clickable()) this.clicked.emit(); + if (this.clickable() && !this.disabled()) this.clicked.emit(); } protected handleKeyActivation(event: Event): void { - if (!this.clickable()) return; + if (!this.clickable() || this.disabled()) return; event.preventDefault(); // Space 防捲頁,Enter 防 form submit this.clicked.emit(); } diff --git a/packages/ui/src/lib/components/avatar/avatar.component.spec.ts b/packages/ui/src/lib/components/avatar/avatar.component.spec.ts index 060e4f61..5d859076 100644 --- a/packages/ui/src/lib/components/avatar/avatar.component.spec.ts +++ b/packages/ui/src/lib/components/avatar/avatar.component.spec.ts @@ -29,7 +29,8 @@ import { AvatarComponent } from './avatar.component'; @@ -38,6 +39,7 @@ import { AvatarComponent } from './avatar.component'; }) class AvatarTestHost { clicks = 0; + countDisabled = false; } describe('AvatarComponent', () => { @@ -80,6 +82,24 @@ describe('AvatarComponent', () => { expect(fixture.componentInstance.clicks).toBe(1); }); + it('exposes disabled semantics and blocks pointer and keyboard activation', () => { + const fixture = TestBed.createComponent(AvatarTestHost); + fixture.componentInstance.countDisabled = true; + fixture.detectChanges(); + + const count = fixture.nativeElement.querySelector('sanring-avatar-group-count') as HTMLElement; + expect(count.getAttribute('aria-disabled')).toBe('true'); + expect(count.getAttribute('tabindex')).toBe('-1'); + expect(count.getAttribute('data-disabled')).toBe('true'); + + count.click(); + count.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + count.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', bubbles: true })); + fixture.detectChanges(); + + expect(fixture.componentInstance.clicks).toBe(0); + }); + it('has no axe-detectable a11y violations', async () => { const fixture = TestBed.createComponent(AvatarTestHost); fixture.detectChanges(); diff --git a/packages/ui/src/lib/components/breadcrumb/breadcrumb-link.component.ts b/packages/ui/src/lib/components/breadcrumb/breadcrumb-link.component.ts index 6d5a7611..7fa102e1 100644 --- a/packages/ui/src/lib/components/breadcrumb/breadcrumb-link.component.ts +++ b/packages/ui/src/lib/components/breadcrumb/breadcrumb-link.component.ts @@ -11,10 +11,7 @@ import { cn } from '../../utils'; '[class]': 'breadcrumbLinkClass()', }, template: ` -
+ `, diff --git a/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.spec.ts b/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.spec.ts index fe822de2..955dde2d 100644 --- a/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.spec.ts +++ b/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.spec.ts @@ -22,10 +22,12 @@ import { BreadcrumbComponent } from './breadcrumb.component'; BreadcrumbEllipsisComponent, ], template: ` - + - Docs + Docs @@ -55,7 +57,7 @@ describe('BreadcrumbComponent', () => { const items = fixture.nativeElement.querySelectorAll('sanring-breadcrumb-item'); expect(breadcrumb.getAttribute('role')).toBe('navigation'); - expect(breadcrumb.getAttribute('aria-label')).toBe('breadcrumb'); + expect(breadcrumb.getAttribute('aria-label')).toBe('導覽路徑'); expect(list.getAttribute('role')).toBe('list'); expect(items[0].getAttribute('role')).toBe('listitem'); }); @@ -65,8 +67,12 @@ describe('BreadcrumbComponent', () => { fixture.detectChanges(); const page = fixture.nativeElement.querySelector('sanring-breadcrumb-page') as HTMLElement; - const divider = fixture.nativeElement.querySelector('sanring-breadcrumb-divider') as HTMLElement; - const ellipsis = fixture.nativeElement.querySelector('sanring-breadcrumb-ellipsis') as HTMLElement; + const divider = fixture.nativeElement.querySelector( + 'sanring-breadcrumb-divider', + ) as HTMLElement; + const ellipsis = fixture.nativeElement.querySelector( + 'sanring-breadcrumb-ellipsis', + ) as HTMLElement; expect(page.getAttribute('aria-current')).toBe('page'); expect(page.getAttribute('aria-disabled')).toBe('true'); diff --git a/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.ts b/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.ts index 6f972a1f..043760df 100644 --- a/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.ts +++ b/packages/ui/src/lib/components/breadcrumb/breadcrumb.component.ts @@ -7,13 +7,14 @@ import { cn } from '../../utils'; changeDetection: ChangeDetectionStrategy.OnPush, host: { role: 'navigation', - 'aria-label': 'breadcrumb', + '[attr.aria-label]': 'ariaLabel()', '[class]': 'breadcrumbClass()', }, template: ``, }) export class BreadcrumbComponent { readonly class = input(); + readonly ariaLabel = input('breadcrumb'); protected readonly breadcrumbClass = computed(() => cn('block', this.class())); } diff --git a/packages/ui/src/lib/components/calendar/calendar.component.ts b/packages/ui/src/lib/components/calendar/calendar.component.ts index 59e42791..1b915409 100644 --- a/packages/ui/src/lib/components/calendar/calendar.component.ts +++ b/packages/ui/src/lib/components/calendar/calendar.component.ts @@ -1,10 +1,7 @@ import { ChangeDetectionStrategy, Component, - DestroyRef, ElementRef, - Injector, - OnInit, booleanAttribute, computed, effect, @@ -12,11 +9,9 @@ import { inject, input, output, - signal, } from '@angular/core'; import { _IdGenerator } from '@angular/cdk/a11y'; -import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; -import { ControlValueAccessor, NG_VALUE_ACCESSOR, NgControl, Validators } from '@angular/forms'; +import { NG_VALUE_ACCESSOR } from '@angular/forms'; import { CALENDAR_LOCALE, CalendarDay, @@ -27,11 +22,11 @@ import { DisabledInput, } from '@sanring/date-picker-core'; import { LucideChevronDown } from '@lucide/angular'; -import { Observable, Subject } from 'rxjs'; import { cn } from '../../utils'; -import { FieldType, SanringFieldControl, SANRING_FIELD_CONTROL } from '../field/field.type'; +import { FieldType, SANRING_FIELD_CONTROL } from '../field/field.type'; import { PopoverComponent } from '../popover/popover.component'; import { PopoverContentComponent } from '../popover/popover-content.component'; +import { SanringCvaBase, SanringFieldControlAdapter } from '../shared/cva-base'; import { CalendarDayDirective } from './calendar-day.directive'; import { CalendarHeaderComponent } from './calendar-header.component'; import { CALENDAR_WEEKDAY_TEXT_CLASS } from './calendar.styles'; @@ -65,7 +60,8 @@ const JUMP_YEAR_RANGE_FUTURE = 50; // adapter class translates between the two, same pattern as Checkbox/Combobox. { provide: SANRING_FIELD_CONTROL, - useFactory: (host: CalendarComponent) => new CalendarFieldControlAdapter(host), + useFactory: (host: CalendarComponent) => + new SanringFieldControlAdapter(FieldType.calendar, host), deps: [forwardRef(() => CalendarComponent)], }, ], @@ -177,9 +173,10 @@ const JUMP_YEAR_RANGE_FUTURE = 50; `, }) -export class CalendarComponent implements ControlValueAccessor, OnInit { +export class CalendarComponent extends SanringCvaBase { protected readonly engine = inject(CalendarEngine); private readonly injectedLocale = inject(CALENDAR_LOCALE, { optional: true }); + private readonly elementRef = inject(ElementRef); readonly class = input(); readonly id = input(inject(_IdGenerator).getId('sanring-calendar-', true)); @@ -226,43 +223,15 @@ export class CalendarComponent implements ControlValueAccessor, OnInit { ]; }); - // ========================================== - // Field 整合:id/disabled/required 會跟上面同名的 @Input 撞名,走下面的 fieldXxx getter, - // 由 CalendarFieldControlAdapter 轉接成 SanringFieldControl 介面(見檔案底部)。 - // ========================================== - focused = false; - ngControl: NgControl | null = null; - - private readonly injector = inject(Injector); - private readonly destroyRef = inject(DestroyRef); - private readonly elementRef = inject(ElementRef); - - private readonly stateChangesSubject = new Subject(); - readonly stateChanges = this.stateChangesSubject.asObservable(); - - // 橋接用:ngControl 的 invalid/touched 是 RxJS 驅動、不是 signal,靠這個計數器把它們接進 - // signal graph,errorState/fieldRequired 才能在驗證狀態改變時正確重算。 - private readonly stateVersion = signal(0); - private readonly fieldDescribedByIds = signal([]); - private readonly disabledState = signal(false); - - protected readonly computedAriaDescribedBy = computed(() => { - const ids = [this.ariaDescribedBy(), ...this.fieldDescribedByIds()].filter( - (v): v is string => !!v, - ); - return ids.length ? ids.join(' ') : undefined; - }); - // 表單層級的「整個控制項停用」跟既有的 disabled(哪些日期不可選)是兩件事——停用時額外疊一個 // 永遠回傳 true 的 matcher,讓所有日期都不可選,而不是動到使用者自己傳入的 disabled matcher。 private readonly effectiveDisabled = computed(() => this.disabledState() ? () => true : this.disabled(), ); - get errorState(): boolean { - this.stateVersion(); - return !!(this.ngControl?.invalid && this.ngControl?.touched); - } + protected readonly computedAriaDescribedBy = this.makeComputedAriaDescribedBy( + this.ariaDescribedBy, + ); get fieldValue(): CalendarValue { return this.mode() === 'range' ? this.engine.selectedRange() : this.engine.selectedDate(); @@ -277,15 +246,12 @@ export class CalendarComponent implements ControlValueAccessor, OnInit { return this.disabledState(); } - get fieldRequired(): boolean { - this.stateVersion(); - return this.required() || !!this.ngControl?.control?.hasValidator(Validators.required); + protected override hasInputRequired(): boolean { + return this.required(); } - private onChange: (value: CalendarValue) => void = () => {}; - private onTouched: () => void = () => {}; - constructor() { + super(); effect(() => { const locale = this.locale(); if (locale) this.engine.setLocale(locale); @@ -310,17 +276,6 @@ export class CalendarComponent implements ControlValueAccessor, OnInit { this.emitStateChanges(); } }); - - this.destroyRef.onDestroy(() => this.stateChangesSubject.complete()); - } - - ngOnInit(): void { - // 跟 checkbox/select 一樣的原因:constructor 階段 self-inject NgControl 會跟 NgModel 搭配時 - // 觸發 NG0200 循環依賴(本元件同時透過 NG_VALUE_ACCESSOR 註冊自己),延後到 ngOnInit 才拿。 - this.ngControl = this.injector.get(NgControl, null, { optional: true, self: true }); - this.ngControl?.control?.events - ?.pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => this.emitStateChanges()); } readonly isDraftActive = computed(() => this.engine.isDraftActive()); @@ -383,26 +338,11 @@ export class CalendarComponent implements ControlValueAccessor, OnInit { return rows; } - protected onFocus(): void { - this.focused = true; - this.emitStateChanges(); - } - - protected onBlur(): void { - this.focused = false; - this.onTouched(); - this.emitStateChanges(); - } - focus(options?: FocusOptions): void { this.elementRef.nativeElement.focus(options); } - setDescribedByIds(ids: string[]): void { - this.fieldDescribedByIds.set(ids); - } - - writeValue(value: CalendarValue): void { + override writeValue(value: CalendarValue): void { if (this.mode() === 'range') { if (value && typeof value === 'object' && 'start' in value) { const range = value as DateRange; @@ -418,72 +358,4 @@ export class CalendarComponent implements ControlValueAccessor, OnInit { this.engine.clearSelection(); } } - - registerOnChange(fn: (value: CalendarValue) => void): void { - this.onChange = fn; - } - - registerOnTouched(fn: () => void): void { - this.onTouched = fn; - } - - setDisabledState(isDisabled: boolean): void { - this.disabledState.set(isDisabled); - this.emitStateChanges(); - } - - private emitStateChanges(): void { - this.stateVersion.update((v) => v + 1); - this.stateChangesSubject.next(); - } -} - -class CalendarFieldControlAdapter implements SanringFieldControl { - readonly controlType = FieldType.calendar; - - constructor(private readonly host: CalendarComponent) {} - - get id(): string { - return this.host.id(); - } - - get value(): CalendarValue { - return this.host.fieldValue; - } - - get empty(): boolean { - return this.host.fieldEmpty; - } - - get focused(): boolean { - return this.host.focused; - } - - get errorState(): boolean { - return this.host.errorState; - } - - get disabled(): boolean { - return this.host.fieldDisabled; - } - - get required(): boolean { - return this.host.fieldRequired; - } - - get ngControl(): NgControl | null { - return this.host.ngControl; - } - - get stateChanges(): Observable { - return this.host.stateChanges; - } - - focus(options?: FocusOptions): void { - this.host.focus(options); - } - - setDescribedByIds(ids: string[]): void { - this.host.setDescribedByIds(ids); - } } diff --git a/packages/ui/src/lib/components/checkbox/checkbox.component.ts b/packages/ui/src/lib/components/checkbox/checkbox.component.ts index 54dfce58..656e6370 100644 --- a/packages/ui/src/lib/components/checkbox/checkbox.component.ts +++ b/packages/ui/src/lib/components/checkbox/checkbox.component.ts @@ -1,10 +1,7 @@ import { ChangeDetectionStrategy, Component, - DestroyRef, ElementRef, - Injector, - OnInit, ViewChild, booleanAttribute, computed, @@ -16,14 +13,17 @@ import { signal, } from '@angular/core'; import { _IdGenerator } from '@angular/cdk/a11y'; -import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; -import { ControlValueAccessor, NG_VALUE_ACCESSOR, NgControl, Validators } from '@angular/forms'; +import { NG_VALUE_ACCESSOR } from '@angular/forms'; import { LucideCheck, LucideMinus } from '@lucide/angular'; -import { Observable, Subject } from 'rxjs'; import { cn } from '../../utils'; import { SELECTION_CONTROL_BASE_CLASS, SELECTION_CONTROL_FOCUS_CLASS } from '../component-styles'; -import { FieldType, SanringFieldControl, SANRING_FIELD_CONTROL } from '../field/field.type'; -import { CHECKBOX_ICON_SIZE_CLASSES, CHECKBOX_SIZE_CLASSES, CHECKBOX_STATE_CLASS } from './checkbox.styles'; +import { SanringCvaBase, SanringFieldControlAdapter } from '../shared/cva-base'; +import { FieldType, SANRING_FIELD_CONTROL } from '../field/field.type'; +import { + CHECKBOX_ICON_SIZE_CLASSES, + CHECKBOX_SIZE_CLASSES, + CHECKBOX_STATE_CLASS, +} from './checkbox.styles'; import { CheckedState, CheckboxSize } from './checkbox.types'; @Component({ @@ -42,7 +42,8 @@ import { CheckedState, CheckboxSize } from './checkbox.types'; // 禁止用 alias 改名 @Input),所以改用 useFactory 產生一個轉接的 adapter 物件。 { provide: SANRING_FIELD_CONTROL, - useFactory: (host: CheckboxComponent) => new CheckboxFieldControlAdapter(host), + useFactory: (host: CheckboxComponent) => + new SanringFieldControlAdapter(FieldType.checkbox, host), deps: [forwardRef(() => CheckboxComponent)], }, ], @@ -83,7 +84,7 @@ import { CheckedState, CheckboxSize } from './checkbox.types'; `, }) -export class CheckboxComponent implements ControlValueAccessor, OnInit { +export class CheckboxComponent extends SanringCvaBase { readonly class = input(); readonly id = input(inject(_IdGenerator).getId('sanring-checkbox-', true)); readonly disabled = input(false, { transform: booleanAttribute }); @@ -111,44 +112,17 @@ export class CheckboxComponent implements ControlValueAccessor, OnInit { CHECKBOX_STATE_CLASS, // 讀 this.errorState(getter)而不是直接寫條件,是為了讓下面 stateVersion 的橋接生效, // 否則 ngControl.invalid/touched 不是 signal,這個 computed 不會在驗證狀態改變時重算 - this.errorState && 'border-[var(--sanring-error-50)] focus-visible:ring-[var(--sanring-error-40)]', + this.errorState && + 'border-[var(--sanring-error-50)] focus-visible:ring-[var(--sanring-error-40)]', this.class(), ), ); - // ========================================== - // Field 整合:底下這些成員都不會跟上面的 @Input 撞名,可以直接放在元件本身; - // 真正會撞名的 (id/disabled/value/required) 走下面的 fieldXxx getter,由 - // CheckboxFieldControlAdapter 轉接成 SanringFieldControl 介面。 - // ========================================== - readonly controlType = FieldType.checkbox; - focused = false; - ngControl: NgControl | null = null; - - private readonly injector = inject(Injector); - private readonly destroyRef = inject(DestroyRef); - @ViewChild('btn') private btnRef!: ElementRef; - private readonly stateChangesSubject = new Subject(); - readonly stateChanges = this.stateChangesSubject.asObservable(); - - // 橋接用:ngControl 的 invalid/touched 是 RxJS 驅動、不是 signal,靠這個計數器把它們 - // 接進 signal graph,errorState/fieldRequired 才能讓上面的 checkboxClass computed 正確重算 - private readonly stateVersion = signal(0); - - private readonly fieldDescribedByIds = signal([]); - protected readonly computedAriaDescribedBy = computed(() => { - const ids = [this.ariaDescribedBy(), ...this.fieldDescribedByIds()].filter( - (v): v is string => !!v, - ); - return ids.length ? ids.join(' ') : undefined; - }); - - get errorState(): boolean { - this.stateVersion(); - return !!(this.ngControl?.invalid && this.ngControl?.touched); - } + protected readonly computedAriaDescribedBy = this.makeComputedAriaDescribedBy( + this.ariaDescribedBy, + ); get fieldValue(): CheckedState | null { return this.checkedSignal(); @@ -162,40 +136,15 @@ export class CheckboxComponent implements ControlValueAccessor, OnInit { return this.isDisabled(); } - get fieldRequired(): boolean { - this.stateVersion(); - return this.required() || !!this.ngControl?.control?.hasValidator(Validators.required); + protected override hasInputRequired(): boolean { + return this.required(); } - private readonly disabledState = signal(false); - private onChange: (value: CheckedState) => void = () => {}; - private onTouched: () => void = () => {}; - constructor() { + super(); effect(() => { this.checkedSignal.set(this.checked()); }); - - this.destroyRef.onDestroy(() => this.stateChangesSubject.complete()); - } - - ngOnInit(): void { - // 不能像 input/textarea 直接用 `inject(NgControl, { optional: true, self: true })` field - // initializer:本元件同時透過 NG_VALUE_ACCESSOR (forwardRef) 註冊自己,若在 constructor - // 階段就 self-inject NgControl,跟 NgModel 搭配時會觸發 NG0200 循環依賴(NgModel 建構時 - // 需要先解出 value accessor 也就是自己,自己建構時又反過來要拿同一個還沒建構完的 NgModel)。 - // 延後到 ngOnInit 拿,因為 Angular 會先讓同一個節點上的所有 directive 建構完才跑 lifecycle hook。 - this.ngControl = this.injector.get(NgControl, null, { optional: true, self: true }); - - // OnPush 元件被跳過 CD 時 ngDoCheck 不會執行,所以不能像 input/textarea 靠輪詢偵測 - // ngControl 狀態變化。不能只聽 statusChanges——那個 Observable 只在 valid/invalid/ - // pending/disabled 這幾種 status 真的變動時才會 emit,markAsTouched() 純粹改 touched - // flag,不會觸發它,導致外部呼叫 markAllAsTouched() 時錯誤訊息不會跳出來。改聽 - // control.events(Angular v18+ 公開 API),touched/pristine/status/value 任何一種 - // 變化都會經過這裡。 - this.ngControl?.control?.events - ?.pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => this.emitStateChanges()); } getState(): string { @@ -212,94 +161,11 @@ export class CheckboxComponent implements ControlValueAccessor, OnInit { this.emitStateChanges(); } - onFocus() { - this.focused = true; - this.emitStateChanges(); - } - - onBlur() { - this.focused = false; - this.onTouched(); - this.emitStateChanges(); - } - focus(options?: FocusOptions): void { this.btnRef?.nativeElement.focus(options); } - setDescribedByIds(ids: string[]): void { - this.fieldDescribedByIds.set(ids); - } - - writeValue(value: CheckedState): void { + override writeValue(value: CheckedState): void { this.checkedSignal.set(value); } - - registerOnChange(fn: (value: CheckedState) => void): void { - this.onChange = fn; - } - - registerOnTouched(fn: () => void): void { - this.onTouched = fn; - } - - setDisabledState(isDisabled: boolean): void { - this.disabledState.set(isDisabled); - this.emitStateChanges(); - } - - private emitStateChanges(): void { - this.stateVersion.update((v) => v + 1); - this.stateChangesSubject.next(); - } -} - -class CheckboxFieldControlAdapter implements SanringFieldControl { - readonly controlType = FieldType.checkbox; - - constructor(private readonly host: CheckboxComponent) {} - - get id(): string { - return this.host.id(); - } - - get value(): CheckedState | null { - return this.host.fieldValue; - } - - get empty(): boolean { - return this.host.fieldEmpty; - } - - get focused(): boolean { - return this.host.focused; - } - - get errorState(): boolean { - return this.host.errorState; - } - - get disabled(): boolean { - return this.host.fieldDisabled; - } - - get required(): boolean { - return this.host.fieldRequired; - } - - get ngControl(): NgControl | null { - return this.host.ngControl; - } - - get stateChanges(): Observable { - return this.host.stateChanges; - } - - focus(options?: FocusOptions): void { - this.host.focus(options); - } - - setDescribedByIds(ids: string[]): void { - this.host.setDescribedByIds(ids); - } } diff --git a/packages/ui/src/lib/components/combobox/combobox-chip-input.component.ts b/packages/ui/src/lib/components/combobox/combobox-chip-input.component.ts index 911152dd..983611d5 100644 --- a/packages/ui/src/lib/components/combobox/combobox-chip-input.component.ts +++ b/packages/ui/src/lib/components/combobox/combobox-chip-input.component.ts @@ -1,4 +1,11 @@ -import { ChangeDetectionStrategy, Component, computed, ElementRef, inject, input } from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + computed, + ElementRef, + inject, + input, +} from '@angular/core'; import { ComboboxComponent } from './combobox.component'; import { cn } from '../../utils'; diff --git a/packages/ui/src/lib/components/combobox/combobox-content.component.ts b/packages/ui/src/lib/components/combobox/combobox-content.component.ts index 002ac9ef..ced3b883 100644 --- a/packages/ui/src/lib/components/combobox/combobox-content.component.ts +++ b/packages/ui/src/lib/components/combobox/combobox-content.component.ts @@ -60,19 +60,24 @@ export class ComboboxContentComponent { effect(() => { if (!this.combobox.isOpen()) return; afterNextRender( - () => this.elementRef.nativeElement.querySelector('input[role="combobox"]')?.focus(), + () => + this.elementRef.nativeElement + .querySelector('input[role="combobox"]') + ?.focus(), { injector: this.injector }, ); }); } protected close(): void { - this.combobox.toggleOpen(false); + this.combobox.closeAndRestoreFocus(); } @HostListener('document:pointerdown', ['$event']) protected handleDocumentPointerDown(event: PointerEvent): void { if (!this.combobox.isOpen() || this.combobox.containsElement(event.target)) return; - this.close(); + // A pointer interaction is choosing a new focus target. Close without scheduling a focus + // restore, otherwise the post-render callback would steal focus back from that target. + this.combobox.toggleOpen(false); } } diff --git a/packages/ui/src/lib/components/combobox/combobox-empty.component.ts b/packages/ui/src/lib/components/combobox/combobox-empty.component.ts index 0735ca88..719c77b3 100644 --- a/packages/ui/src/lib/components/combobox/combobox-empty.component.ts +++ b/packages/ui/src/lib/components/combobox/combobox-empty.component.ts @@ -21,7 +21,5 @@ export class ComboboxEmptyComponent { protected readonly combobox = inject(ComboboxComponent); protected readonly isVisible = computed(() => isCollectionEmpty(this.combobox.visibleCount())); - protected readonly emptyClass = computed(() => - cn(COLLECTION_EMPTY_CLASS, this.class()), - ); + protected readonly emptyClass = computed(() => cn(COLLECTION_EMPTY_CLASS, this.class())); } diff --git a/packages/ui/src/lib/components/combobox/combobox-group.component.ts b/packages/ui/src/lib/components/combobox/combobox-group.component.ts index 26084b33..684f7cd7 100644 --- a/packages/ui/src/lib/components/combobox/combobox-group.component.ts +++ b/packages/ui/src/lib/components/combobox/combobox-group.component.ts @@ -28,7 +28,5 @@ export class ComboboxGroupComponent { protected readonly headingId = uniqueId('sanring-combobox-group-heading'); protected readonly headingClass = cn(COLLECTION_GROUP_HEADING_CLASS, 'py-1.5'); - protected readonly groupClass = computed(() => - cn(COLLECTION_GROUP_CLASS, 'py-1', this.class()), - ); + protected readonly groupClass = computed(() => cn(COLLECTION_GROUP_CLASS, 'py-1', this.class())); } diff --git a/packages/ui/src/lib/components/combobox/combobox-input.component.ts b/packages/ui/src/lib/components/combobox/combobox-input.component.ts index 5021d0ee..937db890 100644 --- a/packages/ui/src/lib/components/combobox/combobox-input.component.ts +++ b/packages/ui/src/lib/components/combobox/combobox-input.component.ts @@ -1,4 +1,12 @@ -import { ChangeDetectionStrategy, Component, booleanAttribute, computed, ElementRef, inject, input } from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + booleanAttribute, + computed, + ElementRef, + inject, + input, +} from '@angular/core'; import { LucideX } from '@lucide/angular'; import { ComboboxComponent } from './combobox.component'; import { ComboboxChipInputComponent } from './combobox-chip-input.component'; @@ -13,7 +21,7 @@ import { FIELD_SIZE_CLASS } from '../component-styles'; template: ` @if (showClearButton()) { - @@ -114,7 +117,7 @@ export class ComboboxInputComponent { if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { this.combobox.onKeydown(event); } else if (event.key === 'Escape') { - this.combobox.toggleOpen(false); + this.combobox.closeAndRestoreFocus(); } else if (event.key === 'Enter') { this.combobox.onKeydown(event); } diff --git a/packages/ui/src/lib/components/combobox/combobox-label.component.ts b/packages/ui/src/lib/components/combobox/combobox-label.component.ts index 6efd92aa..d266429d 100644 --- a/packages/ui/src/lib/components/combobox/combobox-label.component.ts +++ b/packages/ui/src/lib/components/combobox/combobox-label.component.ts @@ -7,7 +7,7 @@ import { cn } from '../../utils'; selector: 'sanring-combobox-label', standalone: true, template: ` -