chore(template): dependency cleanup - #59
Conversation
📝 WalkthroughWalkthroughThe CLI now uses a shared template scaffolding helper. Convex providers share a factory, story generation is reorganized into parsing and healing helpers, unused template dependencies are removed, and a patch changeset is added. ChangesTemplate runtime and generation
Contributor workflow
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/template/scripts/generate-stories.ts (1)
349-393: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winType-only named imports silently lose the
typekeyword when a default import is present.
renderNamedSegmentomits per-nametypeprefixes wheneverallTypeis true (Line 353-355), on the assumption the caller will use theimport type { ... }wrapper. ButrenderImportonly takes that wrapper path when!pi.defaultName(Line 386). If a parsed import has both a default name and named specifiers that are all type-only (e.g.import Foo, { type Bar } from "mod"), the result becomesimport Foo, { Bar } from "mod";— thetypequalifier is dropped, turning a type-only import into a value import. This is directly reachable from a single demo source file (no merge needed), and would surface as a typecheck failure underisolatedModules/verbatimModuleSyntax, which the heal loop can't fix by dropping a demo (it's an import-level issue, not tied to one story), potentially exhaustingMAX_HEAL_PASSESand requiring manual fixes.🐛 Proposed fix
-function renderNamedSegment(named: NamedImport[]): { - allType: boolean; - text: string; -} { - const allType = named.every((n) => n.isType); - const names = named.map((n) => { - const typePrefix = !allType && n.isType ? "type " : ""; +function renderNamedSegment( + named: NamedImport[], + forceIndividualPrefix = false +): { + allType: boolean; + text: string; +} { + const allType = named.every((n) => n.isType); + const useIndividualPrefix = forceIndividualPrefix || !allType; + const names = named.map((n) => { + const typePrefix = useIndividualPrefix && n.isType ? "type " : ""; return n.alias ? `${typePrefix}${n.name} as ${n.alias}` : `${typePrefix}${n.name}`; });if (pi.named.length > 0) { - const { allType, text } = renderNamedSegment(pi.named); + const { allType, text } = renderNamedSegment( + pi.named, + Boolean(pi.defaultName) + ); if (allType && !pi.defaultName) { return `import type ${text} from "${pi.source}";`; } segments.push(text); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/template/scripts/generate-stories.ts` around lines 349 - 393, Preserve type-only named import qualifiers when rendering an import that also has a default name. Update renderNamedSegment and/or renderImport so all-type named specifiers use per-name “type” prefixes whenever pi.defaultName is present, while retaining the import type wrapper for named-only imports.
🧹 Nitpick comments (1)
apps/cli/template/scripts/generate-stories.ts (1)
313-347: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
SKIPPED_IMPORT_SOURCESmostly dead code — already intercepted earlier.4 of the 5 entries here (
@/hooks/use-media-query,@/hooks/use-copy-to-clipboard,@/examples/radix/hooks/use-mobile,@/registry/new-york-v4/hooks/use-mobile) are identical to the sources inHOOK_SOURCE_FLAGS(Line 556-561). SincemarkHookSourceruns first inprocessExampleImports(Line 586) andcontinues on match,transformImportSourcenever actually sees these sources — only@/components/language-selectoris reachable here. Keeping both lists risks them diverging silently (e.g. someone updates one list assuming it's authoritative).♻️ Proposed cleanup
const SKIPPED_IMPORT_SOURCES = new Set([ - "`@/hooks/use-media-query`", - "`@/hooks/use-copy-to-clipboard`", - "`@/examples/radix/hooks/use-mobile`", - "`@/registry/new-york-v4/hooks/use-mobile`", "`@/components/language-selector`", ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/template/scripts/generate-stories.ts` around lines 313 - 347, Remove the four hook entries duplicated by HOOK_SOURCE_FLAGS from SKIPPED_IMPORT_SOURCES, leaving only `@/components/language-selector` for transformImportSource to skip. Keep markHookSource as the authoritative handling for those hook sources and preserve the existing transformImportSource behavior for the remaining entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/cli/template/scripts/generate-stories.ts`:
- Around line 349-393: Preserve type-only named import qualifiers when rendering
an import that also has a default name. Update renderNamedSegment and/or
renderImport so all-type named specifiers use per-name “type” prefixes whenever
pi.defaultName is present, while retaining the import type wrapper for
named-only imports.
---
Nitpick comments:
In `@apps/cli/template/scripts/generate-stories.ts`:
- Around line 313-347: Remove the four hook entries duplicated by
HOOK_SOURCE_FLAGS from SKIPPED_IMPORT_SOURCES, leaving only
`@/components/language-selector` for transformImportSource to skip. Keep
markHookSource as the authoritative handling for those hook sources and preserve
the existing transformImportSource behavior for the remaining entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1cdfbc5-3e87-41f5-b78a-1e4929186754
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.agents/AGENTS.md.changeset/heavy-lamps-repeat.mdapps/cli/scripts/e2e-scaffold.tsapps/cli/scripts/init.test.tsapps/cli/scripts/init.tsapps/cli/scripts/utils.tsapps/cli/template/apps/api/package.jsonapps/cli/template/apps/app/package.jsonapps/cli/template/apps/desktop/package.jsonapps/cli/template/apps/mobile/package.jsonapps/cli/template/apps/storybook/package.jsonapps/cli/template/apps/web/package.jsonapps/cli/template/packages/backend/package.jsonapps/cli/template/packages/convex/package.jsonapps/cli/template/packages/convex/provider.native.tsxapps/cli/template/packages/convex/provider.shared.tsxapps/cli/template/packages/convex/provider.tsxapps/cli/template/packages/design-system/package.jsonapps/cli/template/packages/next-config/package.jsonapps/cli/template/scripts/generate-stories.tspackages/design-system/package.json
💤 Files with no reviewable changes (4)
- apps/cli/template/packages/convex/package.json
- apps/cli/template/packages/backend/package.json
- .agents/AGENTS.md
- apps/cli/template/packages/next-config/package.json
Summary
sharp,import-in-the-middle, andexpo-*kept)provider.tsx/provider.native.tsx) behind a shared factory, keeping the platform env vars, client options, and"use client"boundary intactscaffoldTemplatehelper for the CLI init and e2e scaffold scripts; stop exportingrewriteBunTokensgenerate-stories.ts(behavior-preserving)create-mf2-apppatch releaseTest plan
bun test scripts/inapps/cli— 118 passbun run check(biome) clean at root and on template filestsgo --strictclean ongenerate-stories.ts; convex package typecheck cleanNote
Low Risk
Mostly dependency pruning, internal refactors, and scaffold deduplication; Convex provider behavior is intended to stay equivalent across web and native.
Overview
This PR trims the scaffolded monorepo template by dropping verified-unused dependencies from multiple apps/packages (and mirrors the same cleanup on the root
design-systempackage), while keeping runtime-provided deps likesharpand Sentry instrumentation packages. AI-related packages move to the template rootpackage.jsonwith an@ai-sdk/provider-utilsoverride;@repo/backenddrops directai/@ai-sdk/gatewayin favor of@repo/ai.Convex web and native providers now share
createConvexComponentsinprovider.shared.tsx, so each entry file only sets env URL, client options, and the"use client"boundary.CLI scaffolding copies the template through a new
scaffoldTemplatehelper used byinitand e2e-scaffold (optional tolerant dotfile renames);rewriteBunTokensis no longer exported. Story generation refactorsgenerate-stories.tsinto smaller helpers without intended behavior change. A create-mf2-app patch changeset is included; agent commit guidelines no longer require CodeRabbit before commit.Reviewed by Cursor Bugbot for commit c1aca5a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements