From 723729dd9e9c2979a7b2b321c19beeb5c1489ab2 Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Sat, 9 May 2026 13:17:04 +0200 Subject: [PATCH] fix: address Phase 1 code-quality review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Generator feature templates emit ctx-arg signatures + updated checklist - signUpUseCase test for bus=undefined path - di-explainer.html: bind-dev-seed no-arg → ctx - data-flow-explainer.html: tradeoff card → ctx - BindContextBase no longer exported - CLAUDE.md binder bullet: drop vestigial cast guidance Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 2 +- docs/architecture/data-flow-explainer.html | 2 +- docs/architecture/di-explainer.html | 6 +++--- .../use-cases/sign-up.use-case.test.ts | 15 +++++++++++++++ packages/core-shared/src/di/bind-context.ts | 2 +- turbo/generators/config.ts | 4 ++-- .../templates/feature/src/di/bind-dev-seed.ts.hbs | 11 ++++++----- .../feature/src/di/bind-production.ts.hbs | 11 +++++------ 8 files changed, 34 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index be754fc..3453f06 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,7 @@ Turborepo + pnpm monorepo organized by vertical features. Each feature (`auth`, - **Public surface split** — Feature root (`.`) exports contracts only: types, errors, schemas, IUseCase / IController aliases, router type, constants. UI artifacts (query builders, components) live behind `./ui` (`src/ui/index.ts`). Apps import queries from `@repo//ui`, schemas/types from `@repo/` - **Payload repositories via constructor** — Feature packages receive Payload config at constructor time, not as a direct dependency - **Three binding modes per feature** — Each feature exports two binders: `./di/bind-production` (real Payload) and `./di/bind-dev-seed` (populated mock). The app's `bindAll()` dispatcher in `apps/web-next/src/server/bind-production.ts` picks one by env: `USE_DEV_SEED="true"` → dev seed; `NODE_ENV="production"` → production; otherwise → dev seed (developer default so `pnpm dev` boots without Payload). Dev seed lives in `src/__seeds__/dev.ts` as a lazy `buildDev()` function that uses the feature's existing factory -- **Binders take a `ctx` arg from `core-shared/di`** — `bindProductionX(ctx: BindProductionContext)` for production binders; `bindDevSeedX(ctx: BindContext)` for dev-seed. Required fields: `tracer`, `logger`, plus `config` for production. Optional fields: `bus`, `queue`, `realtime`, `realtimeRegistry` (correspond to optional core packages — guard with `?.` when used, or cast to the full interface if the feature unconditionally requires the dep). Aggregator builds one ctx object and passes it to all feature binders +- **Binders take a `ctx` arg from `core-shared/di`** — `bindProductionX(ctx: BindProductionContext)` for production binders; `bindDevSeedX(ctx: BindContext)` for dev-seed. Required fields: `tracer`, `logger`, plus `config` for production. Optional fields: `bus`, `queue`, `realtime`, `realtimeRegistry` (correspond to optional core packages — guard with `?.` or `if (bus) { ... }` when used; use-case signatures should accept the protocol type when they only need protocol methods, not the full concrete interface). Aggregator builds one ctx object and passes it to all feature binders - **App bootstrap** — Each app calls `bindAll()` from a server entry point (page server component, route handler) before resolving any feature controller. The dispatcher is idempotent - **Instrumentation lives in `core-shared/instrumentation/`** — Two interfaces (`ITracer`, `ILogger`), three implementations (`NoopTracer`/`NoopLogger`, `SentryTracer`/`SentryLogger`, and `RecordingTracer`/`RecordingLogger` from `core-testing`). Feature packages MUST NOT import `@sentry/*` directly (R40, eslint-enforced) - **Spans + capture composed at DI bind time** — Use cases + controllers wrapped via `withSpan(tracer, spanOpts, withCapture(logger, tags, factory(deps)))` inside `bind-production` / `bind-dev-seed`. `withSpan` is outermost so an errored span's timing reflects the capture-and-rethrow. Repository methods are different — they call `this.tracer.startSpan(...)` and `this.logger.captureException(...)` inline per method because they own per-call attributes (R41, R42) diff --git a/docs/architecture/data-flow-explainer.html b/docs/architecture/data-flow-explainer.html index 8c53af1..7d81a4c 100644 --- a/docs/architecture/data-flow-explainer.html +++ b/docs/architecture/data-flow-explainer.html @@ -2209,7 +2209,7 @@ footer .colophon {
di/bind-dev-seed.ts

Dev-seed binder

-

bindDevSeed<F>() — unbinds the empty mock, rebinds a populated mock (post-Plan-9).

+

bindDevSeed<F>(ctx: BindContext) — unbinds the empty mock, rebinds a populated mock (post-Plan-9).

Pros
  • Dev mode shows realistic data without Payload running — design review, storybook, offline work all just work
    • diff --git a/docs/architecture/di-explainer.html b/docs/architecture/di-explainer.html index 3479209..bb2279f 100644 --- a/docs/architecture/di-explainer.html +++ b/docs/architecture/di-explainer.html @@ -764,7 +764,7 @@ footer .colophon {
    file 05 · seeded mock

    bind-dev-seed.ts

    Replaces the empty mock with a populated mock so the running app shows realistic data without Payload.

    -

    Exports bindDevSeedBlog(). Same shape as bind-production — unbind, rebind. The difference: it constructs a fresh MockArticlesRepository, seeds it via buildDevArticles() from src/__seeds__/dev.ts, and binds the populated instance via .toConstantValue(repo). Mutually exclusive with bindProductionBlog — both operate on the same symbol.

    +

    Exports bindDevSeedBlog(ctx). Same shape as bind-production — unbind, rebind. The difference: it constructs a fresh MockArticlesRepository, seeds it via buildDevArticles() from src/__seeds__/dev.ts, and binds the populated instance via .toConstantValue(repo). Mutually exclusive with bindProductionBlog — both operate on the same symbol.

    When it runs: called from app boot when USE_DEV_SEED === "true". Storybook stories that need data may call this directly.
@@ -1093,7 +1093,7 @@ footer .colophon {
Storybook story that wants populated data
-
await bindDevSeedBlog()
+
await bindDevSeedBlog(ctx)
dev seed · populated mock
@@ -1285,7 +1285,7 @@ const MODES = { scenarioTag: 'when this happens', title: 'Dev seed binder ran at app boot', narrative: [ - "App's bindAll() saw USE_DEV_SEED=true and dispatched to bindAllDevSeed(), which called bindDevSeedBlog().", + "App's bindAll() saw USE_DEV_SEED=true and dispatched to bindAllDevSeed(), which called bindDevSeedBlog(ctx).", "That binder unbound IArticlesRepository, constructed a fresh MockArticlesRepository, awaited createArticle for each entity from buildDevArticles(), and rebound the symbol with .toConstantValue(repo).", "Use case + controller bindings did not change — they still resolve through their .toDynamicValue closures, but now the closure fetches the populated mock and threads it through. The dev server now shows the welcome article, the work-in-progress draft, etc." ], diff --git a/packages/auth/src/application/use-cases/sign-up.use-case.test.ts b/packages/auth/src/application/use-cases/sign-up.use-case.test.ts index 71af2fb..d2ba67b 100644 --- a/packages/auth/src/application/use-cases/sign-up.use-case.test.ts +++ b/packages/auth/src/application/use-cases/sign-up.use-case.test.ts @@ -60,6 +60,21 @@ describe("signUpUseCase", () => { ); }); + it("works without an event bus (welcome email skipped silently)", async () => { + const users = new MockUsersRepository([]); + const auth = new MockAuthenticationService(users); + const useCase = signUpUseCase(users, auth, undefined); + + const result = await useCase({ + username: "frank", + password: "secret_password", + confirmPassword: "secret_password", + }); + + expect(result.session.userId).toBeTruthy(); + expect(result.cookie.name).toBe("session"); + }); + it("does NOT publish when sign-up fails (username taken)", async () => { const users = new MockUsersRepository([]); const auth = new MockAuthenticationService(users); diff --git a/packages/core-shared/src/di/bind-context.ts b/packages/core-shared/src/di/bind-context.ts index 9fcadb9..01958ef 100644 --- a/packages/core-shared/src/di/bind-context.ts +++ b/packages/core-shared/src/di/bind-context.ts @@ -8,7 +8,7 @@ import type { } from "./bind-protocols"; /** Always-present fields. Feature binders rely on these unconditionally. */ -export type BindContextBase = { +type BindContextBase = { tracer: ITracer; logger: ILogger; }; diff --git a/turbo/generators/config.ts b/turbo/generators/config.ts index b9996e3..677911c 100644 --- a/turbo/generators/config.ts +++ b/turbo/generators/config.ts @@ -303,8 +303,8 @@ export default function generator(plop: PlopTypes.NodePlopAPI): void { ` 2. apps/web-next/src/server/bind-production.ts:`, ` - import { bindProduction${cap(a.name)} } from "${pkg}/di/bind-production";`, ` - import { bindDevSeed${cap(a.name)} } from "${pkg}/di/bind-dev-seed";`, - ` - call bindProduction${cap(a.name)}(config, tracer, logger) (production branch)`, - ` - call await bindDevSeed${cap(a.name)}(tracer, logger) (dev-seed branch)`, + ` - call bindProduction${cap(a.name)}(ctx) (production branch, ctx: BindProductionContext)`, + ` - call await bindDevSeed${cap(a.name)}(ctx) (dev-seed branch, ctx: BindContext)`, "", ` 3. packages/core-api/src/root.ts:`, ` - import { ${camel(a.name)}Router } from "${pkg}/api";`, diff --git a/turbo/generators/templates/feature/src/di/bind-dev-seed.ts.hbs b/turbo/generators/templates/feature/src/di/bind-dev-seed.ts.hbs index 9470b52..5960f49 100644 --- a/turbo/generators/templates/feature/src/di/bind-dev-seed.ts.hbs +++ b/turbo/generators/templates/feature/src/di/bind-dev-seed.ts.hbs @@ -5,6 +5,7 @@ import { type ITracer, type ILogger, } from "@repo/core-shared/instrumentation"; +import type { BindContext } from "@repo/core-shared/di"; import { {{camelCase name}}Container } from "./container.js"; import { {{constantCase name}}_SYMBOLS } from "./symbols.js"; import { Mock{{pascalCase entity}}Repository } from "../infrastructure/repositories/{{kebabCase entity}}.repository.mock.js"; @@ -16,17 +17,17 @@ import type { I{{pascalCase entity}}Repository } from "../application/repositori /** * Replace the default mock with a populated one for dev mode + storybook. * - * Mutually exclusive with `bindProduction{{pascalCase name}}(config, tracer, logger)`. + * Mutually exclusive with `bindProduction{{pascalCase name}}(ctx)`. * Tests must NOT call this — they construct `new Mock{{pascalCase entity}}Repository()` * directly and seed via factories per-test. * * Idempotent: safe to call multiple times; each call rebuilds a fresh * populated repo and rebinds the symbol. */ -export async function bindDevSeed{{pascalCase name}}( - tracer: ITracer, - logger: ILogger, -): Promise { +export async function bindDevSeed{{pascalCase name}}(ctx: BindContext): Promise { + const { tracer, logger, bus, queue, realtime, realtimeRegistry } = ctx; + void bus; void queue; void realtime; void realtimeRegistry; + // Bind shared instrumentation into feature container if ({{camelCase name}}Container.isBound(INSTRUMENTATION_SYMBOLS.TRACER)) { {{camelCase name}}Container.unbind(INSTRUMENTATION_SYMBOLS.TRACER); diff --git a/turbo/generators/templates/feature/src/di/bind-production.ts.hbs b/turbo/generators/templates/feature/src/di/bind-production.ts.hbs index 97bdfdb..0e40e01 100644 --- a/turbo/generators/templates/feature/src/di/bind-production.ts.hbs +++ b/turbo/generators/templates/feature/src/di/bind-production.ts.hbs @@ -1,4 +1,3 @@ -import type { SanitizedConfig } from "payload"; import { withSpan, withCapture, @@ -6,17 +5,17 @@ import { type ITracer, type ILogger, } from "@repo/core-shared/instrumentation"; +import type { BindProductionContext } from "@repo/core-shared/di"; import { {{camelCase name}}Container } from "./container"; import { {{constantCase name}}_SYMBOLS } from "./symbols"; import { {{pascalCase entity}}Repository } from "../infrastructure/repositories/{{kebabCase entity}}.repository"; import { get{{pascalCase entity}}UseCase } from "../application/use-cases/get-{{kebabCase entity}}.use-case"; import { get{{pascalCase entity}}Controller } from "../interface-adapters/controllers/get-{{kebabCase entity}}.controller"; -export function bindProduction{{pascalCase name}}( - config: SanitizedConfig, - tracer: ITracer, - logger: ILogger, -): void { +export function bindProduction{{pascalCase name}}(ctx: BindProductionContext): void { + const { config, tracer, logger, bus, queue, realtime, realtimeRegistry } = ctx; + void bus; void queue; void realtime; void realtimeRegistry; + // Bind shared instrumentation into feature container if ({{camelCase name}}Container.isBound(INSTRUMENTATION_SYMBOLS.TRACER)) { {{camelCase name}}Container.unbind(INSTRUMENTATION_SYMBOLS.TRACER);