-
Notifications
You must be signed in to change notification settings - Fork 43
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
ScopeFactory Proposal #108
Comments
👍 |
Regarding to the above code block, does this means that we have to explicitly list all the dependencies in the Scope when it is defined? This could be a burden that requires us to make effort in figuring out. One of the benefits that I quite enjoy using Motif is that I do NOT have to worry about the dependencies from parent scopes. All I have to focus on is to list out the current dependencies that is used to build the current Scope. Super neat. And the deeper the scope becomes, the better it could be. |
@TonyTangAndroid I added a bit more clarification in the description. This API change would only apply to root Scopes. The child method API would still exist and any Scopes who are created by some parent Scope do not need to define their dependencies explicitly. |
I see. That is very good to know. Does this mean that the java refection part is also only applied to Root scopes? Let me know when there is a snapshot version that I could try it out for this feature. |
Yes that's right, reflection is only required to instantiate a Motif root scope from outside of Motif. I do want to reiterate here though that it will still be completely compile-time safe since we can verify correctness at annotation processing time. Currently working on implementing this - Will put up a PR to check out once this is ready. |
I see. Cool. It makes sense. Looking forward to the snapshot version. |
PR is ready #113 |
This seems like a positive direction to go. We're less than a 1.0, so a breaking API change at this early stage of the project is probably ok. That being said.
|
The breaking API change would be the removal of the
Reflection is minimal and the performance cost is negligible since it only happens once on the instantiation of a root scope.
This depends on the situation. Since the main benefit of this API would be avoiding referencing generated code, I think we can simply say that annotation-processing rounds will either stay the same or be reduced but will never increase with this change. |
Before I forget, I am wondering whether there is test case to reproduce google/auto#660 and cover google/auto#660 for Scope Factory? |
Great idea - I'll add that in. Update: Added a test to ensure that a Scope can extend a user-defined dependencies interface. That said, the new API avoids referencing generated code, so we can't test that issue directly since it's not longer applicable. |
Closing this out in favor of #125 |
[Obsolete] See #125
This proposal describes an API to instantiate a Motif Scope that doesn't have a parent Scope. The child method api will still exist - this only applies to the instantiation of root Scopes.
Existing Pattern
Limitations
The pattern above requires you to reference generated code (
FooScopeImpl
andFooScopeImpl.Dependencies
). There are a few problems with this:Proposed API
Benefits
You no longer need to reference generated code which unlocks the following benefits:
Trade-offs
@Dependencies
feature.Implementation
FooScopeFactoryHelper
class which allows us to instantiateFooScope
givenFooDependencies
.ScopeFactory.create
method calls the generatedFooScopeFactoryHelper.create
method reflectively.Generated Code
New Framework Classes
Rollout
ScopeFactory
API as a beta featureScopeFactory
API proves to work better than the@Dependencies
API, we will move ScopeFactory out of beta and remove the@Dependencies
API.The text was updated successfully, but these errors were encountered: