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

Support fx.Private w/ fx.Supply #1207

Merged
merged 2 commits into from
May 30, 2024

Conversation

JacobOaks
Copy link
Contributor

fx.Supply is essentially an API that allows for conveniently fx.Provideing an exact value, rather than a function that will return that value. For example, fx.Provide(func() int { return 5 }) is equivalent to fx.Supply(5).

fx.Private allows for usage of a provided constructor's results to be restricted to the current module and its child modules.

fx.Module(
	"parent",
	fx.Invoke(func(int) { /* this will error out! */ }),
	fx.Module(
		"child",
		fx.Provide(func() int { return 5 }, fx.Private),
	),
),

This PR allows for using fx.Private with fx.Supply as well, so that folks can enjoy the convenience of fx.Supply when they also wish to restrict the usage of the supplied value.

fx.Module(
	"parent"
	fx.Invoke(func(int) { /* this will error out! */ }),
	fx.Module(
		"child",
		fx.Supply(5, fx.Private),
	),
),

Ref #1206

Since the behavior between Supply + Private and Provide + Private should be identical, I opted to generalize the existing fx.Private tests to run for both Provide and Supply. This keeps the tests a little more DRY but does complicate them/hurt readability. I feel like this is OK since there are a lot of tests, but I also am the one who wrote the tests, so I am biased regarding its readability. Thus, I am happy to break out Supply + Private into its own tests if folks feel strongly that these tests are hard to read.

`fx.Supply` is essentially an API that allows for conveniently
`fx.Provide`ing an exact value, rather than a function that will return that value.
For example, `fx.Provide(func() int { return 5 })` is equivalent to `fx.Supply(5)`.

`fx.Private` allows for usage of a provided constructor's results
to be restricted to the current module and its child modules.
```go
fx.Module(
	"parent",
	fx.Invoke(func(int) { /* this will error out! */ }),
	fx.Module(
		"child",
		fx.Provide(func() int { return 5 }, fx.Private),
	),
),
```

This PR allows for using `fx.Private` with `fx.Supply` as well,
so that folks can enjoy the convenience of `fx.Supply` when they also wish to
restrict the usage of the supplied value.
```go
fx.Module(
	"parent"
	fx.Invoke(func(int) { /* this will error out! */ }),
	fx.Module(
		"child",
		fx.Supply(5, fx.Private),
	),
),
```

Ref uber-go#1206

Since the behavior between Supply + Private and Provide + Private
should be identical, I opted to generalize the existing `fx.Private` tests
to run for both Provide and Supply. This keeps the tests a little more DRY
but does complicate them/hurt readability. I feel like this is OK since
there are a lot of tests, but I also am the one who wrote the tests,
so I am biased regarding its readability.
I am happy to break out Supply + Private into its own tests
if folks think these tests are getting cluttered.
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.41%. Comparing base (9e6f6c2) to head (258fe12).

Current head 258fe12 differs from pull request most recent head 8c372ea

Please upload reports for the commit 8c372ea to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1207      +/-   ##
==========================================
- Coverage   98.51%   98.41%   -0.10%     
==========================================
  Files          34       34              
  Lines        2900     2908       +8     
==========================================
+ Hits         2857     2862       +5     
- Misses         36       38       +2     
- Partials        7        8       +1     

☔ 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 29, 2024 17:38
JacobOaks added a commit to JacobOaks/fx that referenced this pull request May 29, 2024
This updates the changelog and the version for a 1.22.0 release.

I will wait to merge this until uber-go#1207 is reviewed.

Ref: uber-go#1204
app_test.go Outdated Show resolved Hide resolved
@JacobOaks JacobOaks merged commit b3b1c3b into uber-go:master May 30, 2024
10 checks passed
@JacobOaks JacobOaks mentioned this pull request May 30, 2024
JacobOaks added a commit that referenced this pull request May 30, 2024
This updates the changelog and the version for a 1.22.0 release.

I will wait to merge this until #1207 is reviewed.

I tested Fx tip within Uber's Go codebase and saw no issues.

Ref: #1204
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.

2 participants