# Best practices

Instructions for implementing and reviewing code in all Vendure projects.

## [Coding Best Practices](https://github.com/Pinelab-studio/pinelab-engineering/blob/main/docs/best-practices/coding-best-practices.md)

These guidelines bias toward caution over speed. For trivial tasks, use judgment.

# Coding rules

- Add concise JSDoc to every exported function and public method. Explain their purpose and any non-obvious constraints; do not merely restate the name.
- In functions longer than 30 lines, use short comments to identify meaningful phases or explain non-obvious decisions. Do not narrate individual statements.
- Keep related code together, group based on feature/domain, not technical purpose.
- Prefer early returns over deep nesting.

## Think Before Coding

**Don't assume. Don't hide confusion. Surface tradeoffs.**

Before implementing:
- State your assumptions explicitly. If uncertain, ask.
- If multiple interpretations exist, present them - don't pick silently.
- If a simpler approach exists, say so. Push back when warranted.

## Simplicity First

**Minimum code that solves the problem. Nothing speculative.**

- No features beyond what was asked.
- No abstractions for single-use code.
- No "flexibility" or "configurability" that wasn't requested.
- No error handling for impossible scenarios.
- If you write 200 lines and it could be 50, rewrite it.
- If an edge case is rare and its impact very low, exclude that edge case from implementation if that means we end up with simpler code.

Ask yourself: "Would a senior engineer say this is overcomplicated?" If yes, simplify.

## Surgical Changes

**Touch only what you must. Clean up only your own mess.**

When editing existing code:
- Don't "improve" adjacent code, comments, or formatting.
- Don't refactor things that aren't broken.
- Match existing style, even if you'd do it differently.

The test: Every changed line should trace directly to the user's request.

# Testing rules

Apply these rules to all unit, integration, and end-to-end tests.

- Avoid Over-Testing: Focus on behavior, not implementation details. Don’t test internal state or private functions.
- Tests are for humans to understand and review. Before writing tests, propose their scenarios and expected outcomes and obtain human approval. Ask again only when behaviour or scope changes.
- Prefer integration or end-to-end tests for user-facing behaviour. Add focused unit tests for complex logic or edge cases not meaningfully covered in e2e or integration; avoid duplicating assertions across levels.
- Use helpers for readability, but place them below the test cases: the cases are what readers need to understand first.
- Use hardcoded expected values so tests detect contract changes: compare `result === 'expected-string'`, not `result === SOME_CONSTANT` imported from the implementation.

## [Pull Request Guidelines](https://github.com/Pinelab-studio/pinelab-engineering/blob/main/docs/best-practices/pull-request-guidelines.md)

Write pull request descriptions using the sections below so reviewers can quickly understand the change, validate it, and identify operational requirements. 
Keep the entire description at 200 words or fewer.

## What changed and why

- Use bullet points to describe the change and its purpose in functional, user-facing terms.
- Include screenshots, a recording, or other supporting material where useful.

## Execution flow

- Summarize the code's execution flow as a simple pseudocode diagram using arrows and branches.
- Use at most 10 short steps. Focus on inputs, decisions, actions, and outcomes—not implementation details.

## How to test

- Give numbered steps to reproduce and validate the new functionality.
- State the expected results and include supporting material where needed to make validation quick and clear.

## Before merging

- List required environment variables, migrations, and other prerequisites.
- Write **None** when no prerequisites apply.

## After merging

- List deployment steps, environment variables, dashboard configuration, and follow-up actions.
- Write **None** when no actions apply.

## [Vendure Best Practices](https://github.com/Pinelab-studio/pinelab-engineering/blob/main/docs/best-practices/vendure-best-practices.md)

Apply these guidelines when implementing or reviewing Vendure code, plugins, or React dashboard extensions. They cover code structure, event handling, logging, and other implementation constraints.

# General guidelines

- Place resolvers only in `resolvers: []`, not in `providers: []`.
- Make behaviour configurable through a strategy when consumers may reasonably need to change it; avoid speculative abstractions. For example, expose `sendToAnalytics: (order) => ...` so consumers control how orders are formatted and sent.
- Do not update order custom fields in `OrderPlacedEvent` handlers. Concurrent updates by Vendure and the handler can cause race conditions, leaving orders unsettled or active.
- Always prefer built-in or ready-made config, strategies, services, React components, etc. supplied by Vendure over creating new custom code, even if that means sacrificing some functionality.

# Vendure code structure

- `api/` — GraphQL schema extensions (api-extensions.ts), resolvers and controllers
- `config/` — Vendure configuration, strategies, promotions and shipping strategies. Anything that goes in `vendureConfig` goes here.
- `entities/` — TypeORM entity definitions
- `services/` — Business logic (NestJS @Injectable() services)
- `types.ts` — Plugin options interface and shared type definitions
- `constants.ts` — Injection token symbols, logger context
- `<plugin-name>.plugin.ts` — @VendurePlugin decorator that registers everything

# No async/await in `eventBus.subscribe()`

NEVER use `async/await` inside `eventBus.ofType(*ProductEvent*).subscribe(`. Always wrap the function inside in a `.catch`.

```ts
// NEVER do this, bad
eventBus.ofType(*ProductEvent*).subscribe(async () =>
    await someAsyncFunction();
)

// Do this instead, this is good
eventBus.ofType(*ProductEvent*).subscribe(async () =>
    someAsyncFunction().catch((e) => Logger.error(`Failed: ${e.message}`, loggerCtx, e.stack));
)
```

# Logger

- Use the Vendure built-in logger
- Include `order.code` in order-related messages for traceability; use identifiers such as `product.id` or `variant.sku` for other entities.
- Include `e.stack` for errors

```ts
Logger.error(`It failed for order ${order.code}`, loggerCtx, e.stack); // Always use the nearest loggerCtx you can find
Logger.info(`Order ${order.code} has been processed`, loggerCtx);
```

# Testing

- Prefer GraphQL queries and mutations; call services directly when this makes cases more readable or isolated.
- Never use arbitrary sleeps such as waiting ten seconds. Wait for an observable condition, for example `await waitFor(() => someCondition === true)`, using the test framework's bounded condition-wait helper.
- Always use `gql` from `graphql-tag` for GraphQL documents in tests. Never use `/* GraphQL */` template literal comments.
- Place graphql queries at the bottom of the file, below test cases.

# Vendure documentation

Always search the Vendure docs for existing configurations and strategies before implementing your own.

1. Use the `mcp__vendure_docs__search_docs` tool to search the Vendure docs. Notify me when I dont have it.
2. Prefer the MCP tool for any Vendure-specific concepts (configuration, strategies, plugins, GraphQL, etc.).