Skip to content
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

Move unused arguments to query parameters when generating a URL #247

Open
vjik opened this issue Jun 10, 2024 · 1 comment
Open

Move unused arguments to query parameters when generating a URL #247

vjik opened this issue Jun 10, 2024 · 1 comment
Labels
status:ready for adoption Feel free to implement this issue.

Comments

@vjik
Copy link
Member

vjik commented Jun 10, 2024

UrlGeneratorInterface methods for URL generation have parameters: $arguments and $queryParameters. Currently, arguments used in route path (/route/{id}), and query parameters added to query (?id=7).

This behavior causes difficulties in use. For example, when in route we need to move query parameter to path:

// Before, result: /blog/post?id=42
$route = Route::get('/blog/post')->name('post')
$generator->generate('post', [], ['id' => 42])

// After, result: blog/post/42
$route = Route::get('/blog/post/{id}')->name('post')
$generator->generate('post', ['id' => 42], [])

In this case changed only route, code for generation should keep as is, but currently we need to change both route and generation code. It's bad.

I suggest to move unused arguments to query parameters, if not setted query parameter with such name:

// Before, result: /blog/post?id=42
$route = Route::get('/blog/post')->name('post')
$generator->generate('post', ['id' => 42], [])

// After, result: blog/post/42
$route = Route::get('/blog/post/{id}')->name('post')
$generator->generate('post', ['id' => 42], [])

Not need change generation code. We change only route configuration.

@samdark samdark added the status:ready for adoption Feel free to implement this issue. label Jun 11, 2024
@samdark
Copy link
Member

samdark commented Jun 11, 2024

Good idea. Let's do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue.
Projects
None yet
Development

No branches or pull requests

2 participants