Repository navigation
refactor(web-shared): import web-shared via @kilocode/web-shared - #7051
chrarnoldus wants to merge 12 commits into
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental changes since the prior review move shared test helpers/constants and Verification performed
Files Reviewed (357 files)
Previous Review Summaries (2 snapshots, latest commit 3f5682f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3f5682f)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change since the prior review declares the Files Reviewed (2012 files)
Verification performed
Previous review (commit e97b1ee)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryA 2,010-file mechanical import-specifier refactor ( Overview
Issue Details (click to expand)SUGGESTION
Verification performed
Files Reviewed (2,010 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
e97b1ee to
3f5682f
Compare
Add the explicit specifier next to the existing @/ fallback in every consumer tsconfig, the web Jest config, the trpc rollup resolver and the Storybook webpack alias, so imports can move to it.
Mechanical codemod output only, followed by oxfmt. Every `@/` module specifier that resolved into packages/web-shared/src now uses `@kilocode/web-shared/`: static imports and re-exports, dynamic import(), require(), jest.mock/doMock/dontMock/requireActual/requireMock and `typeof import()` types. Each specifier was resolved with the tsconfig that owns the importing file (own src first, then packages/web-shared/src). 1,989 files, 5,535 specifiers on 5,533 lines.
3f5682f to
02c7516
Compare
report-consumer.glanceable.test.ts loads apps/web's active-sessions-list and mocks the modules it imports by specifier, so the mocks for the web-shared modules follow the codemod to @kilocode/web-shared/.
`@/*` now maps only to the consumer's own src in apps/web, apps/ai-gateway, packages/trpc, the web Jest config and the Storybook alias. Inside packages/web-shared, `@/*` maps to apps/web/src for tests that use apps/web helpers; tsconfig.lib.json drops it so runtime code cannot use `@/` at all. The trpc rollup resolver and the MCP catalog import resolver follow `@kilocode/web-shared/*` into packages/web-shared/src.
Replace the @/ fallback and shadowing rule in packages/web-shared/AGENTS.md and apps/ai-gateway/AGENTS.md, and point comments that name web-shared modules at their new specifiers.
packages/trpc imports @kilocode/web-shared/* from source, like the @kilocode/db and @kilocode/encryption devDependencies it already declares.
Test helpers are about to live in packages/web-shared/src/tests; exclude them from tsconfig.lib.json and Storybook like apps/web/src/tests.
git mv the apps/web test helpers that web-shared tests use (user, organization, microdollar-usage, approval, payment-method, openrouter-models, fake-r2) to packages/web-shared/src/tests/helpers, and the self-contained constant modules they or web-shared tests need (lib/auth/constants, lib/organizations/constants, lib/ai-gateway/latest-model-aliases) to the same paths in web-shared. Script output only, followed by oxfmt: every specifier that resolved to a moved file now uses @kilocode/web-shared/<path> (405 specifiers in 326 files).
git mv apps/web/src/lib/balanceCache.ts, which only depends on web-shared modules and is used by web-shared tests, and point its importers at @kilocode/web-shared/lib/balanceCache (script output, followed by oxfmt).
…ps/web Every test case is kept; only its file changes. - Whole files: kilo-pass-org/consumption.test.ts (its fixtures use the apps/web Kilo Pass org service) and model-stats/model-stats-cache.test.ts (drives the apps/web /api/models/stats routes) move to the same paths in apps/web. - removeUserFromOrganization cases, and the organizations and usage cases that use it, move to organization-member-removal(-usage).test.ts. - next-auth-options, passkey and platform-admin-grant cases from user/server.test.ts move to user/next-auth-options.test.ts, and the getProfileRedirectPath cases to user/profile-redirect-path.test.ts. - The subscription-accounting cases from kilo-pass/issuance.test.ts move to kilo-pass/subscription-accounting.test.ts; their shared createTestSubscription fixture becomes tests/helpers/kilo-pass.helper.ts. - The upstream-budget assertion from trpc/control-plane-budget.test.ts moves to bounded-service-fetch.test.ts.
packages/web-shared/tsconfig.json no longer maps @/ to apps/web/src, and `pnpm --filter @kilocode/web-shared typecheck` now also typechecks the tests with it. An oxlint rule rejects @/ and relative apps/web imports anywhere in the package.
02c7516 to
4f2fc44
Compare
61de6a7 to
4f2fc44
Compare
Summary
#6806 moved ~450 files from
apps/web/srctopackages/web-shared/srcwithout changing their@/specifiers, so every consumer resolved@/*to its ownsrc/*first and topackages/web-shared/src/*second. A path could exist in only one tree, and a duplicate silently shadowed the web-shared file insideapps/web.packages/web-shared/tsconfig.jsonalso fell back toapps/web/src, so the package depended on the app. This PR removes both fallbacks.@kilocode/web-shared/<path>maps topackages/web-shared/src/<path>. It is wired the same way as@kilocode/db/*: tsconfigpathsinapps/web(tsconfig.json,tsconfig.scripts.json, inherited by Storybook),apps/ai-gateway,packages/trpcandpackages/web-shareditself, plus the web JestmoduleNameMapper, the@kilocode/trpcrollup resolver and the Storybook webpack alias. Code inside web-shared uses the same specifier, not relative paths. That keeps one spelling per module everywhere, so moving a file between the two trees only changes its prefix.exportsmap inpackages/web-shared/package.json. The specifiers are extensionless and point at.ts/.tsxfiles andindex.tsdirectories. Anexportswildcard cannot express extension or index probing, so resolution relies on the mappings above, as@kilocode/db/*subpaths do in practice.packages/trpcnow declares@kilocode/web-sharedas aworkspace:*devDependency, as it already does for@kilocode/dband@kilocode/encryption.@/fallback removed.@/*inapps/web,apps/ai-gateway,packages/trpc, the web Jest config and the Storybook alias maps only to the consumer's ownsrc. Shadowing is no longer possible: a@/path that only exists in web-shared fails to resolve.apps/web, tests included.packages/web-shared/tsconfig.jsonhas no@/mapping and no reference toapps/web.pnpm --filter @kilocode/web-shared typechecknow runstsconfig.lib.json(runtime code only, no Jest types) andtsconfig.json(everything, tests included).no-restricted-importsrule rejects@/and relativeapps/webimports anywhere in the package.@/import fails the lib config (TS2307), a test@/import failstsconfig.json(TS2882), and both@/and../../../../apps/web/...imports fail oxlint.apps/webhelpers used by web-shared tests (user,organization,microdollar-usage,approval,payment-method,openrouter-models,fake-r2) moved topackages/web-shared/src/tests/helpers. Four self-contained modules that those helpers or web-shared tests need also moved to the same paths in web-shared:lib/auth/constants,lib/organizations/constants,lib/ai-gateway/latest-model-aliasesandlib/balanceCache. They are constants, plus a 14-line helper that only depends on web-shared. All moves usedgit mv. A script (below) rewrote their importers to@kilocode/web-shared/...: 405 specifiers in 326 files for the helpers and constants, and 8 specifiers in 8 files forbalanceCache.src/testsis excluded fromtsconfig.lib.jsonand from Storybook.apps/webcode moved toapps/web. No test case was dropped; each file's case count adds up.kilo-pass-org/consumption.test.ts(fixtures use the apps/web Kilo Pass org service) andmodel-stats/model-stats-cache.test.ts(drives the apps/web/api/models/statsroutes) moved whole, withgit mv, to the same paths inapps/web.organizations.test.tsandorganization-usage.test.ts: theremoveUserFromOrganizationcases and the cases that use it moved toapps/web/src/lib/organizations/organization-member-removal.test.tsandorganization-member-removal-usage.test.ts(100 = 85 + 15, 60 = 58 + 2).user/server.test.ts: the next-auth-options, passkey and platform-admin-grant cases moved toapps/web/src/lib/user/next-auth-options.test.ts, and thegetProfileRedirectPathcases toprofile-redirect-path.test.ts(125 = 77 + 42 + 6).kilo-pass/issuance.test.ts: thesubscription-accountingcases moved toapps/web/src/lib/kilo-pass/subscription-accounting.test.ts(14 = 12 + 2). Their sharedcreateTestSubscriptionfixture becametests/helpers/kilo-pass.helper.ts.trpc/control-plane-budget.test.tsmoved into its own case inapps/web/src/lib/bounded-service-fetch.test.ts.globalSetup.@/toapps/web/srcand@kilocode/web-shared/topackages/web-shared/src. The rebuiltdist/index.d.tsanddist/mobile.d.tsare byte-identical to the build frommain.apps/web/src/scripts/mcp-catalog/catalog.ts) now follows@kilocode/web-shared/into web-shared. Before, it did not follow the fallback at all. No sub-router lives in web-shared today, so the regeneratedservices/kilo-mcp/catalog.jsonis unchanged.email-literal-guardrail.test.ts,spend-writer-audit.test.ts,kiloclaw-price-rollout-audit.ts) scan file paths, not specifiers, so they needed no change.packages/web-shared/AGENTS.mdandapps/ai-gateway/AGENTS.mddescribe the specifier and the no-apps/webrule instead of the fallback and shadowing rule. Code comments that named web-shared modules by@/path now use the new specifier.Codemod
The specifier rewrite is a TypeScript compiler API codemod (source below). It parses every git-tracked
.ts/.tsx/.js/.jsx/.mjs/.cjsfile underapps/web,apps/ai-gateway,apps/storybook,packages/trpc,packages/web-sharedandservices/deploy-infra/builder/test-postgres. The last one runs under the apps/web Jest config. The codemod collects string literals only in module-specifier positions:import/export … from,import x = require()import(),require(),require.resolve()jest.*('…')call:mock,doMock,dontMock,requireActual,requireMock, …typeof import('…')types anddeclare module '…'It resolves each
@/specifier withts.resolveModuleNameagainst the pre-change tsconfig that owns the file (ownsrcfirst, then web-shared). If the target is inpackages/web-shared/src, it replaces the@/prefix with@kilocode/web-shared/. Files in web-shared were resolved with both the apps/web order (what Next.js and Jest use) and web-shared's own tsconfig order. The run aborts if the two disagree, and they never did. No specifier was unresolved. The only@/string literals outside specifier positions are config and tooling code (the Storybook@/lib/utilsmock alias and resolver prefixes), which the codemod left alone. After rebasing onmain, I re-ran the codemod on top of the first commit rather than resolving import conflicts by hand.apps/webpackages/web-sharedapps/ai-gatewayapps/storybookpackages/trpcservices/deploy-infra/builder/test-postgresThe 5,535 specifiers sit on 5,533 lines: two lines carry two specifiers each. After
oxfmt, the commit is1989 files changed, 6405 insertions(+), 5561 deletions(-). The extra lines are import statements that the longer specifier pushed past the line limit and the formatter rewrapped. The files wereoxfmt-clean before the rewrite, so no unrelated reformatting is mixed in. That is more than the ~2,000 lines estimated in the issue, because@/lib/drizzle,@/lib/user/server,@/lib/trpc/init,@/lib/config.serverand@/lib/constantsalone account for ~2,000 lines.Verification
cd apps/web && pnpm exec tsgo --noEmit,apps/ai-gatewayandapps/storybooktsgo --noEmit, andpnpm --filter @kilocode/web-shared typecheck(both configs): all pass on the final branch.apps/webalso passed right after the codemod, with the fallback still in placepnpm --filter @kilocode/trpc build:dist/index.d.tsanddist/mobile.d.tsare byte-identical to themainbuild.apps/mobiletypecheck passes against itcd apps/web && pnpm script src/scripts/mcp-catalog/dump.ts: writes 864 rows, andservices/kilo-mcp/catalog.jsonis unchangedcd apps/web && NODE_ENV=test pnpm exec jest --listTests: lists 1,122 tests, including all 166 remainingpackages/web-shared/src/**/*.test.tsmain) and 7 helper-dependent suites, all passing. CI runs the full suitespnpm build-storybooksucceeded with the new alias. I ran it before the test-helper moves, which touch Storybook only through a tsconfig excludenext buildofapps/ai-gatewaywithapps/web/.env.testas.env.production.localandapps/web/.envas.env, deleted afterwards: succeedspnpm --filter cloud-agent-next exec vitest run src/telemetry/report-consumer.glanceable.test.ts: 14 passedoxlinton every changed directory,oxfmt --list-differenton changed files andgit diff --check: all cleanVisual Changes
N/A
Reviewer Notes
@kilocode/web-shared/*mapping next to the existing fallback.oxfmtonly. Review it by checking that every changed line is a specifier swap or a formatter rewrap:git show <sha> -U0 | rg '^[+-][^+-]' | rg -v '@kilocode/web-shared/|@/'shows only rewrapped import/jest.mocklines.vi.mock('@/lib/…')specifiers inservices/cloud-agent-next/src/telemetry/report-consumer.glanceable.test.ts. That Vitest test loads apps/web'sactive-sessions-listand mocks its imports by specifier, outside the codemod's scope.@/fallback and updates the trpc and MCP catalog resolvers.src/testsfrom the runtime typecheck.oxfmt).balanceCacheinto web-shared (script output plusoxfmt).@/from web-shared's tsconfig, typechecks tests, and adds the lint rule.Codemod source (
rewrite-web-shared-imports.mjs, run asnode rewrite-web-shared-imports.mjs . && git diff --name-only -z | xargs -0 pnpm exec oxfmt)Move script (
move-to-web-shared.mjs, run asnode move-to-web-shared.mjs . tests/helpers/user.helper.ts … lib/ai-gateway/latest-model-aliases.ts, thennode move-to-web-shared.mjs . lib/balanceCache.ts)