Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Revert "DI: 'inject' option is enabled by default (experimental)"
This reverts commit d5b8b05ffbd032c11205a76b047def0a1ae295ba.
- Loading branch information
a0333bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this revert could be reverted? I think that if you put the presenters in DI Container, you should either turn on the injects on the presenter services by hand, or with some automatic mechanism. Because having the injects statically generated in the DI Container is the preferred way over dynamically resolving them on every request.
This line just makes zero sense to me outside the else branch. What do you think?
a0333bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this is a thing
a0333bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now is
inject
tag and it can be detected in runtime.a0333bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would probably work too! Good idea.
a0333bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And how this feature (checking inject tag) allows to disable auto-inject to presenter?
If I register presenter as service and I give him dependencies on my own, PresenterFactory via $this->container->callInjects($presenter) makes second redundant inject. I think that this revert should be reverted.
a0333bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidkrmela it's solved in dev version.