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

Allow providing types that depend on decorated types from child modules #325

Merged
merged 5 commits into from Feb 25, 2022

Conversation

sywhang
Copy link
Contributor

@sywhang sywhang commented Feb 24, 2022

This fixes the case where a function that's Invoked depends on a type that's decorated.

Specifically, if a type has a decorator in a child Scope, and there's another exported constructor consuming that type within the same Scope, and a function is invoked outside of that Scope, we still should see the decorated type.

Implementation-wise, this means that the constructors should know which Scope it was originally provided to, and invoke it from that Scope if and only if that Scope has a more "specific" Scope than the Scope it was invoked from. To make this comparison of "specific-ness" of the Scopes cheap, a new field was added to Scope to keep track of its distance from the root Container. A Scope with higher degree is the more "specific" Scope than one with a lower degree.

@codecov
Copy link

codecov bot commented Feb 24, 2022

Codecov Report

Merging #325 (cd8c5d0) into master (3a148a2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #325   +/-   ##
=======================================
  Coverage   98.16%   98.17%           
=======================================
  Files          21       21           
  Lines        1419     1423    +4     
=======================================
+ Hits         1393     1397    +4     
  Misses         16       16           
  Partials       10       10           
Impacted Files Coverage Δ
constructor.go 96.96% <100.00%> (+0.09%) ⬆️
param.go 96.47% <100.00%> (ø)
provide.go 100.00% <100.00%> (ø)

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 3a148a2...cd8c5d0. Read the comment docs.

@sywhang sywhang changed the title [WIP] Allow providing types that depend on decorated types from child modules Allow providing types that depend on decorated types from child modules Feb 25, 2022
param.go Outdated Show resolved Hide resolved
@sywhang sywhang merged commit 677f64b into uber-go:master Feb 25, 2022
@sywhang sywhang deleted the transitive-decoration-provide branch February 25, 2022 22:26
sywhang added a commit to uber-go/fx that referenced this pull request Feb 25, 2022
This modifies a test that was added in a previous commit which highlights
a problematic transitive decoration of a parent-module provided-type.

Specifically, consider the following case:

Parent Module provides type A, and type B which depends on type A.
Child Module decorates type A and invokes something that depends on type B.
In such case, the invoked function in child Module should not see a version of
type B created with a decorated type A.

This is because the constructor for the type B lives outside the scope of the
decoration. The decoration can only affect child Scope's providers, and should
not affect parent-provided constructor directly.

This test case is not compliant with such case, so it should be modified.

This also pins Dig dependency to latest commit of Dig's master branch to take
in the fix made in uber-go/dig#325.
luoboton added a commit to luoboton/fx that referenced this pull request Aug 24, 2022
This modifies a test that was added in a previous commit which highlights
a problematic transitive decoration of a parent-module provided-type.

Specifically, consider the following case:

Parent Module provides type A, and type B which depends on type A.
Child Module decorates type A and invokes something that depends on type B.
In such case, the invoked function in child Module should not see a version of
type B created with a decorated type A.

This is because the constructor for the type B lives outside the scope of the
decoration. The decoration can only affect child Scope's providers, and should
not affect parent-provided constructor directly.

This test case is not compliant with such case, so it should be modified.

This also pins Dig dependency to latest commit of Dig's master branch to take
in the fix made in uber-go/dig#325.
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