diff --git a/DECISIONS.md b/DECISIONS.md index 6ea6055..e24a19d 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,7 @@ # Decisions +- **2026-05-02** (m+git@andri.dk) Wave 1 — component name collision warning (A7) and per-router dev mode (B2). Implements the first wave from `plans/frictions-plan.md` §3. A7: a pre-pass in `internal/compiler/compiler.go` `Compile()` detects when two component files produce the same exported name (e.g. `components/post-card.gastro` and `components/post/card.gastro` both yield `PostCard`), emitting a `FileWarning` with both file paths; in strict mode (`gastro generate` / `gastro build` / `gastro check`) the warning is promoted to an error. New `RelPath` field on `componentMeta` carries the path for diagnostics. Three tests: collision warning, strict-mode error, and no-false-positive for distinct names. B2: new `WithDevMode(bool)` option on `New()` that overrides the `GASTRO_DEV` environment variable. The `config` struct grows a `*bool` field so absent vs explicit-false are distinguishable; `New()` falls back to `gastroRuntime.IsDev()` when `WithDevMode` is not called. Documented in `docs/pages.md` §"Forcing dev or production mode". Pre-existing issue fixed: all four `examples/` had missing `go.sum` entries for transitive chroma/goldmark dependencies following the markdown-directive merge; `go get` + `gastro generate` resolved them. All four examples pass `gastro check`. Full `go test -race ./...` green. + - **2026-05-02** (m+git@andri.dk) Track B: page model v2 — ambient `(w, r)` + conditional render. Implements the foundational refactor recorded in `plans/frictions-plan.md` §4. Pre-Track-B pages were GET-only with the request reachable through `gastro.Context()`; mutation handlers had to live in `main.go` and shared no source file or locals with the page that GETs the same URL. SSE-heavy apps paid an interaction-by-interaction maintenance tax for that split. Track B reshapes the page handler model: (1) auto-routes lose the `GET` method prefix — `pages/board.gastro` registers `/board` for every method, frontmatter branches on `r.Method`; (2) frontmatter has ambient `w http.ResponseWriter` and `r *http.Request`, no marker required; (3) the codegen-generated handler wraps `w` in a new unexported `*gastroWriter` (in `pkg/gastro/page_writer.go`) that tracks `headerCommitted` and `bodyWritten` independently, with capability preservation via a four-combination concrete-type pattern (base, +Flusher, +Hijacker, +both) chosen at construction — `http.Pusher` is intentionally not preserved (Q3b: HTTP/2 server push is dead in browsers, the modern replacement is HTTP 103 Early Hints which needs no wrapper); (4) after frontmatter completes, the handler reads `gastroRuntime.BodyWritten(w)` and **conditionally skips the template render** if a body has been committed — status from `WriteHeader` alone is preserved so `WriteHeader(201)` followed by template render emits 201 + rendered HTML; (5) `gastro.Context()` is deprecated (build warning, two-minor-release window per `docs/contributing.md` Deprecation Policy) — the marker still works because the consolidated rewriter in `internal/codegen/generate.go` rewrites `gastro.Context()` to `gastroRuntime.NewContext(w, r)` while emitting the warning; (6) the implicit `gastro.X` namespace is now a finite allowlist (Props, Context, From, FromOK, FromContext, FromContextOK, NewSSE, Render) — anything else produces an "unknown gastro runtime symbol" warning; (7) the `From(*Context)` / `FromOK(*Context)` accessors are removed (pre-1.0 churn per the 2026-04-26 BC posture) — only `FromContext`/`FromContextOK` remain, and the rewriter aliases the page-friendly `gastro.From[T]` to `gastroRuntime.FromContext[T]`; (8) a new shared syntactic AST analyser (`internal/analysis/respwrite.go`) flags any frontmatter call writing to `w` (or `http.Redirect(w, r, …)`) without a following `return` — reported via `info.Warnings`, dev warns, generate/check/build (strict) escalate to errors, matching the dict-key precedent; (9) `gastro.Recover` consults the wrapped writer and logs only when the body is already committed, instead of double-writing a 500 page over a partial response; (10) the codegen dedupes user imports against the auto-injected set (`net/http`, `log`, `html/template`, `bytes`) so frontmatter that imports `"net/http"` for `http.Error` doesn't collide with the generated import block. The plan's literal "last statement of any enclosing block" exemption in the missing-return rules was tightened to "return follows in the same block, or last statement of the synthetic function body"; the looser reading would silently accept `if cond { http.Error(w, …, 400) }` followed by more frontmatter, defeating the analyser's stated purpose. Documented as a deviation in the analyser's doc comment. The LSP shadow gets the minimal Track B treatment per §4.7 (acknowledged limitation): the synthetic wrapper now takes `(w http.ResponseWriter, r *http.Request)` so frontmatter that calls `r.Method` / `w.WriteHeader` type-checks; the existing `var gastro = struct{ Context func() interface{} }{}` stub stays in place rather than rewriting to runtime symbols, so `gastro.From[T]` and friends still surface as unresolved in editors — acceptable for v1; revisit if real adopters report misses. All four `examples/` migrated: `examples/blog` (3 pages, switch to `r.PathValue` and `http.Error`), `examples/gastro` (3 pages, drop unused `ctx`), `examples/dashboard` (no migrations needed; audited clean), `examples/sse` (full rewrite of `pages/index.gastro` to handle GET render + POST SSE patch in the same file; new `examples/sse/app/` package holding `*State` so the generated handler can import the dep type that main is unreachable from). End-to-end smoke: built `examples/sse`, GET `/` returned the rendered page with count=0, POST `/` returned the SSE patch with `
1
`. All four examples pass `gastro check` against the new codegen output. Plan items NOT in scope this round: the `action` keyword (rule 2), co-located route declarations and typed `datastar.Patch` returns (held C1), and follow-ups in plan §2 (anti-goals) and §6 (later waves). Held items remain held; Track B partially defuses the simpler form of C1's friction (mutating handlers in `.gastro` files) without inventing DSL. - **2026-04-30** (m+git@andri.dk) Build-time `(dict ...)` prop validation (audit P0 #3). `MapToStruct` silently dropped unknown keys at render time, so a typo'd `(dict "Tite" ...)` produced a blank Card field with no error log — the audit's "blank card in production" footgun. Fix: new `internal/codegen/validate.go` `ValidateDictKeys` that walks the post-`TransformTemplate` body with `text/template/parse`, finds every `{{ X (dict ...) }}` invocation of an imported component, and cross-checks literal string keys against that component's `[]StructField`. Wired into `internal/compiler/compiler.go` via a cheap pre-pass (`gatherComponentSchemas`) that reads each component file's frontmatter once and builds a path-keyed schema map; `compileFile` then runs the validator after `TransformTemplate` and merges the warnings into the existing `info.Warnings` channel. Path-keyed (not name-keyed) so the user-chosen alias `import MyCard "components/card.gastro"` resolves correctly even when two pages import the same component under different names. Three explicit non-warnings: `__children` (compile-time injected by the wrap rewriter), dynamic-keyed dicts where any odd arg isn't a string literal (skip the whole call — can't know what's inside), and components without a Props schema (extra keys ignored at render time, not strictly illegal). Warning-vs-error policy follows the existing convention from `c8fc130` (ctx-without-Context() warning): `gastro generate`, `gastro build`, and `gastro check` run with `Strict: true` so warnings fail those commands; `gastro dev` runs with `Strict: false` so the dev server keeps rendering on a typo and prints the warning to the console. The plan's softer staged rollout (warning-only for one release) was rejected in favour of matching the existing convention — inconsistent strict semantics across warning sources would be more confusing than a one-shot adoption cost. Verified with end-to-end test `TestCompile_DictKeyTypoSurfacesWarning` (audit reproducer: typo "Tite", checks warning text + strict error promotion) and `TestCompile_DictKeyValidationDoesNotFalsePositive` (wrap form + bare call + dynamic key + propless component all pass clean). 14 unit tests in `internal/codegen/validate_test.go` cover alias resolution, nested-in-range/if/else visiting, parse-error bail, and the three explicit skip cases. All four `examples/` regenerate clean. The audit's three P0s are now closed; remaining items (P1/P2) are deferred per the original plan. diff --git a/docs/pages.md b/docs/pages.md index bc71e16..d0db4c7 100644 --- a/docs/pages.md +++ b/docs/pages.md @@ -229,6 +229,34 @@ list of valid patterns when it does not, so typos fail loudly. Page patterns are method-less ("/", "/blog/{slug}") because the page handles every method. +### Forcing dev or production mode + +Gastro detects `GASTRO_DEV=1` at startup; the `gastro dev` command sets +it automatically. When the env var is insufficient, use `WithDevMode`: + +```go +router := gastro.New( + gastro.WithDevMode(true), // force dev mode +) +``` + +`WithDevMode(true)` forces dev mode (template reload + browser auto-reload +SSE endpoint) regardless of `GASTRO_DEV`. `WithDevMode(false)` forces +production mode regardless of `GASTRO_DEV`. When `WithDevMode` is not +called, the default behaviour (checking `GASTRO_DEV`) applies. + +Use cases: + +- **Library mode.** A larger Go application embeds gastro for component + rendering and doesn't want its dev-vs-prod story tangled up with a + framework-owned env var. The host app sets `WithDevMode` from its own + config flag. +- **Tests.** A test that constructs a Router shouldn't depend on whether + `GASTRO_DEV` happens to be set in the surrounding shell. +- **Production debug.** A short-lived production deploy with live-reload + enabled to investigate a template bug, without setting an env var on + the host. + ## Imports Use Go `import` for both packages and components. Component imports diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index b848b33..871e40c 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -78,6 +78,20 @@ func Compile(projectDir, outputDir string, opts CompileOptions) (*CompileResult, return nil, fmt.Errorf("gathering component schemas: %w", err) } + // Pre-pass: detect component name collisions before any per-file work + // writes Go files to disk. Two component files that produce the same + // ExportedName (e.g. components/post-card.gastro and + // components/post/card.gastro both producing "PostCard") would yield + // duplicate type and method names in render.go and overwrite each + // other's per-file Go output. Catch it here with a clear message + // before either failure mode triggers. + for _, w := range findComponentNameCollisions(componentFiles) { + allWarnings = append(allWarnings, w) + if opts.Strict { + return nil, fmt.Errorf("compiling %s: %s", w.File, w.Message) + } + } + for _, relPath := range allFiles { absPath := filepath.Join(projectDir, relPath) result, err := compileFile(absPath, relPath, absProjectDir, outputDir, propsByPath) @@ -185,6 +199,38 @@ func discoverFiles(dir, prefix string) ([]string, error) { return files, err } +// findComponentNameCollisions scans the list of component file paths and +// returns a warning for each path that produces the same ExportedName as +// an earlier path in the list. The first occurrence is left alone; every +// subsequent occurrence with the same name produces a warning. +// +// Derived purely from path strings (no file I/O), so this can run in the +// pre-pass before any per-file work writes Go output. The mapping is the +// same one the codegen uses: HandlerFuncName(path) -> ExportedComponentName. +func findComponentNameCollisions(componentFiles []string) []FileWarning { + if len(componentFiles) < 2 { + return nil + } + seen := make(map[string]string, len(componentFiles)) // ExportedName -> relPath + var warnings []FileWarning + for _, relPath := range componentFiles { + exported := codegen.ExportedComponentName(codegen.HandlerFuncName(relPath)) + if first, ok := seen[exported]; ok { + warnings = append(warnings, FileWarning{ + File: relPath, + Line: 0, + Message: fmt.Sprintf( + "component name collision: %q and %q both produce the exported name %q; rename one of the files to avoid duplicate type and function names in generated code (and per-file .go output overwriting itself)", + first, relPath, exported, + ), + }) + } else { + seen[exported] = relPath + } + } + return warnings +} + // templateMeta holds per-template metadata needed by routes.go to wire up // FuncMaps and initialise the template registry. type templateMeta struct { @@ -482,6 +528,19 @@ type config struct { funcs template.FuncMap deps map[reflect.Type]any overrides map[string]http.Handler + devMode *bool // nil = use GASTRO_DEV env var; non-nil = override +} + +// WithDevMode overrides the GASTRO_DEV environment variable for this Router. +// When set to true, templates are re-parsed from disk on every request +// and the dev-reload middleware is attached — regardless of GASTRO_DEV. +// When set to false, production mode is forced even when GASTRO_DEV=1. +// When not called, the default behaviour (checking GASTRO_DEV) applies. +// +// Calling WithDevMode multiple times keeps the last value (no panic); +// the option is intended to be set once at New() time. +func WithDevMode(dev bool) Option { + return func(c *config) { c.devMode = &dev } } // WithFuncs registers additional template helper functions. @@ -664,8 +723,13 @@ func New(opts ...Option) *Router { opt(cfg) } + isDev := gastroRuntime.IsDev() + if cfg.devMode != nil { + isDev = *cfg.devMode + } + __r := &Router{ - isDev: gastroRuntime.IsDev(), + isDev: isDev, userFuncs: cfg.funcs, deps: cfg.deps, } diff --git a/internal/compiler/compiler_test.go b/internal/compiler/compiler_test.go index ebcb83d..e96ae2b 100644 --- a/internal/compiler/compiler_test.go +++ b/internal/compiler/compiler_test.go @@ -784,6 +784,85 @@ func indexOf(s, substr string) int { return -1 } +// TestCompile_ComponentNameCollisionWarning verifies that two component +// files producing the same ExportedName emit a warning (and a strict-mode +// error). This is Wave 1 / A7 from plans/frictions-plan.md. +func TestCompile_ComponentNameCollisionWarning(t *testing.T) { + projectDir := filepath.Join("testdata", "collision") + outputDir := t.TempDir() + + // Non-strict mode: should succeed with a warning. + result, err := compiler.Compile(projectDir, outputDir, compiler.CompileOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + found := false + for _, w := range result.Warnings { + if contains(w.Message, "component name collision") && contains(w.Message, "PostCard") { + found = true + if w.File == "" { + t.Error("collision warning should include the file path") + } + } + } + if !found { + t.Errorf("expected a component name collision warning for PostCard; got warnings: %+v", result.Warnings) + } +} + +// TestCompile_ComponentNameCollisionStrictError verifies that strict mode +// promotes a component name collision warning to an error AND that the +// error fires before any per-file Go output is written (which would +// otherwise overwrite itself due to the same goFileName collision). +func TestCompile_ComponentNameCollisionStrictError(t *testing.T) { + projectDir := filepath.Join("testdata", "collision") + outputDir := t.TempDir() + + _, err := compiler.Compile(projectDir, outputDir, compiler.CompileOptions{Strict: true}) + if err == nil { + t.Fatal("expected strict mode to error on component name collision") + } + if !contains(err.Error(), "component name collision") { + t.Errorf("error should mention 'component name collision'; got: %v", err) + } + + // The collision check runs in the pre-pass, before any component + // Go file is written. Verify no clobbered components_post_card.go + // landed in the output dir. + if _, statErr := os.Stat(filepath.Join(outputDir, "components_post_card.go")); statErr == nil { + t.Error("strict-mode collision should fail before per-file Go output is written; found components_post_card.go on disk") + } +} + +// TestCompile_NoCollisionWhenNamesDiffer verifies that distinct component +// names produce no collision warnings. +func TestCompile_NoCollisionWhenNamesDiffer(t *testing.T) { + projectDir := t.TempDir() + compDir := filepath.Join(projectDir, "components") + if err := os.MkdirAll(compDir, 0o755); err != nil { + t.Fatal(err) + } + + // Two components with different exported names: Card and Header. + os.WriteFile(filepath.Join(compDir, "card.gastro"), + []byte("---\ntype Props struct { Title string }\np := gastro.Props()\nTitle := p.Title\n---\n
{{ .Title }}
\n"), 0o644) + os.WriteFile(filepath.Join(compDir, "header.gastro"), + []byte("---\ntype Props struct { Title string }\np := gastro.Props()\nTitle := p.Title\n---\n
{{ .Title }}
\n"), 0o644) + + outputDir := t.TempDir() + result, err := compiler.Compile(projectDir, outputDir, compiler.CompileOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, w := range result.Warnings { + if contains(w.Message, "collision") { + t.Errorf("unexpected collision warning: %s", w.Message) + } + } +} + func assertStringContains(t *testing.T, s, substr string) { t.Helper() if !contains(s, substr) { diff --git a/internal/compiler/devmode_integration_test.go b/internal/compiler/devmode_integration_test.go new file mode 100644 index 0000000..91e3a6b --- /dev/null +++ b/internal/compiler/devmode_integration_test.go @@ -0,0 +1,155 @@ +package compiler_test + +// Integration test for Wave 1 / B2: WithDevMode(bool) option overrides +// the GASTRO_DEV env var. The behaviour we want to lock in: +// +// WithDevMode(true) -> Handler() mounts the /__gastro/reload SSE +// endpoint regardless of GASTRO_DEV being unset. +// WithDevMode(false) -> Handler() does NOT mount /__gastro/reload even +// when GASTRO_DEV=1. +// +// The Router.isDev field is unexported, so we observe behaviour through +// the generated Handler(): mount the handler on a test server and probe +// /__gastro/reload. If the response is a 200 with text/event-stream, we +// know dev mode is on; if it's 404, we know it's off. +// +// Compiled in a subprocess (similar to race_integration_test.go) because +// generating test code requires a complete .gastro/ output and a go.mod +// pointing at this repo via replace directive. Gated by testing.Short() +// because it spawns the Go toolchain. + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/andrioid/gastro/internal/compiler" +) + +func TestCompile_WithDevModeOverridesEnv(t *testing.T) { + if testing.Short() { + t.Skip("skipping dev-mode integration test in -short mode") + } + if _, err := exec.LookPath("go"); err != nil { + t.Skip("go toolchain not in PATH") + } + + repoRoot := findRepoRoot(t) + + projectDir := t.TempDir() + pagesDir := filepath.Join(projectDir, "pages") + if err := os.MkdirAll(pagesDir, 0o755); err != nil { + t.Fatal(err) + } + + // Minimal page so routes.go compiles cleanly. + mustWriteFile(t, filepath.Join(pagesDir, "index.gastro"), + "---\nTitle := \"Hi\"\n---\n

{{ .Title }}

\n") + + gastroOut := filepath.Join(projectDir, ".gastro") + if _, err := compiler.Compile(projectDir, gastroOut, compiler.CompileOptions{}); err != nil { + t.Fatalf("compile: %v", err) + } + + mustWriteFile(t, filepath.Join(projectDir, "go.mod"), + "module gastro_devmode_repro\n\n"+ + "go 1.26.1\n\n"+ + "require github.com/andrioid/gastro v0.0.0\n\n"+ + "replace github.com/andrioid/gastro => "+repoRoot+"\n") + + // Subprocess test that constructs Routers with WithDevMode and probes + // the dev-reload SSE endpoint to observe whether dev mode is on. + mustWriteFile(t, filepath.Join(projectDir, "devmode_test.go"), `package devmode_test + +import ( + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + gastro "gastro_devmode_repro/.gastro" +) + +// probeReloadEndpoint mounts r.Handler() on httptest, GETs /__gastro/reload, +// and returns (statusCode, contentType). A 200 with text/event-stream means +// dev mode is on; 404 means dev mode is off. +func probeReloadEndpoint(t *testing.T, h http.Handler) (int, string) { + t.Helper() + srv := httptest.NewServer(h) + defer srv.Close() + + req, err := http.NewRequest("GET", srv.URL+"/__gastro/reload", nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + // Avoid hanging on the SSE stream by using a custom client that closes + // the body immediately. + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do: %v", err) + } + defer resp.Body.Close() + return resp.StatusCode, resp.Header.Get("Content-Type") +} + +// TestWithDevModeTrue_MountsReloadEndpoint: WithDevMode(true) should mount +// /__gastro/reload even when GASTRO_DEV is unset. +func TestWithDevModeTrue_MountsReloadEndpoint(t *testing.T) { + os.Unsetenv("GASTRO_DEV") + r := gastro.New(gastro.WithDevMode(true)) + status, ct := probeReloadEndpoint(t, r.Handler()) + if status != http.StatusOK { + t.Errorf("WithDevMode(true): expected /__gastro/reload to return 200, got %d", status) + } + if !strings.Contains(ct, "text/event-stream") { + t.Errorf("WithDevMode(true): expected SSE content-type, got %q", ct) + } +} + +// TestWithDevModeFalse_OmitsReloadEndpoint: WithDevMode(false) should NOT +// mount /__gastro/reload even when GASTRO_DEV=1. +func TestWithDevModeFalse_OmitsReloadEndpoint(t *testing.T) { + t.Setenv("GASTRO_DEV", "1") + r := gastro.New(gastro.WithDevMode(false)) + status, _ := probeReloadEndpoint(t, r.Handler()) + if status != http.StatusNotFound { + t.Errorf("WithDevMode(false): expected /__gastro/reload to return 404, got %d", status) + } +} + +// TestWithDevModeUnset_FollowsEnv: with WithDevMode not called, behaviour +// follows GASTRO_DEV. +func TestWithDevModeUnset_FollowsEnv(t *testing.T) { + os.Unsetenv("GASTRO_DEV") + r := gastro.New() + status, _ := probeReloadEndpoint(t, r.Handler()) + if status != http.StatusNotFound { + t.Errorf("default (GASTRO_DEV unset): expected /__gastro/reload to return 404, got %d", status) + } + + t.Setenv("GASTRO_DEV", "1") + r = gastro.New() + status, ct := probeReloadEndpoint(t, r.Handler()) + if status != http.StatusOK { + t.Errorf("default (GASTRO_DEV=1): expected /__gastro/reload to return 200, got %d", status) + } + if !strings.Contains(ct, "text/event-stream") { + t.Errorf("default (GASTRO_DEV=1): expected SSE content-type, got %q", ct) + } +} +`) + + cmd := exec.Command("go", "test", "-race", "-count=1", "-run", "TestWithDevMode", "./...") + cmd.Dir = projectDir + cmd.Env = append(os.Environ(), "GOFLAGS=") // strip outer -race etc. + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("go test -race failed: %v\n%s", err, out) + } + if strings.Contains(string(out), "FAIL") { + t.Fatalf("subprocess test failed:\n%s", out) + } +} diff --git a/internal/compiler/testdata/collision/components/post-card.gastro b/internal/compiler/testdata/collision/components/post-card.gastro new file mode 100644 index 0000000..180ffdd --- /dev/null +++ b/internal/compiler/testdata/collision/components/post-card.gastro @@ -0,0 +1,9 @@ +--- +type Props struct { + Title string +} + +p := gastro.Props() +Title := p.Title +--- +
{{ .Title }}
diff --git a/internal/compiler/testdata/collision/components/post/card.gastro b/internal/compiler/testdata/collision/components/post/card.gastro new file mode 100644 index 0000000..02f0592 --- /dev/null +++ b/internal/compiler/testdata/collision/components/post/card.gastro @@ -0,0 +1,14 @@ +--- +type Props struct { + Title string + Author string +} + +p := gastro.Props() +Title := p.Title +Author := p.Author +--- +
+

{{ .Title }}

+

By {{ .Author }}

+