wire icon indicating copy to clipboard operation
wire copied to clipboard

Can't use wire.Bind inside wire.NewSet

Open maratori opened this issue 4 years ago • 5 comments

Describe the bug

Can't use wire.Bind inside wire.NewSet without concrete type provider. Wire fails to generate with error

wire.Bind of concrete type "..." to interface "...", but mySet does not include a provider for "..."

To Reproduce

type AType string
type AInterface interface{}

var mySet = wire.NewSet(
	wire.Bind(new(AInterface), new(AType)),
)

func BuildFromSet(a AType) (AInterface, error) {
	panic(wire.Build(
		mySet,
	))
}

Expected behavior

It should generate the same code as for following

func BuildDirectly(a AType) (AInterface, error) {
	panic(wire.Build(
		wire.Bind(new(AInterface), new(AType)),
	))
}

Version

v0.4.0

maratori avatar Nov 24 '20 10:11 maratori

This is working as intended, since as the Wire User Guide states:

Any set that includes an interface binding must also have a provider in the same set that provides the concrete type.

The API documentation for wire.Bind probably needs to reflect that.

However, I want to make sure I'm not missing a use case here. Why are you splitting up the provider and the interface binding?

zombiezen avatar Nov 27 '20 18:11 zombiezen

Thanks, I missed that in docs.

I don't know use case where current behavior would be blocker. There are couple workarounds.

However it's very strange for me why it differs from other providers?

maratori avatar Nov 28 '20 10:11 maratori

A somewhat contrived use case example:

// pkg1, pkg2 and pkg3 define the subsets of the FullStorage interface they need.
var SharedSet = wire.NewSet(
   wire.Bind(new(pkg1.Storage), new(FullStorage),
   wire.Bind(new(pkg2.Storage), new(FullStorage),
   wire.Bind(new(pkg3.Storage), new(FullStorage),
   // ... more shared steps
)

var App1Set = wire.NewSet(
   provideMysqlStorage,
   wire.Bind(new(FullStorage), *MysqlStorage), // <-- I don't want to copy-paste binds for pkg1, pkg3, pkg3 here
   SharedSet,
   // ... more app1-specific steps that also need storage
)

var App2Set = wire.NewSet(
   providePostgresStorage,
   wire.Bind(new(FullStorage), *PostgresStorage), // <-- and here
   SharedSet,
   // ... more app2-specific steps steps that also need storage
)

drscre avatar Nov 28 '20 16:11 drscre

In some small-scale experiments we did early on in development, we constructed some relatively confusing examples (I forget the exact details) because of the separation between the concrete type provider and the binding. We also wanted provider sets to obey the principle of "accept interfaces, return concrete types". This led to the decision to keep bindings together with their concrete types, since it would be easier to relax the restriction later if we found compelling evidence that this was a mistake.

@dscre's example of splitting a larger interface into smaller ones is interesting. I would have to think about it a bit more, but I'm leaning toward recommending copy-paste, as the example indicates.

zombiezen avatar Nov 28 '20 18:11 zombiezen

@zombiezen I've got another use case to consider. Imagine a single binary that hosts multiple gRPC services, potentially authored by different teams and responsible for different domains. Each individual service offers their own provider set with dependencies that are only required for that service and no other service:

// service_a.go
var Provider = wire.NewSet(
    NewServiceA,
    ServiceARepository,
)

//service_b.go
var Provider = wire.NewSet(
    NewServiceB,
    ServiceBRepository,
)

// ... and on and on

Imagine now that NewServiceA and NewServiceB both use a FeatureFlagClient that we want to provide once in the injector along these lines:

func newApp(sa *serviceA.Service, sb *serviceB.Service) {
    // run the services
}

func injector() {
    panic(wire.Build(featureflags.Provider, serviceA.Provider, serviceB.Provider, newApp))
}

Where this falls apart is if ServiceA and ServiceB both define their own interfaces for the FeatureFlagClient, as one does in Go. I'd personally be happy with the ability to bind the individual service's interface in its own provider to the concrete type that will be provided in the injector:

var Provider = wire.NewSet(
    NewServiceA,
    ServiceARepository,
    wire.Bind(new(FeatureFlagClient), new(*featureflags.Client)),
)

... but this is disallowed by Wire. I'd be fine with @drscre's proposed workaround of binding to an intermediate interface, but that's not possible with Wire, either. I could also use a globally-defined interface for the feature flag client in the services, but now that's going to violate prevailing Go dogma about defining interfaces away from the consumer.

The only option I'm left with is binding every interface in the top-level injector for the binary, which means pulling in imports that I don't need, and adding more lines of code that ultimately aren't relevant to the binary as a whole, and should only be relevant to the individual service providers:

func injector() {
    panic(wire.Build(
        featureflags.Provider,

        wire.Bind(new(serviceA.FeatureFlagClient), new(*featureflags.Client)),
        wire.Bind(new(serviceADependency.FeatureFlagClient), new(*featureflags.Client)),
        serviceA.Provider,

        wire.Bind(new(serviceB.FeatureFlagClient), new(*featureflags.Client)),
        serviceB.Provider,

        newApp,
    ))
}

This gets particularly unwieldy as a project grows. I'd really like to be able to use a provider set as a way to bundle up a service and the dependencies that only that service needs and hand it off to a top-level injector for the binary, where dependencies that should be shared and injected once per-process live.

If there's a better way to do what I'm doing, or if I'm misusing provider sets here, I'm all ears. If not, though, I think that it'd be really nice to relax the requirement that a provider set include a provider for all concrete types.

wyattanderson avatar May 25 '23 17:05 wyattanderson