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

Are scope requirements meant to be transitive? #140

Closed
mikehearn opened this issue Aug 2, 2021 · 3 comments
Closed

Are scope requirements meant to be transitive? #140

mikehearn opened this issue Aug 2, 2021 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@mikehearn
Copy link

I wrote the moral equivalent of this:


@Scope
@InjectModule(requires = [ A::class, B::class ])
annotation class Foo

@Scope
@InjectModule(requires = [ Foo::class, C::class ])
annotation class Bar

@Bar
class Baz(val a: A, val b: B)

I was expecting that because scope Bar depends on scope Foo, and Foo depends on A (i.e. it's externally provided) that Baz would be able to depend on A. However it seems that doesn't work. I get errors about missing dependencies. Is that intentional? If so, is the right thing to do just duplicating the requires=[] list ?

@rbygrave
Copy link
Contributor

rbygrave commented Aug 2, 2021

Baz would be able to depend on A

Yes, that should work. Baz should be able to depend on A.

However it seems that doesn't work. I get errors about missing dependencies.

Should be a bug.

We have ParentScopeTest but that doesn't test external dependencies on the parent scope which is the case here with the Foo scope above. I'll look to create a test case where the parent scope has external dependencies and get that to reproduce.

rbygrave added a commit that referenced this issue Aug 3, 2021
If a scope depends on a parent scope and that parent scope has
external dependencies - then the scope should also be able to
depend on those external dependencies.

Ultimately the fix for this is in ScopeInfo.providesDependency()
adding in the check to see if the dependency is in requires
(is an external dependency of the scope).
@rbygrave rbygrave linked a pull request Aug 3, 2021 that will close this issue
@rbygrave rbygrave self-assigned this Aug 3, 2021
@rbygrave rbygrave added the bug Something isn't working label Aug 3, 2021
@rbygrave rbygrave added this to the 6.6 milestone Aug 3, 2021
@rbygrave
Copy link
Contributor

rbygrave commented Aug 3, 2021

Ok, fixed in 6.6

@rbygrave rbygrave closed this as completed Aug 3, 2021
@mikehearn
Copy link
Author

Awesome, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants