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 section on dependency resolution and exposure to README #266

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matt-ramotar
Copy link
Member

Description:
I think we should add a new section to the README titled "Dependency Resolution and Exposure" to provide more clarity on how Motif handles dependencies between parent and child scopes.

Related issue(s): #177

@CLAassistant
Copy link

CLAassistant commented Jun 26, 2024

CLA assistant check
All committers have signed the CLA.

README.md Outdated
@@ -202,6 +202,37 @@ MainScope mainScope = new MainScopeImpl();
ChildScope childScope = mainScope.child();
```

## Dependency Resolution and Exposure

Motif requires explicit dependency declarations to properly resolve and inject dependencies across scopes. When a parent scope needs to provide a dependency to a child scope, the dependency must be explicitly exposed using the `@Expose` annotation.
Copy link
Contributor

@davissuber davissuber Jun 26, 2024

Choose a reason for hiding this comment

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

Doesn't line ~140 of README roughly mentions it?

I agree we perhaps need to make the use of @Expose a more pronounced section given how important it is, but perhaps we can remove / restructure the existing paragraph so readers don't think they know everything there is to know about Expose after reading that first section?

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally fine with restructuring the existing paragraph

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, thoughts? I think the explicit dependency declaration requirement is what is not completely clear

README.md Outdated
@@ -202,6 +202,37 @@ MainScope mainScope = new MainScopeImpl();
ChildScope childScope = mainScope.child();
```

## Dependency Resolution and Exposure

Motif requires explicit dependency declarations to properly resolve and inject dependencies across scopes. When a parent scope needs to provide a dependency to a child scope, the dependency must be explicitly exposed using the `@Expose` annotation.

Choose a reason for hiding this comment

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

This is valuable documentation, but can we add the other way to pass dependency to children as well?

interface ParentScope {
   fun childScope(api: Api): ChildScope // Api is exposed to children through a scope factory method parameter.

   @motif.Objects
    abstract class Objects {
        fun provideImpl(): Impl {
            return Impl()
        }

        fun provideApi(impl: Impl): Api {
            return impl
        }
    }
}

README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants