-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Routing] Add params variable to condition expression #46042
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
Conversation
63a713f
to
50fa6ef
Compare
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.
Thanks, that's a nice feature, and the implementation is perfect. This leaves me only bike-shedding :D
So: what about injecting the parameter as variables directly in the expression? eg
#[Route('/foo/{id}', requirements: ['id' => '\d+'], condition: "id < 100")]
Of course we should then forbid to use context/request as var names, or at least give them higher priority.
Or maybe just rename |
50fa6ef
to
acefeb9
Compare
Thanks for the review, I've renamed the variable to |
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.
To the next reviewer: please help us answer this open question above :)
I prefer injecting a |
I agree with stof. |
I think it would be ok to still release this in 6.1 before the RC. It fits with #44405 and it would make sense to document/new in symfony this together. |
--- | ||
|
||
* Add `params` variable to condition expression | ||
* Deprecate not passing route parameters as the forth argument to `UrlMatcher::handleRouteRequirements()` |
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.
* Deprecate not passing route parameters as the forth argument to `UrlMatcher::handleRouteRequirements()` | |
* Deprecate not passing route parameters as the fourth argument to `UrlMatcher::handleRouteRequirements()` |
Thank you @HypeMC. |
…(HypeMC) This PR was merged into the 6.1 branch. Discussion ---------- [Routing] Fix changelog & deprecation message for #46042 | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Since #46042 was merged into 6.1 the changelog & deprecation message are now wrong. cc `@fabpot` Commits ------- bb37826 [Routing] Fix changelog & deprecation message for #46042
Currently it's not possible to use
request.attributes.get('_route_params')
in route conditions because the parameter is not yet set when the expression is being evaluated. That happens after the matcher is finished.This PR adds a new
params
variable to the condition option expression syntax: