encore-code-review

Encore Code Review Instructions

When reviewing Encore.ts code, check for these common issues:

Critical Issues 1. Infrastructure Inside Functions // WRONG: Infrastructure declared inside function async function setup() { const db = new SQLDatabase("mydb", { migrations: "./migrations" }); const topic = new Topic("events", { deliveryGuarantee: "at-least-once" }); }

// CORRECT: Package level declaration const db = new SQLDatabase("mydb", { migrations: "./migrations" }); const topic = new Topic("events", { deliveryGuarantee: "at-least-once" });

1. Using require() Instead of import // WRONG const { api } = require("encore.dev/api");

// CORRECT import { api } from "encore.dev/api";

1. Wrong Service Import Pattern // WRONG: Direct import from another service import { getUser } from "../user/api";

// CORRECT: Use ~encore/clients import { user } from "~encore/clients"; const result = await user.getUser({ id });

1. Missing Error Handling // WRONG: Returning null for not found const user = await db.queryRowSELECT * FROM users WHERE id = ${id}; if (!user) return null;

// CORRECT: Throw APIError import { APIError } from "encore.dev/api";

const user = await db.queryRowSELECT * FROM users WHERE id = ${id}; if (!user) { throw APIError.notFound("user not found"); }

1. SQL Injection Risk // WRONG: String concatenation await db.query(SELECT * FROM users WHERE email = '${email}');

// CORRECT: Template literal with automatic escaping await db.queryRowSELECT * FROM users WHERE email = ${email};

Warning Issues 6. Missing Type Annotations // WEAK: No explicit types export const getUser = api( { method: "GET", path: "/users/:id", expose: true }, async ({ id }) => { return await findUser(id); } );

// BETTER: Explicit request/response types interface GetUserRequest { id: string; } interface User { id: string; email: string; name: string; }

export const getUser = api( { method: "GET", path: "/users/:id", expose: true }, async ({ id }: GetUserRequest): Promise => { return await findUser(id); } );

1. Exposed Internal Endpoints // CHECK: Should this cron endpoint be exposed? export const cleanupJob = api( { expose: true }, // Probably should be false async () => { / ... / } );

2. Non-Idempotent Subscription Handlers // RISKY: Not idempotent (pubsub has at-least-once delivery) const _ = new Subscription(orderCreated, "process-order", { handler: async (event) => { await chargeCustomer(event.orderId); // Could charge twice! }, });

// SAFER: Check before processing const _ = new Subscription(orderCreated, "process-order", { handler: async (event) => { const order = await getOrder(event.orderId); if (order.status !== "pending") return; // Already processed await chargeCustomer(event.orderId); }, });

1. Secrets Called at Module Level // WRONG: Secret accessed at startup const stripeKey = secret("StripeKey"); const client = new Stripe(stripeKey()); // Called during import

// CORRECT: Access inside functions const stripeKey = secret("StripeKey");

async function charge() { const client = new Stripe(stripeKey()); // Called at runtime }

Review Checklist All infrastructure at package level Using ES6 imports, not require() Cross-service calls use ~encore/clients Proper error handling with APIError SQL uses template literals Request/response types defined Internal endpoints have expose: false Subscription handlers are idempotent Secrets accessed inside functions, not at import time Migrations follow naming convention (001_name.up.sql) Output Format

When reviewing, report issues as:

[CRITICAL] [file:line] Description of issue [WARNING] [file:line] Description of concern
[GOOD] Notable good practice observed