Skip to content

[Routing] Allow query-specific parameters in UrlGenerator using _query #60508

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2025
Merged

[Routing] Allow query-specific parameters in UrlGenerator using _query #60508

merged 1 commit into from
Jun 5, 2025

Conversation

BenMorel
Copy link
Contributor

@BenMorel BenMorel commented May 22, 2025 •
edited by OskarStark
Loading

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

This PR adds support for a special _query key in $parameters of UrlGenerator::generate(), that is used exclusively to generate query parameters. This is useful when query parameters may conflict with route parameters of the same name.

Concrete use case:

My application has a route that looks like:

https://{siteCode}.{domain}/admin/stats

And I want to generate this URL:

https://fr.example.com/admin/stats?siteCode=us

With this PR, I can now call:

$urlGenerator->generate('admin_stats', [
    'siteCode' => 'fr',
    'domain' => 'example.com',
    '_query' => [
        'siteCode' => 'us',
    ]
]);

@carsonbot carsonbot added this to the 7.3 milestone May 22, 2025
@carsonbot carsonbot changed the title Allow query-specific parameters in UrlGenerator using _query [Routing] Allow query-specific parameters in UrlGenerator using _query May 22, 2025
@OskarStark OskarStark changed the title [Routing] Allow query-specific parameters in UrlGenerator using _query [Routing] Allow query-specific parameters in UrlGenerator using _query May 22, 2025
@OskarStark
Copy link
Contributor

If someone is using _query already, it could be a BC break, which would need a deprecation first.

@BenMorel
Copy link
Contributor Author

If someone is using _query already, it could be a BC break, which would need a deprecation first.

I agree, unless it is considered that names starting with an underscore are reserved for Symfony, and not covered by the BC promise; I don't know about this, but there's a precedent here with _locale.

@stof
Copy link
Member

stof commented May 22, 2025

The difference is that _locale exists since the beginning of Symfony.

A better example might be to look at how we handled _fragment. I'm almost sure we added it later.

@BenMorel
Copy link
Contributor Author

@stof Good point. _fragment was introduced in 2014, in 6d79a56, PR #12979, and was first released in v3.2.0 — a minor release.

The BC break concern was raised by @Koc back then, but was dismissed by @Tobion:

And the tiny BC break is negligible because it's unlikely.

I'll leave it up to you to decide whether introducing another one in a minor release is still acceptable in 2025; maybe this is something that could be documented in Our Backward Compatibility Promise?

xabbuh
xabbuh reviewed May 22, 2025
View reviewed changes
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
nicolas-grekas
nicolas-grekas reviewed May 30, 2025
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, just one comment

@BenMorel BenMorel force-pushed the _query branch 3 times, most recently from bcc1a01 to 34568da Compare June 1, 2025 22:59
nicolas-grekas
nicolas-grekas approved these changes Jun 2, 2025
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 minor changes and GTM

@nicolas-grekas
Copy link
Member

Thank you @BenMorel.

@nicolas-grekas nicolas-grekas merged commit 33ecd1c into symfony:7.4 Jun 5, 2025
10 of 11 checks passed
@nicolas-grekas nicolas-grekas mentioned this pull request Jun 5, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GromNaN GromNaN GromNaN left review comments

@xabbuh xabbuh xabbuh left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@OskarStark OskarStark OskarStark approved these changes

@mtarld mtarld mtarld approved these changes

Assignees
No one assigned
Labels
Feature Routing Status: Reviewed
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.
9 participants
@BenMorel @OskarStark @stof @nicolas-grekas @GromNaN @xabbuh @mtarld @fabpot @carsonbot