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

[Routing] Improve localized routes performances #35855

Merged

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Feb 25, 2020

Q A
Branch? 4.4
Bug fix? no
License MIT

Implementation of the following idea: #35735 (review)

Improve route matching performances by turning dynamic routes with fixed _locale to actual static routes.

@mtarld mtarld force-pushed the feature/localized-route-improvements branch from 7d6e446 to 8e9eafe Compare February 25, 2020 12:41
@nicolas-grekas nicolas-grekas added this to the 4.4 milestone Feb 25, 2020
@nicolas-grekas
Copy link
Member

Thank you @mtarld.

@nicolas-grekas nicolas-grekas merged commit ef95f2e into symfony:4.4 Feb 25, 2020
nicolas-grekas added a commit that referenced this pull request Mar 2, 2020
This PR was merged into the 4.4 branch.

Discussion
----------

[Routing] Fix some wrong localized routes tests

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

~~Since #35855, the `_locale` path param is directly substituted so those tests are not valid cases anymore. Instead, the path directly contain the right locale.~~ Actually, the compilation is done after, so instead we need to set the new requirement in all tests to reflect the "reality".

#35855 also causes a BC break on one case:
```php
$compiledUrlGenerator->generate('foo.fr', ['_locale' => 'en']))
```

Previously, the generated route would be the `/en/fourchette`. Now that the locale is hardcoded in the route path, it will always be `/fr/fourchette`. I changed `foo` to relevant words because it is easier to understand like that.

Commits
-------

99ae55f [Routing] Fix some wrong localized routes tests
symfony-splitter pushed a commit to symfony/routing that referenced this pull request Mar 2, 2020
This PR was merged into the 4.4 branch.

Discussion
----------

[Routing] Fix some wrong localized routes tests

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

~~Since symfony/symfony#35855, the `_locale` path param is directly substituted so those tests are not valid cases anymore. Instead, the path directly contain the right locale.~~ Actually, the compilation is done after, so instead we need to set the new requirement in all tests to reflect the "reality".

symfony/symfony#35855 also causes a BC break on one case:
```php
$compiledUrlGenerator->generate('foo.fr', ['_locale' => 'en']))
```

Previously, the generated route would be the `/en/fourchette`. Now that the locale is hardcoded in the route path, it will always be `/fr/fourchette`. I changed `foo` to relevant words because it is easier to understand like that.

Commits
-------

99ae55ff1a [Routing] Fix some wrong localized routes tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants