diff --git a/DECISIONS.md b/DECISIONS.md index 546df4e..003e4d4 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,7 @@ # Decisions +- **2026-05-17** (m+git@andri.dk) `gastro watch --asset CMD` — reload-aware sibling of `--build`. Bug surfaced from a real adopter workflow: when editing a component template body to add a new Tailwind utility class (e.g. `class="size-12"`), the change is classified as RELOAD-class by `internal/watcher/watcher.go` `ClassifyChange` (frontmatter unchanged, only body diff), so the running app keeps serving and devloop fires `OnReload` to refresh the browser. `--build` commands are wired into `gastro watch`'s `OnRestart` hook only (cmd/gastro/watch.go:436 pre-change), so a Tailwind step placed in `--build` was being silently skipped on every body-only edit — the browser refreshed with stale CSS and the new class did nothing. Touching frontmatter or restarting the dev server were the only workarounds. Three solution shapes were weighed: (a) escalate body-edit changes to RESTART-class — rejected, defeats the whole RELOAD optimization that makes body-only edits feel instant; (b) bake Tailwind support into gastro — rejected, opinionated and would have to duplicate for esbuild, image opt, fingerprint hashing, etc.; (c) generic pre-reload hook + `--asset CMD` flag — adopted, generic over any content-derived asset generator and composes cleanly with the existing `--build`/`--run` model. Implementation: new `Config.PreReload func(ctx) error` in `internal/devloop/devloop.go` that runs between `Generate` and `OnReload` on reload-class changes; failure returns an error which suppresses the `OnReload` signal for that cycle so the browser doesn't refresh onto a half-rebuilt asset state (the build-error signal is written separately by the caller via the same channel `--build` uses today). New `--asset CMD` / `-a CMD` flag on `gastro watch` (repeatable, like `--build`). On RESTART-class change the asset chain runs first, then the build chain, then the app restarts; on RELOAD-class change the asset chain runs, then the reload signal fires. `--build` and `--asset` execution share a new `runChain(ctx, label, cmds)` helper in `cmd/gastro/watch.go` so failure semantics and the `.gastro/.build-error` writeback are identical between the two. The output-collision warning at `cmd/gastro/watch.go:323` extends to `--asset` so a stray `--asset 'go build -o tmp/app'` would still warn about reload-loop risk (the existing `suspectedBuildOutputCollision` helper already exempts `static/` so legitimate Tailwind setups don't trip). `gastro dev` continues to reject all flags except `--watch GLOB`; the rejection message was updated to mention `--asset` alongside `--build`/`--run` so users who hit it have a clearer escape hatch. **Tests** (all `-race`-clean): 3 new devloop tests (`PreReload` fires between Generate and OnReload on body change; PreReload error suppresses OnReload; nil PreReload preserves the historical `gastro dev` path), 5 new parser tests (`--asset` single + repeated, `--asset`/`--build` coexistence, value-required error, `-a`/`--asset` rejection from `gastro dev`), 2 new integration tests (asset chain fires on RELOAD-class body edit; asset chain failure suppresses the reload signal AND writes `.gastro/.build-error`). **Migration of the in-repo example workflow:** `scripts/dev/example-website` moves `tailwindcss` from `--build` to `--asset`. **Docs:** `docs/dev-mode.md` gains an "Asset chain vs build chain" subsection with the two-row decision table and a worked Tailwind example; `gastro watch --help` text and the help-page constant in `cmd/gastro/watch.go` document both flags side-by-side with a clear NOTE on `--build` calling out the RESTART-only behaviour. **What this is not:** not a Tailwind integration (the framework stays composition-friendly; `--asset` is generic over any shell command), not a change to the classification model (`ClassifyChange` is unchanged — body edits stay RELOAD), not a `gastro dev` feature (zero-config dev mode stays zero-config; users who need asset pipelines reach for `gastro watch`, which is the documented escape hatch). **Pre-existing issues addressed:** none flagged. + - **2026-05-14** (m+git@andri.dk) `WithRequestFuncs` — per-request template helpers. Plan: `tmp/withrequestfuncs-plan.md`. Ships a fourth option-tier next to the existing `WithFuncs` (static), `WithMiddleware` (request wrapper), and `WithDeps[T]` (DI): `gastro.WithRequestFuncs(binder func(*http.Request) template.FuncMap)`. The binder runs once per request and the closures it returns capture request state — enabling `{{ t "Welcome" }}` (i18n), `{{ csrfField }}` (CSRF tokens), `{{ cspNonce }}` (CSP nonces), named-route reversal, asset hashing, feature flags, anything that needs to read per-request state at template time. The feature lands across two PRs: PR1 (#29, this entry) ships the framework change — runtime, codegen, LSP, reference docs in `docs/helpers.md`; PR2 (#30) ships the dev-watcher `--watch` flag, three example consumers (`examples/i18n/`, `examples/csrf/`, `examples/csp/`), and follow-ups merged via PR #32 (nested-component request propagation lifted from PR1's deferred list; LSP hover, go-to-def, and an info-level diagnostic on non-literal binder sites; binder/component-name shadowing rejected at probe time; nested-clone benchmarks). **Resolved decisions** (8, from the plan's §10 log): D1 probe context — adopter's problem (binders and their `FromCtx` accessors must tolerate empty contexts; Gastro injects no sentinel values, keeping the public surface minimal); D2 binder runtime panic — recover + log with binder index + 500 (one bad binder cannot crash the server, matches typical Go HTTP middleware behaviour); D3 dev-mode binder path — skip Clone, apply funcs to the per-request fresh parse directly (saves the Clone allocation in dev where templates re-parse anyway); D4 `Render.With(r)` lifetime — reusable within a single request, not goroutine-safe, not retained across requests (best ergonomics for SSE/multi-fragment handlers without overcommitting future internals); D5 built-in shadowing — forbidden, collisions panic at `New()` with both sources named (no silent override of `upper`, `lower`, `dict`, etc.); D6 dev-watcher PO defaults — no magic, adopter passes `gastro dev --watch "i18n/*.po"` (drops the originally-proposed default-in if `i18n/` exists; most explicit); D7 LSP non-literal binder UX — silent degrade in completion/hover plus an info-level diagnostic on the binder call site naming the limitation (helpers still resolve at runtime; shipped in PR #32); D8 PR shape — two PRs, this is PR1. **Rejected alternatives** (explicitly out of scope, recorded for posterity): `pkg/gastro/i18n/`, `pkg/gastro/csrf/`, `pkg/gastro/csp/` helper packages (each would be ~95% generic Go that mature alternatives already provide — gotext, gorilla/csrf, etc. — carrying them in Gastro is scope creep for ~15 LOC of glue per consumer; example apps make the architectural argument better than a helper package would); `gastro i18n extract` CLI subcommand (xgettext covers most adopters); `markdownLocalized` directive (multi-call branching in the recipe is honest about what's happening for N=2 or N=3 locales); `langPath` framework helper (lives in the recipe as ~10 LOC of copy-paste sample code); per-locale pre-parse alternative (template C2 from the brainstorm; Clone path is cheap enough, revisit only if profiling shows Clone is pathological); declared keys on the option (LSP discovery via AST scan covers this cheaper). **Implementation — PR1 (4 commits).** _Phase 1, runtime._ New `WithRequestFuncs` option emitted into the generated `routes.go`; multiple binders compose; helper names unique across the union of built-ins, `WithFuncs`, and all binders — duplicates panic at `New()`. New `Router.__gastro_renderPage(name, w, r, data)` method on the codegen-emitted Router struct; zero binders → falls through to the same direct-Execute path as before, no Clone allocation; binders present → builds the per-request FuncMap, clones the cached template in prod (or applies funcs to the fresh dev parse), buffers Execute output, then writes. Probe at `New()` invokes each binder with a synthetic `httptest.NewRequest` to discover its key set; closure bodies inside the returned FuncMap are NOT invoked during probing, only the map's keys are read; recover wrapper around binder invocation and template execution converts panics into a logged 500. Page handler call site changes once: from inline `__gastro_getTemplate(name).Execute(w, data)` to `__router.__gastro_renderPage(name, w, r, __data)`. _Phase 2, `Render.With(r)`._ Adds `(*renderAPI).With(req *http.Request) *renderAPI` on the codegen-emitted Render API — lets SSE handlers and custom HTTP handlers produce HTML with request-aware helpers in scope. Implementation threads the request via an internal `__gastro_request` sentinel key in the propsMap (pulled out before `MapToStruct` like the existing `Children` key, so it never reaches user code), avoiding the need to duplicate frontmatter into a sibling `componentXForRequest` method. The renderAPI struct gains a `req *http.Request` field; each generated `Render.X(props)` method injects the sentinel when req is non-nil. The `FindFrontmatterStart` anchor for components-without-Props shifted from `_ = __children` to `_ = __gastro_req` because the new sentinel block sits last before frontmatter. _Phase 2 also lifts the originally-deferred "nested-component request propagation" limitation._ Components called transitively from a page template via `{{ Component . }}` or `{{ wrap Component (dict ...) }}` now inherit request-aware helpers via a new `__gastro_componentClosuresForRequest(name, r)` codegen helper that emits per-template closures capturing r and threading the sentinel into child propsMaps. Wrap-block slot bodies inherit too because `__gastro_render_children` is now installed as a per-request closure over the clone (in prod) or fresh parse (in dev) rather than over the static registry copy. _Phase 3, LSP._ New `internal/lsp/server/request_funcs.go` AST-scans the project's `main.go` for `gastro.WithRequestFuncs(...)` calls; two binder shapes recognised statically (inline function literal returning `template.FuncMap{...}`; reference to a top-level func in the same file — one-hop resolution; same FuncMap-return extraction). Anything else is recorded as a non-literal site for the future D7 diagnostic surface; helpers from non-literal binders don't appear in completion (silent degrade by design). Lookup is cached per project root, invalidated by main.go modtime. New `ParseTemplateBodyWithRequestFuncs(body, uses, names)` and `FuncMapCompletionsWithRequestFuncs(snippet, names)` thread discovered names into template parse stubs (so `{{ t "..." }}` parses without spurious "function not defined" errors) and completion items (detail string "request-aware helper"). Wired into completion, hover, definition surfaces. LSP shadow Router stub mirrors the new methods. _Phase 5d (PR1 portion), docs._ `docs/helpers.md` gains a substantial "Request-aware Helpers" section: the binder contract, runtime recovery, the three-axis worked-examples table, and the editor-support notes. **Implementation — PR2** (separate branch `feat/with-request-funcs-examples`, builds on this work): dev-watcher `--watch GLOB[,GLOB...]` flag for both `gastro dev` and `gastro watch` (the sole flag now allowed on `gastro dev`), three example apps end-to-end (`examples/i18n/` with the hand-rolled PO loader and the full locale-detection middleware; `examples/csrf/` with mixed return types in one binder — `csrfToken` string + `csrfField` template.HTML — and double-submit cookie verification; `examples/csp/` with helper-to-middleware coordination via the `Content-Security-Policy: nonce-X` header agreement with the rendered `