[HttpFoundation] UrlHelper is now aware of RequestContext changes #50309
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stof
reviewed
May 12, 2023
|
NOTE: this PR introduce a BC on the constructor of UrlHelper, but since it's final maybe is not required to keep backward compatility. A final constructor is not an internal one. You can still write code than instantiate the class, and would be impacted by the BC break. Constructors are not only called as |
445807a to
2624c79
Compare
|
Thank you for the feedback.
Yes, I'm very well aware, I meant that I didn't know if you care of BC of a final helper class. Since you do, I've implemented the compatibility with the deprecation and all. |
2624c79 to
59438c4
Compare
|
https://symfony.com/bc has a good overview of the Symfony BC policy, including what kind of changes you are allowed to do as a contributor. |
59438c4 to
a016392
Compare
a016392 to
75eb469
Compare
nicolas-grekas
approved these changes
May 19, 2023
|
Thank you @giosh94mhz. |
nicolas-grekas
added a commit
that referenced
this pull request
May 19, 2023
…changes (giosh94mhz) This PR was merged into the 5.4 branch. Discussion ---------- [HttpFoundation] UrlHelper is now aware of RequestContext changes | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | yes | Tickets | | License | MIT | Doc PR | `RequestContext` in routing can change at runtime after the `UrlHelper` has been created, so using a `RequestContextAwareInterface` instance (i.e. the router) will workaround URL generation issues. In my application I have a CLI command that dynamically create the RequestContext, based on database queries and other user-driven configuration. With this information at hand, I retrieve the router and set the new context that should then be used on any other URL generation. This works for the Router, but then I render some twig templates (to send formatted email) and found that the `UrlHelper` is using the old context, and not helping at all :) Also, the helper is final and without interface, so I have no way of decorating or replacing it. Long story short: in my opinion, the `RequestContext` should be treated just like the `Request`, and so we should use `RequestContextAwareInterface` just like we use the `RequestStack`. ....or maybe the context should be carried out with the `RequestStack`? Any other ideas? ~~no longer relevant: this PR introduce a BC on the constructor of `UrlHelper`, but since it's final maybe is not required to keep backward compatility. If not, I may update the class to support both `RequestContextAwareInterface` and `RequestContext` as the second parameter.~~ Commits ------- a016392 UrlHelper is now aware of RequestContext changes
|
Thank to you guys, for all your hard work! |
This was referenced May 22, 2023
Merged
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RequestContextin routing can change at runtime after theUrlHelperhas been created, so using aRequestContextAwareInterfaceinstance (i.e. the router) will workaround URL generation issues.In my application I have a CLI command that dynamically create the RequestContext, based on database queries and other user-driven configuration. With this information at hand, I retrieve the router and set the new context that should then be used on any other URL generation.
This works for the Router, but then I render some twig templates (to send formatted email) and found that the
UrlHelperis using the old context, and not helping at all :) Also, the helper is final and without interface, so I have no way of decorating or replacing it.Long story short: in my opinion, the
RequestContextshould be treated just like theRequest, and so we should useRequestContextAwareInterfacejust like we use theRequestStack.....or maybe the context should be carried out with the
RequestStack? Any other ideas?no longer relevant: this PR introduce a BC on the constructor ofUrlHelper, but since it's final maybe is not required to keep backward compatility. If not, I may update the class to support bothRequestContextAwareInterfaceandRequestContextas the second parameter.