refactor(depinject)!: require exported functions & types #12797
Conversation
…ject-codegen-require-export
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aaronc
changed the title
…-export' into aaronc/depinject-codegen-require-export
| } | ||
|
|
||
| loc := LocationFromPC(val.Pointer()) | ||
| loc := LocationFromPC(val.Pointer()).(*location) |
There was a problem hiding this comment.
Exported Providers and Invokers are a requirement for codegen, did you consider allowing them for them for runtime DI? Maybe this would be too confusing for users of this lib though.
There was a problem hiding this comment.
I think you mean allowing unexported providers at runtime. I think it would be too confusing to have different behavior for runtime and compile time. Also, we want to ensure that all code can be executed at compile time if desired so doing this check at runtime ensures that.
|
Overall the direction makes sense given the constraints of codegen. It seems too complex to maintain side by side compatibility with the current runtime API, right? Also I guess we can’t merge this without some refactoring (you mentioned this in the PR description). Is the plan to branch that work off of this PR? |
That's my thinking
No, this PR can be merged as is. The other code references a tag of depinject so we can do it in a totally separate PR either with a new tag or a replace directive |
…ject-codegen-require-export
…cosmos/cosmos-sdk into aaronc/depinject-codegen-require-export
| // option.Provide(ProviderDescriptor{ ... }) | ||
| type ProviderDescriptor struct { | ||
| // option.Provide(providerDescriptor{ ... }) | ||
| type providerDescriptor struct { |
There was a problem hiding this comment.
Yes there was a replace directive but when I remove it go can't resolve the tagged module
…ject-codegen-require-export
|
Don't we need to go work sync? |
|
Maybe the |
|
I believe we always use the latest version with |
|
I'm checking to see if removing go.work fixes the issues here |
…ject-codegen-require-export
Codecov Report
@@ Coverage Diff @@
## main #12797 +/- ##
==========================================
+ Coverage 53.17% 54.10% +0.92%
==========================================
Files 641 662 +21
Lines 54871 56426 +1555
==========================================
+ Hits 29180 30528 +1348
- Misses 23365 23515 +150
- Partials 2326 2383 +57
|
…-export' into aaronc/depinject-codegen-require-export
Close cosmos#17743 Since cosmos#12797, it's no longer possible to use non-exported functions as providers. The README used to contain examples with function literals as provider, so this change replaces them with exported functions. An additional change updates the `BindInterface` arguments, which wasn't working on my side when running this code in go module called "duck".
Description
Ref #12556
This PR:
ProviderDescriptorunexported so that custom providers (which aren't declared a compile time) cannot be builtIt does not attempt to handle generic parameters because that involves some complex parsing. It can be added in a future PR if desired.
In a follow-up PR, we will need to make all existing module providers exported and change the core/appconfig code.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change