-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Session] Add getFlashBag() to SessionInterface #12420
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
[Session] Add getFlashBag() to SessionInterface #12420
Conversation
znerol
commented
Nov 5, 2014
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | yes |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | - |
License | MIT |
Doc PR | - |
This is a BC break |
This is one more reason to have the method on the interface, because then the only way to cleanly get at it is through |
@yguedidi updated the issue summary. Still I'm wondering on the impact of this BC break. Existing third-party session implementations are forced to add a new method, is that the problem? |
@znerol Yes it is. Maybe they aren't open sources implementations, but maybe they are in closed sources projects. |
There are no concrete plans, but I doubt this will be added as it is. |
I did my share in grepping and pickaxing through the git history. Here is what I found: The large session refactoring in #2853 introduced the How likely is it that a third party |
@znerol this was back in 2.1 if I checked correctly. As of 2.3 semver is being followed and BC breaks should be avoided, this is one of those cases. |
@iltar I do not argue that a BC break is okay here because it was fine then. I just want to point out that this was broken from the beginning and that it is still in |
Duplicate of #11279 #10036 #5568. It's not broken, it's like this by design (I'm talking about getFlashBag not being on SessionInterface, not about client code using SessionInterface as if it was a Session). See @Drak's reasoning for rejecting this in the past here: #5568 (comment)
The SessionInterface is technically not tagged with an |
How is |
@znerol that's why you can inject the |
@znerol FrameworkBundle/Controller/Controller.php does not operate on SessionInterface but on the session service which is configured to be https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml#L8 |
@Tobion I mentioned other examples as well, e.g. |