Skip to content

Commit aa89321

Browse files
authored
chore: expand AGENTS.md (#1363)
1 parent 8c31fbc commit aa89321

1 file changed

Lines changed: 76 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,50 @@ Use pnpm. Run from the repo root.
3131
| `pnpm code:checks` | prettier + eslint + tsc. |
3232
| `pnpm fix` | auto-fix prettier + eslint. |
3333
| `pnpm --filter <example> preview` | build + preview an example app end-to-end. Add `SKIP_NEXT_APP_BUILD=true` when only the adapter changed. |
34-
| `pnpm e2e` / `pnpm e2e:dev` | Playwright suites against the example apps. |
34+
| `pnpm e2e` / `pnpm e2e:dev` | Playwright suites against the example apps. See [Running e2e tests](#running-e2e-tests). |
35+
| `pnpm e2e-turbopack` | same, for the examples that also build with Turbopack. |
3536
| `pnpm --filter <example> e2e` | Run a specific example's Playwright suite. |
3637
| `pnpm changeset` | create a changeset for changes. |
3738

39+
`--filter` takes the **package name**, which is often not the directory name: `examples/prisma-7` is `prisma-7-next-app`, `examples/e2e/app-router` is `app-router`. Check the example's `package.json`.
40+
41+
## Running e2e tests
42+
43+
The examples' `e2e` script only runs Playwright. What builds the worker is the Playwright `webServer` command in `examples/common/config-e2e.ts`, and **it behaves differently depending on `CI`**:
44+
45+
| | `CI` unset | `CI=true` |
46+
| --------------------- | ------------------------------- | ----------------------------------------------------- |
47+
| worker build | prepends `pnpm build:worker &&` | none - CI has a preceding `pnpm -r build:worker` step |
48+
| retries | 0 | 2 |
49+
| workers | parallel | 1 |
50+
| `reuseExistingServer` | yes | no |
51+
52+
So `CI=true pnpm e2e` runs against **whatever `.open-next` is already on disk**. After changing the adapter you must either rebuild the workers first (`pnpm -r build:worker`, what CI does) or drop `CI` and let each example rebuild itself.
53+
54+
Recipes:
55+
56+
```sh
57+
# Adapter change, full sweep. Rebuilds every example worker, so it is slow.
58+
PLAYWRIGHT_HTML_OPEN=never pnpm -r --no-bail e2e
59+
60+
# Reproduce CI exactly.
61+
pnpm build && pnpm -r build:worker && CI=true pnpm e2e
62+
63+
# Single example, adapter-only change.
64+
pnpm build
65+
SKIP_NEXT_APP_BUILD=true pnpm --filter <example> build:worker
66+
CI=true pnpm --filter <example> e2e
67+
```
68+
69+
Things that will waste your time:
70+
71+
- **Set `PLAYWRIGHT_HTML_OPEN=never` (or `CI=true`).** Otherwise, on failure Playwright serves the HTML report and waits - the recursive run never returns.
72+
- **`--no-bail`.** `pnpm -r` stops at the first failing package, so one failure hides the rest of the suite.
73+
- **Never pipe a run into `head`.** SIGPIPE kills it mid-way and leaves `wrangler`/`workerd` processes holding example ports; the next run then dies with `http://localhost:8771 is already used`. Redirect to a file and grep it. If it happens: `pkill -f workerd; pkill -f wrangler`.
74+
- **`.wrangler/state` persists between runs.** Cache-sensitive tests (ISR, `enableCacheInterception`) only reproduce CI on a cold cache - CI always starts fresh. A test that passes once and then fails forever is this. `rm -rf examples/*/.wrangler/state examples/*/*/.wrangler/state`.
75+
- **Clean up Playwright artifacts.** `test-results/` and `playwright-report/` are gitignored but not prettier-ignored, so `pnpm code:checks` fails after an e2e run.
76+
- Before claiming a failure is pre-existing, re-check it with the same worker bundle _and_ a clean `.wrangler/state`. Stashing the source is not enough: the example worker is not rebuilt by `CI=true` runs.
77+
3878
# Conventions
3979

4080
- **Strict TypeScript**. Don't loosen; reach for generics or narrowing.
@@ -52,6 +92,23 @@ Use pnpm. Run from the repo root.
5292
- **`src/cli/build/patches/`** contains esbuild plugins and `@ast-grep/napi` transforms that rewrite Next's emitted code to run on Workers. Every patch needs a spec, and ideally a minimal fixture of the input it's matching. Upstream Next changes break these; when a patch stops matching, fix the matcher, don't widen it blindly.
5393
- **Overrides in `src/api/overrides/`** implement contracts defined in `@opennextjs/aws`. Check the upstream type before changing a signature. `@opennextjs/aws` is pinned in `package.json`, so bumping it is a deliberate change with its own changeset.
5494

95+
## Working on patches
96+
97+
A patch that stops matching fails **silently** - the build succeeds and the Worker throws at runtime. So:
98+
99+
- **Assert on the built artifact, not just the spec.** `rg -c '<the call you removed>' examples/<app>/.open-next/server-functions/default/**/handler.mjs` should be 0. A green spec only proves the rule matches the fixture you wrote.
100+
- **Find out what Next actually emits.** The chunks Turbopack/webpack write have sourcemaps with `sourcesContent`, which gives you the original TypeScript of the emitted helper and its module name - far faster than reading minified output:
101+
```sh
102+
node -e 'const m=require("./path/to/chunk.js.map");console.log(m.sources);console.log(m.sourcesContent[0])'
103+
```
104+
- **Pin the version boundary before writing the matcher**, so you know which shapes you must support:
105+
```sh
106+
npm pack next@16.2.12 next@16.3.0 # then untar and diff the emitting template
107+
```
108+
- **`patchCode(code) === code` is not a valid "nothing matched" test.** It reparses and re-prints, so whitespace differs and it always reports a change. Use `applyRule(rule, root).edits.length` when the patch has to be a no-op for non-matching files.
109+
- **`stopBy: end` matches enclosing nodes too.** Two overlapping matches produce overlapping edits in `commitEdits`. Anchor on `field: body` + a direct child when you mean "the innermost function".
110+
- Match on **shape** (arity, `async`, statement position) when the emitted code is minified - names are gone.
111+
55112
# Pre-PR checklist
56113

57114
1. `pnpm code:checks` is clean.
@@ -79,4 +136,22 @@ Format:
79136
- New feature -> `minor`.
80137
- Breaking changes -> `major`.
81138

139+
The changeset is the changelog entry, so write it for a user who hit the bug, not for a reviewer reading the diff: what broke, from which Next.js version, and what the fix does. Say "silently stopped matching" rather than "updated the rule". Include the version boundary and the user-visible error string when there is one - that is what people search for.
140+
141+
The generated filename (`nervous-moons-hide.md`) can be renamed to something descriptive; the repo has both.
142+
143+
## Commit messages
144+
145+
Same `<type>: <imperative title>` as the changeset, and in practice **the commit title is the changeset title**. Keep them identical unless a commit covers several changesets.
146+
147+
```
148+
fix: patch the Turbopack wasm helpers that Next.js 16.3 emits in the chunks
149+
chore: bump Next.js to 15.5.24 and 16.3.3
150+
feat: support Node.js middleware (proxy.ts)
151+
```
152+
153+
- PRs are squash-merged and GitHub appends ` (#1234)`; don't add it by hand.
154+
- `Version Packages (#…)` commits are produced by the changesets bot - never write one.
155+
- Only commit when asked. Stage the changeset with the code (`git add .changeset/*.md`) so the change and its changelog entry land together.
156+
82157
Full rules in [CONTRIBUTING.md](CONTRIBUTING.md).

0 commit comments

Comments
 (0)