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

Add Fx.Replace #837

Merged
merged 4 commits into from Feb 14, 2022
Merged

Add Fx.Replace #837

merged 4 commits into from Feb 14, 2022

Conversation

sywhang
Copy link
Contributor

@sywhang sywhang commented Feb 11, 2022

This adds fx.Replace, which replaces a value already provided in the graph with another value. This is similar to how fx.Supply provides an instantiated value to the graph.

For example the following code that uses fx.Decorate to modify the *bytes.Buffer type...

fx.New(
  fx.Provide(func() *bytes.Buffer { ... }),
  fx.Decorate(func() *bytes.Buffer {
    ...
  }),
  fx.Invoke(func(b *bytes.Buffer) {
    // I get a modified buffer!
  }),
)

is the same as this version that uses fx.Replace:

b := func() *bytes.Buffer { ... }

fx.New(
  fx.Provide(func() *bytes.Buffer { ... }),
  fx.Replace(b),
  fx.Invoke(func(b *bytes.Buffer) {
    // I get a modified buffer!
  }),
)

Implementation-wise, it is very similar to how fx.Supply creates a function that produces the given value using reflection and dig.Provides it - fx.Replace creates a function that produces the given value with no argument using reflection, and dig.Decorate with that function.

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #837 (923c0e8) into master (9173ccd) will decrease coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #837      +/-   ##
==========================================
- Coverage   98.56%   98.42%   -0.15%     
==========================================
  Files          29       30       +1     
  Lines        1043     1076      +33     
==========================================
+ Hits         1028     1059      +31     
- Misses         10       11       +1     
- Partials        5        6       +1     
Impacted Files Coverage Δ
replace.go 100.00% <100.00%> (ø)
app.go 94.95% <0.00%> (-0.92%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9173ccd...923c0e8. Read the comment docs.

@sywhang sywhang mentioned this pull request Feb 11, 2022
Copy link
Collaborator

@abhinav abhinav left a comment

Choose a reason for hiding this comment

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

Minor issue in the test case.


Separately from this PR, we might need to tweak the value group handling a bit more.
Question: if I do this, what's the value in group:"t" at the top-level/what should it be?

foo := fx.Module("foo",
  fx.Supply(
    fx.Annotate([]string{"a", "b", "c"}, `group:"t,flatten"`),
  ),
)

fx.New(
  fx.Module("wrapfoo", 
    foo,
    fx.Replace(
      fx.Annotate([]string{"d", "e", "f"}, `group:"t"`),
    ),
  ),
)

replace_test.go Outdated Show resolved Hide resolved
replace.go Outdated Show resolved Hide resolved
replace_test.go Outdated Show resolved Hide resolved
@sywhang
Copy link
Contributor Author

sywhang commented Feb 14, 2022

@abg thanks for catching the issue with the test. Fixed that.

Question: if I do this, what's the value in group:"t" at the top-level/what should it be?

I believe that should be []string{"d", "e", "f"}? Unless that wasn't what you were thinking. Decoration at top-level should override anything provided by child modules.

@sywhang sywhang merged commit b2c4636 into uber-go:master Feb 14, 2022
@sywhang sywhang deleted the fx-replace branch February 14, 2022 19:29
josephinedotlee pushed a commit that referenced this pull request Feb 18, 2022
This adds `fx.Replace`, which replaces a value already provided in the graph with another value. This is similar to how `fx.Supply` provides an instantiated value to the graph.

For example the following code that uses fx.Decorate to modify the `*bytes.Buffer` type...
```go
fx.New(
  fx.Provide(func() *bytes.Buffer { ... }),
  fx.Decorate(func() *bytes.Buffer {
    ...
  }),
  fx.Invoke(func(b *bytes.Buffer) {
    // I get a modified buffer!
  }),
)
```

is the same as this version that uses fx.Replace:
```go
b := func() *bytes.Buffer { ... }

fx.New(
  fx.Provide(func() *bytes.Buffer { ... }),
  fx.Replace(b),
  fx.Invoke(func(b *bytes.Buffer) {
    // I get a modified buffer!
  }),
)
```

Implementation-wise, it is very similar to how fx.Supply creates a function that produces the given value using reflection and `dig.Provide`s it - fx.Replace creates a function that produces the given value with no argument using reflection, and `dig.Decorate` with that function.
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

3 participants