Add fx.From Annotation to allow passing an implementation type as an interface parameter
The most common pattern I hear about to use in go is accept interfaces and return structs. The newest fx framework addition fx.As(...) makes following this paradigm much more intuitive and cleaner in fx modules. But it seems we're only halfway there. I would like to propose a new fx method fx.From(...) which serves as a companion for fx.As(...) to manipulate input types rather than the outputs.
The current functionality works well in situations like this
package a
type Bar interface {...} // Bar is implemented by Foo
type Foo struct {...}
func NewFoo() *Foo {...}
var Module = fx.Options(
fx.Provide(fx.Annotate(
NewFoo, fx.As(new(Bar))),
),
)
package b
type Baz struct {...}
func NewBaz(bar a.Bar) *Baz {...}
var Module = fx.Options(
fx.Provide(NewBaz),
a.Module,
)
But other situations like below may fall short. For example if the interface were in package b generating the following diff to support it would cause a cyclic dependency error. This situation could come up often as it is common in go for the interface to be defined on the caller side.
package a
type Foo struct {...}
func NewFoo() *Foo {...}
var Module = fx.Options(
- fx.Provide(NewFoo),
+ fx.Provide(fx.Annotate(
+ NewFoo, fx.As(new(b.Bar))),
+ ),
)
package b
type Bar interface {...} // Bar is implemented by Foo
type Baz struct {...}
func NewBaz(bar Bar) *Baz {...}
var Module = fx.Options(
fx.Provide(NewBaz),
a.Module,
)
Yes, we could make use of new packages to hold the fx logic and/or interface to break the cycle, but it seems ideal for fx limitations not to enforce package management decisions. Similarly, package a might be part of some external codebase or very old legacy code that no one wants to touch and inserting an fx.As(...) in there years later may create unintended side-effects.
I'd like to propose fx.From(...) which works exactly like fx.As(...) except to aid in manipulating input parameter types rather than outputs.
Taking the second example, it may work like this leaving package a entirely unaware of this dilemma.
package b
type Bar interface {...} // Bar is implemented by Foo
type Baz struct {...}
func NewBaz(bar Bar) *Baz {...}
var Module = fx.Options(
fx.Provide(fx.Annotate(
NewBaz, fx.From(new(Bar))),
),
a.Module,
)
Perhaps more generally, fx.From would let you easily do things like this:
// in a library
type SpeedyJSON struct{}
var Module = fx.Provide(func() *SpeedyJSON { ... })
// for users of that library
fx.Invoke(
fx.Annotate(
fx.From(new(lib.SpeedyJSON)),
Thing,
),
)
// can reuse constructor/invoke target without being type-specific,
// e.g. swap prod and test implementations easily.
func Thing(m json.Marshaler) { ... }
Currently the way to do this with the same interface argument would be to consume and re-provide the implementation as needed, e.g.
fx.Provide(func(s *SpeedyJSON) json.Marshaler {
return s
})
which works fine, but I keep seeing people confused by that construct until they get deeper into understanding Fx and Go's types in general.
That re-provide pattern also means that you can't provide more than one json.Marshaler, e.g. to depend on the interface but inject both *SpeedyJSON and the stdlib marshaler for different purposes in different locations (maybe *SpeedyJSON has restrictions that don't make it suitable for some pieces of code). Though here too there's an already-available workaround with name:"..." tags.
This is a fair request. A couple of questions regarding the API surface for clarification:
- What would happen when the annotated function has multiple arguments? I'm guessing you'd pass the
Fromannotation once in this form?
fx.Annotate(func(p1 Interface1, p2 Interface2) Out {
// ...
}, fx.From(new(Struct1), new(Struct2)))
- What behavior do we want for embedded interfaces? Should we allow conversion using
Fromfor these?
yeah, that matches what fx.As does, seems best to mirror it: https://github.com/uber-go/fx/blob/7bb5b404dfed59b9ebc97e5e8bfaf37f6b1affce/annotated.go#L228-L241
embedded interfaces: you mean like this?
type stringable struct {
fmt.Stringer
also string
}
I assume you can return a stringable and fx.As(new(fmt.Stringer)), and that just treats it as if your func returned a fmt.Stringer in the first place?
Seems fine to do that for fx.From too. From would need to be the type that's in the container of course, but as long as it's assignable to the func argument(s) it seems valid to me.
I suppose there's a bit of an open question about downcasting, e.g. should you allow func(stringable), fx.From(new(fmt.Stringer)), as it's a bit different than the rest of fx seems to do...
Since it's not ambiguous, it probably would be worth supporting? And it kinda seems like something that might become a feature request later, for the same kind of reasons as started this issue. Doing a manual func(i iface) { actual(i.(concrete)) } wrapper would achieve the same thing with the same safety (panic if wrong), but requires a wrapper func.
Just to confirm something: I think the original proposal had a typo. In it, the interface was supplied to fx.From.
fx.Provide(fx.Annotate(
NewBaz, fx.From(new(Bar))),
),
// Where,
// type Bar interface{ ... }
// And,
// func NewBaz(Bar) *Baz
That is undesirable because there should not be any uncertainty in which implementation of an interface would be chosen. @rossb83, did you mean for it to match @Groxx's variant where the implementation is specified?
fx.Annotate(
fx.From(new(lib.SpeedyJSON)),
Thing,
),
// Where,
// type SpeedyJSON struct
// And,
// func Thing(m json.Marshaler)
@abhinav and @Groxx that is a great suggestion, my original thoughts were to specify the implementation but I just didn't know how to do that in "fx". This makes more sense. Can we amend the proposal to follow this API? In essence this is a bit of a quicker way to write a wrapper method to supply an interface and return a struct ptr.
ah, yeah, I didn't even notice that.
to be clear: my thinking has been "As refers to type-to-put-in-container, so From refers to type-in-container". and then "if X is assignable to Y, good enough" for the conversion between signature/container types. no other implicit conversions at all.
ok so just for clarity's sake based on these comments this is how it would work based on my original example
package a
type Foo struct {...}
func NewFoo() *Foo {...}
var Module = fx.Options(
fx.Provide(
NewFoo
),
)
package b
type Bar interface {...} // Bar is implemented by Foo
type Baz struct {...}
func NewBaz(bar Bar) *Baz {...}
var Module = fx.Options(
fx.Provide(
fx.Annotate(
NewBaz,
fx.From(new(*a.Foo)),
),
),
a.Module,
)
Yes, that looks right @rossb83. Looking forward to the PR. Thanks!
created a draft pull request https://github.com/uber-go/fx/pull/938
Done with #990