Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorate does not work if inside fx.Module and Populate is called from outside Module #1222

Open
CTrando opened this issue Jun 28, 2024 · 6 comments

Comments

@CTrando
Copy link

CTrando commented Jun 28, 2024

Describe the bug

First, we can:

  • create an fx.Module
  • provide a type A inside the module
  • decorate the type A inside the module
  • outside the fx.Module, populate type A

The result is that the decorate function would not have been applied.

To Reproduce

I have a testcase to reproduce this:

func TestReproPopulate(t *testing.T) {
	t.Parallel()

	var something string
	app := fxtest.New(
		t,
		fx.Module("something",
			fx.Provide(func() string {
				return "something"
			}),
			fx.Decorate(func(h string) string {
				return "else"
			}),
		),
		fx.Populate(&something),
	)
	app.RequireStart()
	app.RequireStop()

	require.Equal(t, "else", something)
}

I expect that the decorate statement would have run to replace the string to else, but it stays as something. As a result, this testcase fails. I'm on the latest version of fx (v1.22.1).

However, this passes:

func TestReproPopulate(t *testing.T) {
	t.Parallel()

	var something string
	app := fxtest.New(
		t,
		fx.Module("something",
			fx.Provide(func() string {
				return "something"
			}),
		),
		fx.Decorate(func(h string) string {
			return "else"
		}),
		fx.Populate(&something),
	)
	app.RequireStart()
	app.RequireStop()

	require.Equal(t, "else", something)
}

Expected behavior

I expect that the decorate inside the module still runs, and we populate the type after the decorate executes. The testcase above should pass.

Additional context

I believe this is because fx.Populate runs as an fx.Invoke, which runs at the top level scope and doesn't look at decorators in the child scope.

@r-hang
Copy link
Contributor

r-hang commented Jul 16, 2024

Hey @CTrando, fx.Decorate are scoped to the deepest Fx module inside which the decorator was provided. In case 2, that's the root scope which is why the decoration works.

@CTrando
Copy link
Author

CTrando commented Jul 17, 2024

Hey @CTrando, fx.Decorate are scoped to the deepest Fx module inside which the decorator was provided. In case 2, that's the root scope which is why the decoration works.

In the first case, shouldn't the decorator apply to the provide statement because they're in the same module?

My expectation would be that a module provides a given type, and any decorators inside that module would run as normal if necessary to provide that type, is my understanding incorrect?

@r-hang
Copy link
Contributor

r-hang commented Jul 17, 2024

In the example above, fx.Populate is in the root scope not the scope of the Module where fx.Decorate is called. Moving fx.Populate into the Module where fx.Decorate is called should pass the test.

@CTrando
Copy link
Author

CTrando commented Jul 17, 2024

In the example above, fx.Populate is in the root scope not the scope of the Module where fx.Decorate is called. Moving fx.Populate into the Module where fx.Decorate is called should pass the test.

You're right, the test does pass! So if I had a module that exposed a type, where that type was provided and decorated inside the module, how would I go about viewing that type from outside the module?

Even if I did something like:

app := fxtest.New(
    t,
    fx.Module("something",
        // expose a string type with the annotation myString
        fx.Provide(
            fx.Annotate(func() string {
                return "something"
            }, fx.ResultTags(`name:"myString"`)),
        ),
        // use a decorator to adjust the string
        fx.Decorate(
            fx.Annotate(func(h string) string {
                return "else"
            }, fx.ParamTags(`name:"myString"`)),
        ),
    ),

    // try to pull out the type exposed in the module
    fx.Provide(
        fx.Annotate(func(x string) string {
            // check if the decorator ran - but it fails
            require.Equal(t, "else", x)
            return ""
        }, fx.ParamTags(`name:"myString"`)),
    ),
    fx.Populate(&something),
)

This fails too - the decorator changes don't appear outside the module. From what I'm seeing, it means that decorators on types are local to a given module, and their changes don't appear to any caller outside the module that references those types. Is that intentional?

@JacobOaks
Copy link
Contributor

JacobOaks commented Jul 23, 2024

Hey @CTrando - yes, this is intentional and expected behavior. Take a look at https://pkg.go.dev/go.uber.org/fx#hdr-Decorator_scope. I think the idea is that a decorator in some small nested subscope shouldn't be able to just completely hijack some value for the entire application.

That said, is there something preventing you from using your decorator at a higher-level scoping such that it applies everywhere you want it to?

@CTrando
Copy link
Author

CTrando commented Jul 23, 2024

Ah, I see, in that documentation it doesn't matter where the logger is provided. My mistake then, sorry about that.

That said, is there something preventing you from using your decorator at a higher-level scoping such that it applies everywhere you want it to?

There is nothing preventing me from using decorators at a higher scope, and that's what I ended up doing, I was just curious about this. I think I was just confused at why decorators seemed special here, I viewed them as a piece of the module to be executed. For example:

I think the idea is that a decorator in some small nested subscope shouldn't be able to just completely hijack some value for the entire application.

This makes sense to me, if there's a decorator in a module that affects a type defined outside the module. But for decorators on types inside the module, I find it reasonable that decorators would still run, and that the type is not "complete" until those decorators have run.

Perhaps my understanding of modules is incorrect - from these docs,

An Fx module is a shareable Go library or package that provides self-contained functionality to an Fx application.

I was considering the decorator to be part of that self-contained functionality, it's still a bit surprising to me that its side effects are not considered outside the module.

I agree that this seems intentional, although I wonder if this aligns with the idea of an fx.Module. You can close this issue, but I'd like to hear your thoughts on this. Thank you for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants