-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Routing] Add locale requirement for localized routes #35692
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
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.
I would make this strictness opt-in, with a strict_i18n
option defaulting to false for BC.
We should then trigger a deprecation that can be implemented in this PR, to default to true
in Symfony 6.
If it happens that there are use cases for false
within two years we can keep the option, otherwise deprecate it in 6.1 to remove it in 7.0.
1c269ae
to
8b3e8d0
Compare
I'd prefer no options for this. Options increase complexity a lot. |
8b3e8d0
to
1520c4a
Compare
Should target 4.4 I'd say. |
I agree, but this implementation is based on Should I create another PR ? |
Yes please |
Co-authored-by: Mathias Arlaud <mathias.arlaud@gmail.com> Co-authored-by: Ahmed Tailouloute <ahmed.tailouloute@gmail.com>
1520c4a
to
5e3f9d8
Compare
Closing in favor of #35735 actually. |
…rld) This PR was merged into the 4.4 branch. Discussion ---------- [Routing] Add locale requirement for localized routes | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT 4.4 version of #35692 If you're using localized routes, you expect to have these kind of routes available: - `/fr/accueil` - `/en/home` But nowadays, these routes are unexpectedly available: - `/en/accueil` - `/fr/home` When importing routes like that: - `prefix: "/{_locale}"` - `@Route({"en": "/home", "fr": "/accueil"}, name="home")` This PR proposes to add a strict locale requirement for localized so that the above routes won't be available. Commits ------- 50d7445 [Routing] Add locale requirement for localized routes
If you're using localized routes, you expect to have these kind of routes available:
/fr/accueil
/en/home
But nowadays, these routes are unexpectedly available:
/en/accueil
/fr/home
This PR proposes to add a strict locale requirement for localized so that the above routes won't be available.