Allow multiple annotations per function

[sywhang]

With the new OnStart annotations being added in #895, some annotations actually might make sense for multiple annotations to take place for the same function.

For instance, the following nested annotation would allow us to specify OnStart hooks using annotations without being confusing about the order in which the hooks are executed:

fx.Annotate(
  fx.Annotate(
    func() { ... },
    fx.OnStart(...) // this hook will run first
  ),
  fx.OnStart(...) // followed by this
)

[vallahaye]

Hello @sywhang,

Sorry to dig up an old issue, for me it's very relevant and I'd like to support the idea with an additional use case.

The way I write a service in Go is by defining an interface that declares the core "business" methods, then have a structure implement it.

Most of the time, this structure also has Init and Shutdown methods to initialize/uninitialize some resources (e.g. subscribing to a pubsub mechanism). These two methods are only relevant to the implementation and are therefore not present in the interface. This is a fairly common practice and is supported out of the box by Service Weaver, for example.

To date, the best way to do this with Fx (v1.21) is as follows:

func provideService(lc fx.Lifecycle) Service {
  s := NewService() // NewService returns a pointer to the structure type, not the interface, which allows us to access the Init and Shutdown methods.
  lc.Append(fx.StartStopHook(s.Init, s.Shutdown))
  return s
}

var Module = fx.Module("service", fx.Provide(provideService))

Or:

var Module = fx.Module("service",
  fx.Provide(
    fx.Annotate(NewService, fx.OnStart((*ServiceImpl).Init), fx.OnStop((*ServiceImpl).Shutdown)),
    func(s *ServiceImpl) Service { return s },
  ),
)

If we could annotate an annotated function, I think we could rewrite the above code as follows:

var _ Service = (*ServiceImpl)(nil) // Interface guard to ensure that the call to fx.As can be satisfied

var Module = fx.Module("service",
  fx.Provide(
    fx.Annotate(
      fx.Annotate(NewService, fx.OnStart((*ServiceImpl).Init), fx.OnStop((*ServiceImpl).Shutdown)),
      fx.As(new(Service)),
    ),
  ),
)

Also note that merging the two calls to Annotate (i.e. fx.Annotate(NewService, fx.OnStart((*ServiceImpl).Init), fx.OnStop((*ServiceImpl).Shutdown), fx.As(new(Service)))) doesn't work (rightly so) and produces the following error:

$ go run .
[Fx] PROVIDE	fx.Lifecycle <= go.uber.org/fx.New.func1()
[Fx] PROVIDE	fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm()
[Fx] PROVIDE	fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm()
[Fx] PROVIDE	pkg.Service <= fx.Annotate(go.vallahaye.net/fxtest/pkg.NewService(), fx.As([[pkg.Service]]) from module "service"
[Fx] INVOKE		main.main.func1()
[Fx] ERROR		fx.Invoke(main.main.func1()) called from:
main.main
	/go/src/go.vallahaye.net/fxtest/main.go:9
runtime.main
	/usr/lib/go/src/runtime/proc.go:271
Failed: missing dependencies for function "main".main.func1
	/go/src/go.vallahaye.net/fxtest/main.go:9:
missing type:
	- *pkg.ServiceImpl (did you mean to use pkg.Service?)
[Fx] ERROR		Failed to start: missing dependencies for function "main".main.func1
	/go/src/go.vallahaye.net/fxtest/main.go:9:
missing type:
	- *pkg.ServiceImpl (did you mean to use pkg.Service?)
exit status 1

Let me know if you're still interested in solving this issue, I can take a closer look if necessary.

[vallahaye]

Hello again @sywhang,

Although, in my opinion, the use case I described above is still valid, a new primitive that solves the problem (fx.Self #1201) has been implemented in the meantime.

[sywhang]

hey @vallahaye, sorry for the delayed response - I haven't been too active on GitHub lately so apologies for the delay.

Yup, I am aware of fx.Self that was recently added. @JacobOaks who authored that PR works with me at Uber and we did look at your comment above during triaging, which inspired him to do that work :-) .

We'll be tagging a release shortly that releases that feature.

Meanwhile, allowing multiple annotations per function is still an issue we need to address and that may take a while more since the change involved is quite a bit more work.

HTH - thanks!