From a533be3c3a6d30f7873c1af80890ca1706ec161f Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Tue, 5 May 2026 13:50:32 +0200 Subject: [PATCH] fix(core-testing): address code review feedback for Task 1 - Remove unused @trpc/tanstack-react-query dependency - Document renderWithProviders tRPC provider omission (boundary constraint) - Implement deep merge in defineFactory (preserves nested sibling keys) - Document httpBatchLink rationale in mock-trpc.ts - Align core-testing's own vitest.config with safety defaults (mockReset, unstubGlobals) - Add createMockTrpcClient usage example to AGENTS.md Reviewer: superpowers:code-reviewer (Task 1 of Plan 7). Co-Authored-By: Claude Sonnet 4.6 --- packages/core-testing/AGENTS.md | 17 ++++++++++++++ packages/core-testing/package.json | 1 - .../src/factory/define-factory.test.ts | 22 ++++++++++++++++++ .../src/factory/define-factory.ts | 23 ++++++++++++++++++- packages/core-testing/src/react/mock-trpc.ts | 3 +++ .../src/react/render-with-providers.tsx | 6 +++++ packages/core-testing/vitest.config.ts | 2 ++ pnpm-lock.yaml | 3 --- 8 files changed, 72 insertions(+), 5 deletions(-) diff --git a/packages/core-testing/AGENTS.md b/packages/core-testing/AGENTS.md index bb15948..d7bb83f 100644 --- a/packages/core-testing/AGENTS.md +++ b/packages/core-testing/AGENTS.md @@ -7,6 +7,7 @@ Shared testing utilities. Tag: `tooling`. May be depended on by any package as a - `@repo/core-testing/factory` — `defineFactory(builder)` for test data factories - `@repo/core-testing/contract` — `defineContractSuite(name, suite)` for cross-impl contract tests - `@repo/core-testing/react` — `renderWithProviders`, `createMockTrpcClient` + - `renderWithProviders` does NOT include a tRPC provider. Consumers needing tRPC should wire their own TRPCProvider (from their app's tRPC client setup) and use `createMockTrpcClient` as the client. This constraint exists because tooling packages cannot import `AppRouter` from `@repo/core-api`. - `@repo/core-testing/payload` — `stubPayloadConfig`, `mockPayloadModule` - `@repo/core-testing/setup/jsdom` — vitest setupFile (jest-dom + cleanup) - `@repo/core-testing/setup/node` — vitest setupFile (no-op placeholder) @@ -23,6 +24,22 @@ export const articleFactory = defineFactory
(({ sequence }) => ({ })); ``` +## Using createMockTrpcClient + +For component tests that consume tRPC procedures, mock the responses by procedure path (dot-separated): + +```typescript +import { createMockTrpcClient } from "@repo/core-testing/react"; +import type { AppRouter } from "@repo/core-api"; // import in your app/feature, not in core-testing + +const trpcClient = createMockTrpcClient({ + "blog.articleBySlug": { id: "1", title: "Hello", slug: "hello" }, + "blog.listArticles": [], +}); +``` + +Combine with your app's TRPCProvider for components that need a tRPC client in the render tree. + ## Adding a contract suite See `docs/guides/tdd-workflow.md` §"Contract suite usage". diff --git a/packages/core-testing/package.json b/packages/core-testing/package.json index fb64a5c..2d172ff 100644 --- a/packages/core-testing/package.json +++ b/packages/core-testing/package.json @@ -23,7 +23,6 @@ "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.5.0", "@trpc/client": "^11.0.0", - "@trpc/tanstack-react-query": "^11.0.0", "@tanstack/react-query": "^5.59.0", "react": "^19.0.0", "react-dom": "^19.0.0", diff --git a/packages/core-testing/src/factory/define-factory.test.ts b/packages/core-testing/src/factory/define-factory.test.ts index 4afda7a..2735b66 100644 --- a/packages/core-testing/src/factory/define-factory.test.ts +++ b/packages/core-testing/src/factory/define-factory.test.ts @@ -55,4 +55,26 @@ describe("defineFactory", () => { userFactory.reset(); expect(userFactory.build().id).toBe("user-1"); }); + + it("deep-merges nested object overrides without losing sibling keys", () => { + const factory = defineFactory<{ id: string; meta: { source: string; tags: string[] } }>(({ sequence }) => ({ + id: `id-${sequence}`, + meta: { source: "default", tags: ["a", "b"] }, + })); + const result = factory.build({ meta: { source: "custom" } as never }); + expect(result.meta.source).toBe("custom"); + expect(result.meta.tags).toEqual(["a", "b"]); // sibling key preserved + }); + + it("replaces array overrides atomically (does not concat)", () => { + const factory = defineFactory<{ tags: string[] }>(() => ({ tags: ["a", "b"] })); + const result = factory.build({ tags: ["c"] }); + expect(result.tags).toEqual(["c"]); + }); + + it("replaces Date overrides atomically", () => { + const factory = defineFactory<{ when: Date }>(() => ({ when: new Date("2026-01-01") })); + const result = factory.build({ when: new Date("2030-12-31") }); + expect(result.when.getFullYear()).toBe(2030); + }); }); diff --git a/packages/core-testing/src/factory/define-factory.ts b/packages/core-testing/src/factory/define-factory.ts index dcf2d40..980eab1 100644 --- a/packages/core-testing/src/factory/define-factory.ts +++ b/packages/core-testing/src/factory/define-factory.ts @@ -8,6 +8,27 @@ export interface Factory { reset(): void; } +function isPlainObject(v: unknown): v is Record { + return typeof v === "object" && v !== null && Object.getPrototypeOf(v) === Object.prototype; +} + +function deepMerge(base: T, overrides: Partial): T { + if (!isPlainObject(base) || !isPlainObject(overrides)) { + return (overrides ?? base) as T; + } + const result: Record = { ...base }; + for (const key of Object.keys(overrides)) { + const baseVal = (base as Record)[key]; + const overrideVal = (overrides as Record)[key]; + if (isPlainObject(baseVal) && isPlainObject(overrideVal)) { + result[key] = deepMerge(baseVal, overrideVal); + } else { + result[key] = overrideVal; + } + } + return result as T; +} + export function defineFactory( builder: (ctx: FactoryContext) => T, ): Factory { @@ -16,7 +37,7 @@ export function defineFactory( build(overrides) { sequence += 1; const base = builder({ sequence }); - return { ...base, ...(overrides ?? {}) } as T; + return deepMerge(base, overrides ?? {}); }, buildList(count, overrides) { return Array.from({ length: count }, () => this.build(overrides)); diff --git a/packages/core-testing/src/react/mock-trpc.ts b/packages/core-testing/src/react/mock-trpc.ts index 15dc610..62801bf 100644 --- a/packages/core-testing/src/react/mock-trpc.ts +++ b/packages/core-testing/src/react/mock-trpc.ts @@ -18,6 +18,9 @@ export function createMockTrpcClient( return new Response(JSON.stringify([{ result: { data: superjson.serialize(result) } }]), { status: 200 }); }; + // httpBatchLink requires a conditional TransformerOptions that + // depends on whether the router uses a transformer; making this generic-friendly + // without parameterizing twice requires here. // eslint-disable-next-line @typescript-eslint/no-explicit-any const link = httpBatchLink({ url: "http://mock/api/trpc", diff --git a/packages/core-testing/src/react/render-with-providers.tsx b/packages/core-testing/src/react/render-with-providers.tsx index 277b479..d27c0c0 100644 --- a/packages/core-testing/src/react/render-with-providers.tsx +++ b/packages/core-testing/src/react/render-with-providers.tsx @@ -7,6 +7,12 @@ export interface RenderOptions { queryClient?: QueryClient; } +// Wraps the given UI with QueryClientProvider only. +// This helper intentionally omits a TRPCProvider. Adding one would require importing +// AppRouter from @repo/core-api, which violates the tooling→core-composition boundary rule. +// For components that need tRPC in the render tree, the consumer must: +// 1. Wire their own TRPCProvider (from their app's tRPC client setup). +// 2. Pass a client built with createMockTrpcClient as the tRPC client. export function renderWithProviders( ui: ReactElement, options: RenderOptions = {}, diff --git a/packages/core-testing/vitest.config.ts b/packages/core-testing/vitest.config.ts index da48845..506941c 100644 --- a/packages/core-testing/vitest.config.ts +++ b/packages/core-testing/vitest.config.ts @@ -9,6 +9,8 @@ export default defineConfig({ setupFiles: ["./src/setup/jsdom.ts"], clearMocks: true, restoreMocks: true, + mockReset: true, + unstubGlobals: true, }, resolve: { alias: { "@": path.resolve(__dirname, "./src") }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c8b5a65..c43904f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -443,9 +443,6 @@ importers: '@trpc/server': specifier: ^11.0.0 version: 11.16.0(typescript@5.9.3) - '@trpc/tanstack-react-query': - specifier: ^11.0.0 - version: 11.16.0(@tanstack/react-query@5.96.2(react@19.2.4))(@trpc/client@11.16.0(@trpc/server@11.16.0(typescript@5.9.3))(typescript@5.9.3))(@trpc/server@11.16.0(typescript@5.9.3))(react@19.2.4)(typescript@5.9.3) payload: specifier: ^3.0.0 version: 3.81.0(graphql@16.13.2)(typescript@5.9.3)