Open tyler-at-fast opened 3 years ago
Hey Tyler, thanks for the well-written issue!
Part of what you're suggesting is already possible! In your second example, if you implement the method on the pointer, and return that from the func
, things work as expected:
-func (d Duck) Quack() {
+func (d *Duck) Quack() {
fmt.Printf("Quack quack, I'm %v, quack quack.\n", d.Name)
}
fx.Provide(
func() Name { return Name("tyler") },
- func(d Duck) Quacker { return d },
+ func(d Duck) Quacker { return &d },
),
This lets you have methods on the pointer with public fields on the struct. Does that satisfy the core need?
I'll acknowledge that a couple pieces are missing here that would make things easier:
Support for unexported fields on the fx.In
-tagged struct that Fx ignores. We guard against this today. If this guard was removed, you'd be able to have unexported fields on Duck
which you'd be able to initialize in the func(Duck) Quacker
.
type Duck struct {
fx.In
Name Name
quack string // cached
}
fx.Provide(func(d Duck) Quacker {
d.quack = fmt.Sprintf(...)
return &d
})
However, a trade-off here is that users will assume that unexported fields will be filled by Fx/dig. Perhaps support for unexported fields could be opt-in. I've created https://github.com/uber-go/dig/issues/273 to design and implement this.
An easier means of casting Duck
to Quacker
when the func
is empty outside of return d
. This will be addressed in #673 which we hope to pick up again soon.
A means of specifying a struct should be treated like an fx.In
without modifying it. This would be similar to fx.Extract
except it would specify that specific arguments of a constructor should be treated as fx.In
. An API design to do this isn't obvious to me right now. I've created https://github.com/uber-go/fx/issues/726 to track this.
Minus the above features, ProvideAndPopulateAs
(name TBD) is possible today with something like the following:
func ProvideAndPopulateAs(ptr interface{}, iface interface{}) fx.Option {
ptrt := reflect.TypeOf(ptr)
ifacet := reflect.TypeOf(iface)
// TODO: validate that ptrt is a pointer type
// TODO: validate that ifacet is an interface type
// TODO: validate that ptrt implements ifacet
fn := reflect.MakeFunc(
reflect.FuncOf([]reflect.Type{ptrt.Elem()}, []reflect.Type{ifacet}, false),
func(args []reflect.Value) []reflect.Value {
return []reflect.Value{args[0].Addr()}
},
) // == func(d Duck) Quacker { return &d }
return fx.Provide(fn.Interface())
}
// Usage: ProvideAndPopulateAs(new(Duck), new(Quacker))
If we can figure out a good name and explanation for this feature, we'd be happy to include it in Fx out of the box.
Meanwhile, I hope the sample above helps for now.
Thanks abg! Hope you're doing well.
I think you're right about using the pointer, I had mistakenly implemented it wrong:
fx.Provide(
func() Name { return Name("tyler") },
- func(d Duck) Quacker { return d },
+ func(d *Duck) Quacker { return d },
),
which results in:
Failed: cannot provide function "main".main.func2 (/tmp/sandbox770149436/prog.go:31):
bad argument 1: cannot depend on a pointer to a parameter object, use a value instead:
*main.Duck is a pointer to a struct that embeds dig.In
I tried your suggestion and it works perfectly. Agreed that unexported fields are still an issue, and that dig.As
may help with some of this too.
And 💯 💯 💯 that code snippet looks great, I'm giving it a shot.
Thanks again for the detailed and thoughtful response!
Hello fx-ers, I'm curious if an idea like this has been discussed yet:
Common DI Patterns
Today, I imagine the most common way to construct a concrete type and then depend on the interface is as follows (https://play.golang.org/p/riYmEo2wtsL):
Quacker
with methodQuack()
Duck
(with private fields) with constructorNewDuck
that returns aQuacker
fx.Provide
providesNewDuck
Quacker
and invokesQuack()
Problem: this ends up with some boilerplate still. If you want to add a new field to your
Duck
, you need to copy it in three places: (1) the struct definition, (2) the constructor function signature, and (3) when constructing the concrete type in the return statement.Alternative Available Today
As an alternative, you could make the fields of your concrete type public, and let DI do some of the heavy lifting for you (https://play.golang.org/p/wAewK_sbXK-):
Quacker
with methodQuack()
Duck
that embedsfx.In
and has all public fields.fx.Provide
binds the concrete type to the interfacefunc(d Duck) Quacker { return d }
Quacker
and invokesQuack()
This works, but only because it abuses the
fx.In
statement, by turning the concrete type into a parameter struct. It forces the implementation to be aware of fx, which is not ideal. For example, your concrete type can't have a pointer receiver, making many usages impossible.Alternatives of the Future?
What if we could
Populate()
the concreteDuck
and thenProvide()
it as a dependency that implements theQuacker
interface? That would be pretty magical:Quacker
with methodQuack()
Duck
with all public fields.fx.PopulateAndProvide(&Duck{}, func(d *Duck) Quacker { return d })
(...or something, maybe Supply is better)Quacker
and invokesQuack()
This would effectively populate the
Duck
with all the properties it needs, then provide it as aQuacker
to the fx graph.Is this even idiomatic Go?
Maybe, maybe not. On the one hand, the
Duck
now has these public fields likeName
, which shouldn't be available to the rest of the application. On the other hand, since it's bound to theQuacker
interface in the fx graph, those public fields are not directly accessible without type asserting to the concrete type, at which point you're breaking encapsulation anyway so everything's out the window.But why really?
Recently I migrated a codebase to
fx
, and idioms can vary based on experience/maturity of the application. If a codebase makes significant use of public structs/fields that implement interfaces, then more code must be written to take advantage offx
. And at the end of the day,fx
's goal should be to reduce boilerplate code, not require more of it.🦄 🦄 🦄 tyler