From 3b127c3eb3663c6a0dd44a590cbbeadd2343de26 Mon Sep 17 00:00:00 2001 From: Jackie Li Date: Fri, 16 Oct 2026 12:59:12 +0100 Subject: [PATCH 1/2] fix: drop the unreachable interface-key lookup in getArg addArg registers each value under its dynamic type, so the registry never has an interface key and the exact lookup for an interface parameter could not hit. Correct the doc line that promised it. Claude-Session: https://claude.ai/code/session_01Dp3Jmu2ePyCME2yG9yd672 --- args.go | 13 +++++-------- docs/dependency-injection.md | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/args.go b/args.go index da49edc..2c80695 100644 --- a/args.go +++ b/args.go @@ -22,16 +22,13 @@ func (args argRegistry) addArg(v any) error { return nil } -// getArg finds the registered value for a parameter of type pt. A -// registration of exactly that type wins (a registered pointer also fills -// its value type). Failing that, an interface parameter takes the one -// registered value whose type implements it; more than one is an error, -// since picking one would be a guess. found is false when nothing matches. +// getArg finds the registered value for a parameter of type pt. Values +// are registered under their dynamic types, so an interface parameter +// takes the one registered value whose type implements it; more than one +// is an error, since picking one would be a guess. A concrete parameter +// goes through getConcreteArg. found is false when nothing matches. func (args argRegistry) getArg(pt reflect.Type) (v reflect.Value, found bool, err error) { if pt.Kind() == reflect.Interface { - if iv, ok := args[pt]; ok { - return iv, true, nil - } return args.implementationOf(pt) } v, found = args.getConcreteArg(pt) diff --git a/docs/dependency-injection.md b/docs/dependency-injection.md index 6bc6c0a..2754ab7 100644 --- a/docs/dependency-injection.md +++ b/docs/dependency-injection.md @@ -77,7 +77,7 @@ Each `WithArgs` value is registered under its type, and a parameter takes the re props, err := usersPage{}.Props(req, fakeUsers{}) ``` - If two registrations implement the interface, `Mount` fails and names both: declare the concrete type, or narrow the interface. A registration of the parameter's exact type, and the per-request values in the table, are always used first. + If two registrations implement the interface, `Mount` fails and names both: declare the concrete type, or narrow the interface. The per-request values in the table are always used first: a `Props(w io.Writer)` gets the response writer, whatever is registered. - **Generic types work** — type parameters, slices and maps as dependencies, aliases, function types and complex constraints. Anywhere these docs say "type", read it as "any reflect-distinguishable type". `Mount` checks every `Props` and `ServeHTTP` parameter for ambiguity up front, so two registrations implementing one interface parameter fail at startup with the page and both types named, not on the first request to that page. `Parse` skips the check. From 9ebfc1d527cb2332f72cb0f27f2c6aba64e3a24b Mon Sep 17 00:00:00 2001 From: Jackie Li Date: Fri, 16 Oct 2026 13:03:14 +0100 Subject: [PATCH 2/2] fix: a registered pointer to an interface fills that interface again Before interface filling, WithArgs(&s) with s of interface type Svc filled a Svc parameter through the pointer-to-value rule. The new interface path skipped it, since *Svc doesn't implement Svc. Look for a registered *Iface first: that restores the old behaviour and gives a way to pick one implementation when several are registered. Docs: Mount's ambiguity check covers Props and extended ServeHTTP only (Page/component params fail at request time), the error names every implementation, and the check knows the writer only as http.ResponseWriter. Claude-Session: https://claude.ai/code/session_01Dp3Jmu2ePyCME2yG9yd672 --- CLAUDE.md | 2 +- args.go | 7 ++++++- di_interface_test.go | 18 ++++++++++++++++++ docs/dependency-injection.md | 13 +++++++++++-- skills/structpages/SKILL.md | 4 +++- skills/structpages/reference.md | 2 +- 6 files changed, 40 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 24ca331..94288b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,7 @@ go tool gsx fmt -w . - Apply globally via `WithMiddlewares` or per-page via a `Middlewares()` method (also applies to descendants) 6. **Dependency Injection**: Type-based DI via `WithArgs(...)` - - `args.go`: `argRegistry` matches by type; a registered pointer fills a value param (not the reverse); an interface param takes the one registration implementing it, and two are an error + - `args.go`: `argRegistry` matches by type; a registered pointer fills a value param (not the reverse); an interface param takes a registered `*Iface` first, else the one registration implementing it, and two are an error - Each registered type appears once; use named types to disambiguate - `Mount` checks every `Props`/extended `ServeHTTP` param for ambiguity up front (`checkInjection`); `Parse` skips it. A missing dependency stays a request-time error, since pages may be mounted in configurations that don't register their services diff --git a/args.go b/args.go index 2c80695..b889ae6 100644 --- a/args.go +++ b/args.go @@ -23,12 +23,17 @@ func (args argRegistry) addArg(v any) error { } // getArg finds the registered value for a parameter of type pt. Values -// are registered under their dynamic types, so an interface parameter +// are registered under their dynamic types, so the only way to register +// under an interface type is a pointer to an interface variable; that +// fills an interface parameter first. Otherwise an interface parameter // takes the one registered value whose type implements it; more than one // is an error, since picking one would be a guess. A concrete parameter // goes through getConcreteArg. found is false when nothing matches. func (args argRegistry) getArg(pt reflect.Type) (v reflect.Value, found bool, err error) { if pt.Kind() == reflect.Interface { + if pv, ok := args[reflect.PointerTo(pt)]; ok { + return pv.Elem(), true, nil + } return args.implementationOf(pt) } v, found = args.getConcreteArg(pt) diff --git a/di_interface_test.go b/di_interface_test.go index 2c01b82..3ccc4ca 100644 --- a/di_interface_test.go +++ b/di_interface_test.go @@ -79,6 +79,24 @@ func TestInjection_exactRegistrationWins(t *testing.T) { } } +// TestInjection_pointerToInterfaceIsUsedFirst pins that registering a +// pointer to an interface variable fills that interface, ahead of (and +// without ambiguity against) the registered implementations. +func TestInjection_pointerToInterfaceIsUsedFirst(t *testing.T) { + var n namer = &pgStore{name: "chosen"} + mux := http.NewServeMux() + _, err := Mount(mux, namerRoot{}, "/", "App", + WithArgs(&n, &pgStore{name: "pg"}, &memCache{})) + if err != nil { + t.Fatalf("Mount: %v", err) + } + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/namer", http.NoBody)) + if rec.Body.String() != "chosen" { + t.Errorf("GET /namer = %q, want %q", rec.Body.String(), "chosen") + } +} + // writerPropsPage asks for an io.Writer, which the per-request // http.ResponseWriter fills before any registration is considered. type writerPropsPage struct{} diff --git a/docs/dependency-injection.md b/docs/dependency-injection.md index 2754ab7..34ec95b 100644 --- a/docs/dependency-injection.md +++ b/docs/dependency-injection.md @@ -77,10 +77,19 @@ Each `WithArgs` value is registered under its type, and a parameter takes the re props, err := usersPage{}.Props(req, fakeUsers{}) ``` - If two registrations implement the interface, `Mount` fails and names both: declare the concrete type, or narrow the interface. The per-request values in the table are always used first: a `Props(w io.Writer)` gets the response writer, whatever is registered. + If more than one registration implements the interface, that's an error naming them all. Declare the concrete type, narrow the interface, or pick one by registering a pointer to an interface variable, which fills that interface ahead of the implementations: + + ```go + var users userLister = pgStore + structpages.WithArgs(pgStore, cacheStore, &users) + ``` + + The per-request values in the table are always used first: a `Props(w io.Writer)` gets the response writer, whatever is registered. - **Generic types work** — type parameters, slices and maps as dependencies, aliases, function types and complex constraints. Anywhere these docs say "type", read it as "any reflect-distinguishable type". -`Mount` checks every `Props` and `ServeHTTP` parameter for ambiguity up front, so two registrations implementing one interface parameter fail at startup with the page and both types named, not on the first request to that page. `Parse` skips the check. +`Mount` checks every `Props` and extended `ServeHTTP` parameter for ambiguity up front, so an interface parameter more than one registration implements fails at startup with the page and the types named, not on the first request to that page. `Init` and `Middlewares` run during `Mount`, so their ambiguity fails it too. Parameters of `Page` and other component methods aren't checked ahead; their ambiguity fails the request. `Parse` skips the check. + +The check knows the writer only as `http.ResponseWriter`. A parameter of another interface the real writer happens to implement, such as `io.StringWriter`, is filled by the writer at request time, but `Mount` fails if two registrations also implement it. Declare `http.ResponseWriter` or `io.Writer` for the writer. A parameter that matches nothing fails the request with `method X requires argument of type T, but not found`. That isn't checked at `Mount`, so a page can stay mounted in a configuration, or a test, that doesn't register its dependencies. diff --git a/skills/structpages/SKILL.md b/skills/structpages/SKILL.md index a87a771..cd36734 100644 --- a/skills/structpages/SKILL.md +++ b/skills/structpages/SKILL.md @@ -379,7 +379,9 @@ components. Each type may be registered once (use named types to register two va A registered pointer also fills a value parameter, but a registered value does not fill a pointer parameter, so register services as pointers. An interface parameter takes the one registration that implements it, so a `Props` can declare a narrow interface and a test can call it with a fake; two -implementations fail `Mount` (`Parse` doesn't check). A missing dependency fails the request, not `Mount`. +implementations of a `Props`/`ServeHTTP` parameter fail `Mount` (`Parse` doesn't check). Register a +pointer to an interface variable (`var s Store = pg; WithArgs(&s)`) to pick one. A missing dependency +fails the request, not `Mount`. `*PageNode` is always injectable. Application services go here, not in globals. ## 8. Testing renders with a bare context diff --git a/skills/structpages/reference.md b/skills/structpages/reference.md index ec0afb8..2634ef7 100644 --- a/skills/structpages/reference.md +++ b/skills/structpages/reference.md @@ -145,7 +145,7 @@ component. | Option | Effect | |---|---| -| `WithArgs(args ...any)` | DI registry. Each type once; duplicates fail `Mount`. An interface parameter takes the one registration implementing it. A parameter two registrations could fill fails `Mount`; one nothing provides fails the request. | +| `WithArgs(args ...any)` | DI registry. Each type once; duplicates fail `Mount`. An interface parameter takes a registered pointer to that interface, else the one registration implementing it. A `Props`/`ServeHTTP` parameter two registrations could fill fails `Mount`; one nothing provides fails the request. | | `WithErrorHandler(func(http.ResponseWriter, *http.Request, error))` | Renders every error from `Props`, `ServeHTTP`, component lookup and render. Default: plain 500. | | `WithMiddlewares(mw ...MiddlewareFunc)` | Global middleware, first is outermost, runs before page `Middlewares`. | | `WithTargetSelector(TargetSelector)` | Chooses the `RenderTarget`. Default `HTMXRenderTarget`; use `HTMXv4RenderTarget` for htmx 4. |