Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Routing] Deal with hosts per locale#36187
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
d5e5146
toe48a5b8
Comparenicolas-grekas commentedMar 31, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
What happens when the route defines both localized paths and localized hosts?
I'm not sure this is correct: other attributes (defaults, condition, etc) give precedence to the importing side, isn't it? |
odolbeau commentedMar 31, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Both will be used. I didn't add any test for this case cause TBH I don't know what's the expected behavior.
You tell me. :) I can remove this behavior (here) if you think it makes sense. |
can you please add one? the behavior should be that we synchronize both path and host to create only one route for the locale, that matches hostand path, both at the same time.
I confirm, seethis line that already has the correct precedence. It would make no sense to have precedence vary by type of declaration I think. |
This comment has been minimized.
This comment has been minimized.
e71c8b9
to79942b2
Comparesrc/Symfony/Component/Routing/Loader/Configurator/Traits/LocalizedRouteTrait.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Looks like all implementations (xml, yaml & php) are working now. :) Now waiting for your reviews! :) |
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.
Looks nice. Here are some questions + suggestions.
src/Symfony/Component/Routing/Loader/Configurator/ImportConfigurator.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Routing/Loader/Configurator/Traits/LocalizedRouteTrait.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Routing/Loader/Configurator/Traits/HostTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Routing/Loader/Configurator/Traits/HostTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
foreach ($host as $locale => $localeHost) { | ||
$localizedRoute = clone $route; | ||
$localizedRoute->setDefault('_locale', $locale); | ||
$localizedRoute->setRequirement('_locale', $locale); |
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.
missingpreg_quote()
btw, don't we miss this line inPrefixTrait
? if confirmed, the fix should be for 4.4
src/Symfony/Component/Routing/Loader/Configurator/Traits/HostTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
sweet thanks
Just one thing: can you please rebase and update the changelog of the component? |
Done! 👍 |
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.
LGTM, made a minor comment on the CHANGELOG entry.
Uh oh!
There was an error while loading.Please reload this page.
Documentation available here:symfony/symfony-docs#13571 |
Thank you@odolbeau. |
Hello@odolbeau , Could it work with the subdomain system? With the files below, will it be possible to have
Thanks :) Here are my different files: # config/routes/annotations.yamlcontrollers: resource:../../src/Controller/ type:annotation host: fr:www.example.fr en:www.example.com // src/Controller/BlogController.phpnamespaceApp\Controller;useSymfony\Bundle\FrameworkBundle\Controller\AbstractController;useSymfony\Component\Routing\Annotation\Route;class BlogControllerextends AbstractController{/** * @Route("/blog", name="blog_list") */publicfunctionlist() {// ... }} // src/Controller/Admin/BlogController.phpnamespaceApp\Controller\Admin;useSymfony\Bundle\FrameworkBundle\Controller\AbstractController;useSymfony\Component\Routing\Annotation\Route;class BlogControllerextends AbstractController{/** * @Route("/", name="admin_index", host="admin") */publicfunctionindex() {// ... }} |
This PR was squashed before being merged into the 5.2 branch.Discussion----------[Routing] Explain host per locale<!--If your pull request fixes a BUG, use the oldest maintained branch that containsthe bug (seehttps://symfony.com/releases for the list of maintained branches).If your pull request documents a NEW FEATURE, use the same Symfony branch wherethe feature was introduced (and `5.x` for features of unreleased versions).-->Reopens#13571 initially opened by `@odolbeau` but staled for a while then closed when removing master branch.This is just a copy/paste from the inital PR.Documents feature:symfony/symfony#36187Closes#13572Commits-------ebb266c [Routing] Explain host per locale
Uh oh!
There was an error while loading.Please reload this page.
Allow to define a different host for each locale in routing.
It's now possible to define this kind of configuration:
It's still possible to define an unique host (
host: wwww.example.com
) and if a host is defined for a given route directly, it's not overridden.To be done: