mirror of
https://github.com/multica-ai/multica.git
synced 2026-09-28 13:23:48 +08:00
41ddf6c0a1
* refactor(frontend): drop dead UI components and phantom deps, add knip `.npmrc` sets shamefully-hoist=true, which hides both halves of dependency drift: an undeclared import still resolves, and an obsolete declaration still installs. Neither build, typecheck, nor lint reads the import graph, so package.json stopped being the source of truth for what is actually used. Removed 16 zero-reference components from packages/ui (1,907 lines, all `pnpm ui:add` output that never got wired up) plus mention-hover-card.tsx, which was written but never imported — member and agent mentions render as a plain `.mention` span. The comment in issue-hover-card.tsx claiming otherwise was stale; corrected to match rich-content.tsx. Deleting pagination.tsx orphaned its two i18n keys, dropped from all four locales and from the `ui` namespace type. Dependency declarations removed where nothing imports them: - vaul, embla-carousel-react — only the deleted drawer/carousel used them - date-fns, @tanstack/react-query-devtools — zero imports repo-wide - motion, tw-animate-css from packages/ui — real users are views/desktop/web - 44 of apps/web's 58 declarations, including all 15 @tiptap/* entries. One had already drifted: apps/web pinned tailwind-merge ^3.5.0 against the catalog's ^3.4.0, a second source of truth for the same package. Added @types/hast to packages/views, which imports it directly. knip enforces this going forward, scoped to files/dependencies/unlisted — `exports` is unusable here because ui and views are consumed through per-file exports maps. The CI step runs in warn mode (continue-on-error) and reports 9 pre-existing findings left out of this cleanup; clear those before promoting it to blocking. Verified: build, typecheck, lint green across web/desktop/ui/core/views. 6,503 tests pass; the 9 failures are pre-existing on a clean tree (a local jsdom localStorage issue). Web dev server boots and serves /login 200. Co-authored-by: multica-agent <github@multica.ai> * fix(ci): cover the packages/ui blind spot knip cannot see Review caught that knip provided no regression protection for the exact thing the preceding cleanup was about. knip derives entry points from a workspace's package.json `exports` — graph/build.js calls `getEntrySpecifiersFromManifest` unconditionally, with no config flag to opt out — and packages/ui exports four wildcards. Each expands to a glob over its whole directory, so every file under components/ui, components/common, markdown, and hooks registers as an entry point and can never be reported unused. A zero-reference probe component confirmed it: knip's output was byte-identical with and without it. `entry: []`, negated `entry` patterns, `--production`, and `--strict` were all tried. Manifest entries come from a separate code path that none of them subtract from. packages/views is unaffected because 44 of its 45 exports name a specific file, which is why knip does flag dead files there. Added scripts/check-ui-wildcard-exports.mjs to cover those four directories, blocking from the start since it reports nothing once the one file it found is gone. It reads the wildcards from the manifest, so narrowing an export hands that directory back to knip without the two overlapping. Its test asserts all three states: clean tree passes, a stranded component fails and is named, and importing that component clears it again. The check found packages/ui/hooks/use-auto-scroll.ts — 80 lines whose only occurrence repo-wide was its own definition. Same category as the 16 components already removed, and invisible to knip for the same reason. Also from review: - The path filter missed knip.jsonc and apps/docs/**, so config-only and docs-only PRs skipped knip even though it analyzes docs. - The knip step paired `continue-on-error` with `|| true`, which zeroed the exit code before Actions saw it — no warning surfaced, only log text. Dropping `|| true` makes the warn phase visible and the later promotion to blocking a one-line change. - Corrected the knip.jsonc comment that claimed apps were the only entry points for all three shared packages. True for views and core, never true for packages/ui. Verified: build, typecheck, lint green (12/12, 0 errors). 6,503 tests pass; the same 9 pre-existing failures as before, reproduced on a clean tree. Co-authored-by: multica-agent <github@multica.ai> * fix(ci): resolve import paths in the ui export check instead of matching names Review found the check passed a dead component whenever any file elsewhere in the repo imported a same-named module. The relative-import branch was a whole-repo text regex on the basename, so `../components/option-card` in packages/views vouched for an untouched packages/ui/components/ui/option-card. That is not a corner case: 17 of the 59 guarded files already share a basename with another file in the repo — `button`, `card`, `input`, `label`, `avatar`, `tabs`, `switch`, `skeleton`, `separator`, `theme-provider` among them. shadcn names are generic by construction, so the hole covered the most likely names a future `pnpm ui:add` would produce. Specifiers are now resolved against each importing file's own directory and compared against real paths, with `@multica/ui/...` mapped through the same manifest wildcards the candidate list is built from. Liveness is now reachability from outside the guarded directories rather than a reference count, so two dead components importing only each other no longer vouch for one another. Also implements the exemption the failure message promises: a file named by a non-wildcard export is an entry point in its own right and is skipped. The candidate loop previously read the wildcard directories unconditionally, so following that advice would not have cleared the check. Tests cover all five states: clean tree, stranded component named, cleared by a real import, the option-card basename collision, and a mutually-importing dead pair reported as two files. Verified: check and its tests pass, knip unchanged at 8 files + 1 dependency, build/typecheck/lint 12/12 green. Co-authored-by: multica-agent <github@multica.ai> * fix(ci): parse imports with the TypeScript parser, not a source regex Review found the scanner counted commented-out imports as real edges. It ran a regex over raw source, so a component kept its liveness from a line that does not execute — and "delete the last real import, leave the comment behind" is a normal step in a refactor, which makes the false edge appear at exactly the moment the component stops being used. Edges now come from real syntax nodes: static import/export declarations, `import()`, `require()`, and `import("x")` type nodes. `vi.mock("...")` is excluded on purpose — it names a module without importing it, so a component whose only remaining mention is a test mock is dead, and counting it would let one outlive its last real use. Scope, measured rather than assumed: 52 specifiers in this repo appear only in comments or prose, spread over 43 files, so the mechanism is live. None of them currently resolve into packages/ui, so no dead component is being missed today — this closes a latent hole, not an active miss. (An earlier count of 3 was wrong: those were `lazy(() => import(...))` calls that my throwaway measurement script failed to treat as real.) Test gains a sixth state: a component referenced only by a commented-out import and a string containing one must still be reported. Verified: six-state test passes, clean tree passes in 0.9s over 2,089 files, knip unchanged at 8 files + 1 dependency, build/typecheck/lint 12/12 green. `typescript` is already a root devDependency, so no new declaration. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai>