FOSJsRoutingBundle
FOSJsRoutingBundle copied to clipboard
Parameters without defaults are not required to generate URL in some cases
Route definition
test_route:
path: /test/{required1}/{required2}/{optional}
defaults:
_controller: '\Some\Class::method'
optional: 0
requirements:
optional: \d+
required1: \d+
required2: \d+
methods:
- GET
options:
expose: true
translated into this
fos.Router.setData({
"base_url": "",
"routes": {
"test_route": {
"tokens": [
["variable", "\/", "\\d+", "optional"],
["variable", "\/", "\\d+", "required2"],
["variable", "\/", "\\d+", "required1"],
["text", "\/test"]
],
"defaults": {
"optional": 0
},
"requirements": {
"optional": "\\d+",
"required1": "\\d+",
"required2": "\\d+"
},
"hosttokens": [],
"methods": ["GET"],
"schemes": []
}
},
"prefix": "",
"host": "localhost",
"port": "",
"scheme": "http",
"locale": "ru"
});
Cases:
Execute: Routing.generate('test_route')
Expected result: "Uncaught Error: The route "test_route" requires the parameter "required1" (or required2)
Actual result: "/test"
Execute: Routing.generate('test_route', {required1: 1})
Expected result: "Uncaught Error: The route "test_route" requires the parameter "required2"
Actual result: "/test/1"
Works as expected when setting an optional parameter:
Execute: Routing.generate('test_route', {optional: 1})
Expected result: "Uncaught Error: The route "test_route" requires the parameter "required1" (or required2)
Actual result: "Uncaught Error: The route "test_route" requires the parameter "required2"
Setting optional = false by default here https://github.com/FriendsOfSymfony/FOSJsRoutingBundle/blob/master/Resources/public/js/router.js#L293 fixes my issue, although I haven't tested other cases yet and not quite sure what is the purpose of this variable at all.
optional = true was added here https://github.com/FriendsOfSymfony/FOSJsRoutingBundle/pull/81#discussion-diff-3648557
Hi @RealJTG, thanks for pointing out this issue. It was actually introduced in https://github.com/friendsofsymfony/FOSJsRoutingBundle/commit/8298523387158aa11fbfae2893d6b17866321111#diff-8e1e018bfe2bd15df10d1864a922ff0cR64, the diff you indicated was just a rewrite of the same code. It has to do with the definition of optional parameters in Symfony, of which more can be read here: https://symfony.com/doc/current/routing.html#optional-parameters. The current implementation in this bundle does not comply with the requirements written there. I think it is not doable to make it comply with that, since defaults for parameters can be implemented by giving a default value in the PHP method definition when using annotations. These default values are not available to the tools generating the routing export, so instead of that it is implemented by looking for the first defined variable routing part (or text part), after which each variable is required to be defined since otherwise the route will be invalid. Any routes not sticking to this assumption will be invalid for that place, which will lead to interesting behavior.
Do you have any suggestions on how to implement this in a better way? We cannot simply assume every parameter to be defined or have a default in the export, since that is not required by Symfony.