Bind SessionState handler interface in container #147
Conversation
src/Auth0/Login/Auth0Service.php
Outdated
| array $auth0Config = null, | ||
| StoreInterface $sessionStorage = null, | ||
| SessionStateHandler $sessionStateHandler = null | ||
| StateHandler $stateHandler = null |
There was a problem hiding this comment.
We were going to remove the default checking here, correct? Would it be common for a developer to call this directly?
There was a problem hiding this comment.
If I understand your question correctly it's about removing the = null from the arguments?
I'd love to make these argument required in the future and remove the fallback behaviour in the constructor. Maybe this is something we can do in the 7.0.0-dev branch?
There was a problem hiding this comment.
aah, I see you changed the base branch. Should I get rid of the fallback logic in the constructor? 😄
There was a problem hiding this comment.
Yeah, I think so. As long as the default package initialization is set to always pass, I would say it's best to force these dependencies as well. I don't imagine this would be called directly that often outside of register()
There was a problem hiding this comment.
Alright, that makes sense. 👍
Binding the interface allows for an easier replacement with a different implmentation lateron.
nstapelbroek
force-pushed
the
use-statehandler-interface-in-container
branch
from
5ebbcf1 to
aad8af6
Compare
nstapelbroek
changed the title
Dropped BC constructor logic to stimulate moving all constructor and configuration logic to a centralized place e.g. a service provider.
|
Heya,
These are the functional changes extracted from #143
Changes
Instead of binding the SessionStateHandler in the container we are now binding the StateHandler interface. This allows for an easier replacement in case someone wants to.
References
#143
Testing
[x] This change has been tested on the latest version Laravel
Checklist
[x] I have read the Auth0 general contribution guidelines
[x] I have read the Auth0 Code of Conduct