uber-go/fx

object is not recreated when its dependency is decorated in different module

yiminc opened this issue ยท 12 comments

Not sure if this is a bug or by design but I think a lot of users will be surprised by the result.
Below code the namedService is only created once even thought its dependency is decorated in each module.
Depending on the order of FooServiceModule / BarServiceModule, the result is different.

If this is by design, we need better documentation.

type namedService struct {
	name string
}

func NewService(name string) *namedService {
	return &namedService{name: name}
}

func (s *namedService) Start() error {
	fmt.Printf("start service %v\n", s.name)
	return nil
}

var FooServiceModule = fx.Module("foo",
	fx.Decorate(func() string {return "foo"}),
	fx.Invoke(serviceLifecycleHooks),
)

var BarServiceModule = fx.Module("bar",
	fx.Decorate(func() string {return "bar"}),
	fx.Invoke(serviceLifecycleHooks),
)

func serviceLifecycleHooks(
	lc fx.Lifecycle,
	service *namedService,
) {
	lc.Append(fx.Hook{
		OnStart: func(context.Context) error {
			return service.Start()
		},
	})
}

var MainModule = fx.Module("server",
	fx.Provide(NewService),
	fx.Provide(func() string {return "main"}),
	FooServiceModule,
	BarServiceModule,
)

func main() {
	app := fx.New(
		MainModule,
		fx.NopLogger,
	)
	app.Start(context.Background())
}

Yes, in this case, it is by design. Any constructor provided to Fx will be invoked once only, so it can't be "recreated" with a different dependency.

I can see how it is prone to potential errors. After fx.Module, fx.Replace/fx.Decorate ships I will post a discussion/issue with recommended pattern and things to avoid when using these features.

Hey @yiminc, thanks for trying it out further, and thanks for the thorough bug report! We really appreciate it.

@sywhang and I discussed this in some detail and we've come to a good conclusion around the model of decorates. Specifically: decorates aren't transitive, and the invariant of "one invocation per function" will continue to hold.

So when you decorate a type inside a module, you're only replacing the definition of that type for that module. You're not changing types in other modules.

For your example, NewService is provided by the main module. There's only one service with the name "main". The submodules replace the definition of "string" for themselves, but they still depend on the same Service provided by the main module.

graph LR
    subgraph Main
        Service --> string["'main'"]
    end
    subgraph Foo
        foo["'foo'"]
        hooksFoo[hooks] --> Service
    end

    subgraph Bar
        bar["'bar'"]
        hooksBar[hooks] --> Service
    end

To get Foo and Bar services, the submodules have to provide their own service instances that will consume the foo or bar names.

With the changes @sywhang is making in uber-go/dig#325, you can achieve this with something like the following:

fx.Module("foo",
    fx.Decorate(func() string { return "foo" }),
    fx.Provide(fx.Annotate(
        NewService,
        fx.ResultTags(`name:"foo"`),
    )),
    fx.Invoke(fx.Annotate(hooks, fx.ParamTags(`name:"foo"`)))
)

This will add a separate Service (named) service node in the foo module that consumes the string within the package.

graph LR
    subgraph Main
        Service --> string["'main'"]
    end

    subgraph Bar
        hooksBar[hooks] --> ServiceBar["Service<br/>name='bar'"] --> bar["'bar'"]
    end

    subgraph Foo
        hooksFoo[hooks] --> ServiceFoo["Service<br/>name='foo'"] --> foo["'foo'"]
    end

Just to clarify, in the example I gave, the namedService instance created has name "foo" not "main" because foo's invoke is called first.

rhzs commented

Hey @yiminc, thanks for trying it out further, and thanks for the thorough bug report! We really appreciate it.

@sywhang and I discussed this in some detail and we've come to a good conclusion around the model of decorates. Specifically: decorates aren't transitive, and the invariant of "one invocation per function" will continue to hold.

So when you decorate a type inside a module, you're only replacing the definition of that type for that module. You're not changing types in other modules.

For your example, NewService is provided by the main module. There's only one service with the name "main". The submodules replace the definition of "string" for themselves, but they still depend on the same Service provided by the main module.

To get Foo and Bar services, the submodules have to provide their own service instances that will consume the foo or bar names.

With the changes @sywhang is making in uber-go/dig#325, you can achieve this with something like the following:

fx.Module("foo",
    fx.Decorate(func() string { return "foo" }),
    fx.Provide(fx.Annotate(
        NewService,
        fx.ResultTags(`name:"foo"`),
    )),
    fx.Invoke(fx.Annotate(hooks, fx.ParamTags(`name:"foo"`)))
)

This will add a separate Service (named) service node in the foo module that consumes the string within the package.

Hi, I am curious..

How do you make visualize graph? Can I make it via code?

rhzs commented

@rhzs https://github.blog/2022-02-14-include-diagrams-markdown-files-mermaid/ ๐Ÿ™‚

Sorry I misunderstand you, I thought you do it in automated way from Dig/FX codes. Like this https://ofabry.github.io/go-callvis/ or this one https://github.com/kisielk/godepgraph

@rhzs https://github.blog/2022-02-14-include-diagrams-markdown-files-mermaid/ ๐Ÿ™‚

Sorry I misunderstand you, I thought you do it in automated way from Dig/FX codes. Like this https://ofabry.github.io/go-callvis/ or this one https://github.com/kisielk/godepgraph

Oh I see. Yeah, I believe @abhinav hand wrote this using Mermaid.

@abhinav / @sywhang , to be honest, the result is very suppressing, especially that now the result depends on the order of which module (foo or bar) is added first.

@abhinav / @sywhang , to be honest, the result is very suppressing, especially that now the result depends on the order of which module (foo or bar) is added first.

@yiminc Can you clarify? With the change I mentioned earlier, it's order independent: if a module decorates a thing, its constructors use the decorated thing. Otherwise, they use whatever the parent module did. Sibling modules don't affect anything.

Yes, the workaround that you mentioned would work, which is to decorate every thing explicitly including all their upstream dependencies and tag them explicitly. That is a pretty big burden for user to take in order to use decorate for this purpose.

Otherwise, if user just does something similar to my sample code, the result depends on the order of which module is added first.

Just to poke on this again. The example that I gave in the issue would output below lines:

start service foo
start service foo

If I added fx.Decorate(NewService), to FooServiceModule, the output become:

start service main
start service bar

Adding decorate in Foo module would result in the object been created with its dependency (the string) from its parent scope. And also the object get "re-created" in Bar module.

This is very confusing.
@sywhang @abhinav

looks like latest release fixed the issue, thanks.