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

fx.Self: a parameter to fx.As for providing a type as itself #1201

Merged
merged 5 commits into from
May 16, 2024

Conversation

JacobOaks
Copy link
Contributor

@JacobOaks JacobOaks commented May 14, 2024

We frequently see requests for folks who want to use fx.As to provide a type as another type, while also providing it as itself. To name a few:

This is currently not possible via strictly using fx.As + a single constructor, since fx.As causes a constructor to no longer provide its original type. The workaround we often give is for folks to do something like this, which involves adding a second "constructor":

fx.Provide(
    newConcreteType,
    func(ct *concreteType) Interface {
        return ct
    }
)

which is admittedly not very ergonomic. A somewhat common pattern mistaken to be a workaround is providing the constructor twice instead:

fx.Provide(
    newConcreteType,
    fx.Annotate(newConcreteType, fx.As(new(Interface))),
)

This PR adds fx.Self(), which returns a special value to indicate an fx.As should retain that original return type:

fx.Provide(
    newConcreteType,
    fx.As(fx.Self()),
    fx.As(new(Interface)),
)

As an alternative, I considered a new annotation altogether, named something like fx.AlsoAs, but adding a special type that can be passed as an argument to fx.As directly allows for more fine-tuned control over individual positional return values.

For example, this function's return types can be easily expressed as *asStringer and io.Writer using fx.Self():

fx.Provide(
	fx.Annotate(
		func() (*asStringer, *bytes.Buffer) {/* ... */ },
		fx.As(fx.Self(), new(io.Writer)), // return values will be: *asStringer, io.Writer
	),
),

Whereas something like fx.AlsoAs wouldn't provide the ability to skip over the first positional return value entirely.

JacobOaks and others added 2 commits May 14, 2024 17:47
We frequently see requests for folks who want to use `fx.As`
to provide a type as another type, while also providing it as itself:
* uber-go#1196
* uber-go#1148
* uber-go#1079

This is currently not possible because `fx.As` causes a constructor
to no longer provide its original type.

The workaround we often give is for folks to do something like:
```go
fx.Provide(
    newConcreteType,
    func(ct *concreteType) Interface {
        return ct
    }
)
```
which is admitedly not very ergonomic, and leads to a common mistake
of providing the cosntructor twice instead:
```go
fx.Provide(
    newConcreteType,
    fx.Annotate(newConcreteType, fx.As(new(Interface))),
)
```

This PR adds `fx.Self()`, which returns a special value to indicate
an `fx.As` should retain that original return type:
```go
fx.Provide(
    newConcreteType,
    fx.As(fx.Self()),
    fx.As(new(Interface)),
)
```

This can be used as positional arguments to `fx.As` as well.
Copy link

codecov bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.46%. Comparing base (cb9cccf) to head (afe49a5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1201      +/-   ##
==========================================
- Coverage   98.49%   98.46%   -0.03%     
==========================================
  Files          34       34              
  Lines        2860     2676     -184     
==========================================
- Hits         2817     2635     -182     
+ Misses         36       34       -2     
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JacobOaks JacobOaks marked this pull request as ready for review May 14, 2024 18:16
@JacobOaks JacobOaks requested review from tchung1118 and sywhang and removed request for tchung1118 May 14, 2024 20:22
Copy link
Contributor

@tchung1118 tchung1118 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look great and pretty isolated. Added a couple of comments.

annotated.go Outdated Show resolved Hide resolved
annotated_test.go Show resolved Hide resolved
@JacobOaks
Copy link
Contributor Author

Changes look great and pretty isolated. Added a couple of comments.

Addressed!

Copy link
Contributor

@tchung1118 tchung1118 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JacobOaks JacobOaks merged commit 7dd18c6 into uber-go:master May 16, 2024
12 checks passed
@JacobOaks JacobOaks deleted the joaks/self branch May 16, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants