fix: address Phase 1 code-quality review findings
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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/<feature>/ui`, schemas/types from `@repo/<feature>`
|
||||
- **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<Entities>()` 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)
|
||||
|
||||
@@ -2209,7 +2209,7 @@ footer .colophon {
|
||||
<div class="tradeoff-card">
|
||||
<div class="tag">di/bind-dev-seed.ts</div>
|
||||
<h3>Dev-seed binder</h3>
|
||||
<p class="blurb"><code>bindDevSeed<F>()</code> — unbinds the empty mock, rebinds a populated mock (post-Plan-9).</p>
|
||||
<p class="blurb"><code>bindDevSeed<F>(ctx: BindContext)</code> — unbinds the empty mock, rebinds a populated mock (post-Plan-9).</p>
|
||||
<div class="pc-cols">
|
||||
<div class="pros"><h5>Pros</h5><ul>
|
||||
<li>Dev mode shows realistic data without Payload running — design review, storybook, offline work all just work</li>
|
||||
|
||||
@@ -764,7 +764,7 @@ footer .colophon {
|
||||
<div class="cast-tag">file 05 · seeded mock</div>
|
||||
<h3>bind-dev-seed.ts</h3>
|
||||
<p class="role">Replaces the empty mock with a populated mock so the running app shows realistic data without Payload.</p>
|
||||
<p>Exports <code>bindDevSeedBlog()</code>. Same shape as <code>bind-production</code> — unbind, rebind. The difference: it constructs a fresh <code>MockArticlesRepository</code>, seeds it via <code>buildDevArticles()</code> from <code>src/__seeds__/dev.ts</code>, and binds the populated instance via <code>.toConstantValue(repo)</code>. Mutually exclusive with <code>bindProductionBlog</code> — both operate on the same symbol.</p>
|
||||
<p>Exports <code>bindDevSeedBlog(ctx)</code>. Same shape as <code>bind-production</code> — unbind, rebind. The difference: it constructs a fresh <code>MockArticlesRepository</code>, seeds it via <code>buildDevArticles()</code> from <code>src/__seeds__/dev.ts</code>, and binds the populated instance via <code>.toConstantValue(repo)</code>. Mutually exclusive with <code>bindProductionBlog</code> — both operate on the same symbol.</p>
|
||||
<div class="runs-when"><strong>When it runs:</strong> called from app boot when <code>USE_DEV_SEED === "true"</code>. Storybook stories that need data may call this directly.</div>
|
||||
</div>
|
||||
|
||||
@@ -1093,7 +1093,7 @@ footer .colophon {
|
||||
</div>
|
||||
<div class="conditions-row">
|
||||
<div class="scenario">Storybook story that wants populated data</div>
|
||||
<div class="trigger">await bindDevSeedBlog()</div>
|
||||
<div class="trigger">await bindDevSeedBlog(ctx)</div>
|
||||
<div class="outcome">dev seed · populated mock</div>
|
||||
</div>
|
||||
<div class="conditions-row">
|
||||
@@ -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."
|
||||
],
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
@@ -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";`,
|
||||
|
||||
@@ -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<void> {
|
||||
export async function bindDevSeed{{pascalCase name}}(ctx: BindContext): Promise<void> {
|
||||
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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user