-
-
Notifications
You must be signed in to change notification settings - Fork 515
Description
Describe the bug
Currently, this package causes the app locale to be set when visiting routes that are not within a localized route group even when included in the urlsIgnored config option. This is caused by 'prefix' => LaravelLocalization::setLocale() which is part of the recommended configuration.
One would expect the app to use the default locale when visiting a route that is not localized. Since urlsIgnored is only checked within the middleware of this package and not in the LaravelLocalization::setLocale() method, this is not happening.
So the setLocale method will eventually negotiate the language from the user's Accept-Language header.
A short term solution might be to respect the urlsIgnored option in the LaravelLocalization::setLocale() method.
However, I think the entire LaravelLocalization::setLocale() being evaluated each time the routes file gets called is weird behaviour. From my experience working with Laravel, I would expect the locale to be set through middleware.
This however, would require a substantial amount of this package to be rewritten and would definitely be a breaking change.
I wouldn't mind having a go at this, so here are some thoughts that come to mind when setting the locale through middleware:
- Separate routes would need to be registered for each prefix (locale) within the
supportedLocalesconfig option.
This is something that should be handled by the package, not by the end user.
This would cause the amount of registered routes to grow, but should be fine when using route caching.
Ideally a user is able to register a group of routes that should be localized
- Route::group([
- 'prefix' => LaravelLocalization::setLocale(),
- 'middleware' => ['localeSessionRedirect', 'localizationRedirect', 'localeViewPath']
- ], function () {
- Route::get('about', function () { ... });
- });
+ Route::localizedGroup(function () {
+ Route::get('about', function () { ... });
+ });-
Separate routes per locale would allow this package to use Laravel's default
php artisan route:cachecommand.
This is currently a custom command within this package and has caused some issues in the past. -
Separate routes per locale would also show all localized routes within the
php artisan route:listcommand.
This would cause the command to display the following:
- GET|HEAD about-mcamara ..................................................................
+ GET|HEAD es/about-mcamara ...............................................................
+ GET|HEAD en/about-mcamara ...............................................................-
Setting the locale through middleware would make it easier for users to use this package within tests.
Currently, it is really cumbersome to start every test with$this->refreshApplicationWithLocale('en'). -
Setting the locale through middleware would make it easier to test this package itself.
A test case for the issues above looked something like this, where you really need to know the internals to know what's going on.
final class IgnoreUrlsTest extends TestCase
{
use UsesLocalizedRoutes;
protected function setUp(): void
{
putenv('APP_RUNNING_IN_CONSOLE=false');
parent::setUp();
}
protected function defineRoutes($router): void
{
$router->group([
'prefix' => LaravelLocalization::setLocale(),
'middleware' => [
LaravelLocalizationRoutes::class,
LaravelLocalizationRedirectFilter::class,
LocaleSessionRedirect::class,
],
], function () {});
$router->get('without-localization', function () {
return app()->getLocale();
});
}
protected function defineEnvironment($app): void
{
$app['config']->set('app.locale', 'en');
$app['config']->set('laravellocalization.urlsIgnored', ['/without-localization']);
$app->instance('request', Request::create(
uri: '/without-localization',
server: ['HTTP_ACCEPT_LANGUAGE' => 'es'],
));
}
public function testCanIgnoreUrls(): void
{
$response = $this->get('/without-localization');
$response->assertStatus(200);
$this->assertEquals('en', $response->getContent());
}
}So just sharing my thoughts here, but this could solve some related issues like #617
Steps to reproduce
- Create a route file with the following content
Route::group([
'prefix' => LaravelLocalization::setLocale(),
'middleware' => ['localeSessionRedirect', 'localizationRedirect', 'localeViewPath']
], function () {
Route::get('localized', fn () => app()->getLocale());
});
Route::get('not-localized', fn () => app()->getLocale());- Provide at least 2
supportedLocalesin the config option:
'supportedLocales' => [
'en' => ['name' => 'English', 'script' => 'Latn', 'native' => 'English', 'regional' => 'en_GB'],
'es' => ['name' => 'Spanish', 'script' => 'Latn', 'native' => 'español', 'regional' => 'es_ES'],
],- Configure your
app.localeto be set toen - Include the route that is not localized within the
urlsIgnoredconfig option:
'urlsIgnored' => ['/not-localized'],- Configure your browser to prefer the
eslanguage - Browse to
/not-localizedin your browser
Expected behavior
You should see the current locale be es instead of the expected en.