FEATURE: Add queryParameters option to Neos.Fusion:ActionUri - #5882
FEATURE: Add queryParameters option to Neos.Fusion:ActionUri#5882bwaidelich wants to merge 3 commits into
queryParameters option to Neos.Fusion:ActionUri#5882Conversation
Currently the only way to append query parameters to a generated URL is to use routing arguments with `appendExceedingArguments`, which is a bad idea since it creates a route part / cache entry for every combination. This adds an explicit `queryParameters` option that is appended to the built URI as a plain query string instead. Resolves #5881
mhsdesign
left a comment
There was a problem hiding this comment.
Youre right that this change is long due. As confirmation of that is that @mficzel has prepared a PR already in 2022 that is to this day unfortunately left unmerged because of missing adjustments and we are waiting on official guidance of the Flow Framework (aka us) to deprecate appendExceedingArguments and that and how queryParameters should be used instead.
A change like this obviously invokes demons from beyond as the common-folk was allowed to put all kind of things - even living objects - into the dirty bag of routing arguments. Only via dark magic potions under full-moon were these things then put into a string for the query.
That magic would be no more for queryParameters, we would just expect simple primitives and probably correctly reject any entities. Though we currently would also fail to serialise simples value objects like a NodeAggregateId which does not seem to be correct and modern practice - so that doesnt fit together?.
I love to have something solid in this direction but it should be solid and thought out from the start. Wdyt?
| } | ||
| $queryParameters = $this->getQueryParameters(); | ||
| if ($queryParameters !== []) { | ||
| $uri .= '?' . http_build_query($queryParameters, arg_separator: '&'); |
There was a problem hiding this comment.
this is a bit too simple and kinda illegal.
The routing framework allows to build uris with query parameters already and concatenating this together leads to funny urls.
I have already thought ahead and there is a utility that can be used Neos\Flow\Http\Helper\UriHelper::uriWithAdditionalQueryParameters
There was a problem hiding this comment.
Right, thanks!
I could not use the Helper because it relies on UriInterface and for historic reasons UriBuilder::build() returns a string.
In practice, using queryParameters and appendExceedingArguments is not viable (well, using appendExceedingArguments alone is not viable and should be deprecated and burned – it was probably my worst contribution to Flow..).
But I realized, that the Router allows the segment (aka fragment or hash) to be specified for some weird reason – what's that to do with routing? Anyways, I added support for that too but it turns this into a bit of a hack.. Not sure about it anymore :)
...and fix support for sections (aka fragments)
|
@mhsdesign Thanks for your comment. I agree that it needs to be solid, but IMO we don't have to do any fancy magic like silently converting your input.. Anyways, with the fact that the Router can return URLs with query strings and sections, this change got less clean than I had hoped.. |
Currently the only way to append query parameters to a generated URL is to use routing arguments with
appendExceedingArguments, which is a bad idea since it creates a route part / cache entry for every combination.This adds an explicit
queryParametersoption that is appended to the built URI as a plain query string instead.Usage
Resolves #5881