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

Carbon localization not set via App::setLocale() #551

Closed
Cluster2a opened this issue Jul 7, 2022 · 7 comments · Fixed by briannesbitt/Carbon#2640, #552 or #557
Closed

Carbon localization not set via App::setLocale() #551

Cluster2a opened this issue Jul 7, 2022 · 7 comments · Fixed by briannesbitt/Carbon#2640, #552 or #557

Comments

@Cluster2a
Copy link

  • Octane Version: 1.2
  • Laravel Version: 8.54
  • PHP Version: 8.1
  • Server & Version: Swoole 4.8.8
  • Database Driver & Version: 8.0.28

Description:

I am using a localization middleware that loads the user language and sets the App localization:

$userLang = Auth::user()->settings->get('language')->value;
if ($userLang) {
    App::setLocale($userLang);
}

Setting the localization of the App will result in heaving localized Carbon dates (Monday / Montag), as long as I run the App using NGINX Unit.

If I switch to Octane this code above is not working properly, as Carbon is always using the default language (en), instead of the App language (de). I additionally need to call Carbon::setlocale() to fix this behaviour:

$userLang = Auth::user()->settings->get('language')->value;
if ($userLang) {
    App::setLocale($userLang);
    Carbon::setlocale($userLang);
}

I am not sure why Carbon behaves differently on NGINX Unit / Octane. This might be a bug - I am not sure.
I would assume that the code (without Carbon::setlocale($userLang);) behaves in the same way on both servers.

Steps To Reproduce:

  1. run App using NGINX Unit
  2. set App localization
  3. Carbon behaves in a correct way
  4. switch to Onctane
  5. Carbone wont use the App localization for localization of dates
  6. Adding Carbon::setlocale() in the middleware will fix the error
@mcolominas
Copy link

mcolominas commented Jul 11, 2022

This is an expected problem, because of how Laravel Octane works.

Laravel Octane is instantiated only once, so carbon is also instantiated only once, even if you call it several times in different requests, therefore, it will always get the default language it was instantiated with.

But be careful, if for example, it is instantiated by the default language en, but you change it to another language, for example, es, then every time you call carbon, it will be in the language es, even if they are other requests.

In the case of languages, Laravel (at least in version 9) dispatch the Illuminate\Foundation\Events\LocaleUpdated event every time app()->setLocale() is called.

You could listen to this event, for each time the language is changed in your application, update it also in carbon and in other places where you need to update it.

EDIT:
Reviewing the internal files of Laravel, I just found a bug following this thread, the "LocaleUpdated" event is being heard within the Carbon\Laravel\ServiceProvider class, but in the following line:
https://github.com/briannesbitt/Carbon/blob/a9000603ea337c8df16cc41f8b6be95a65f4d0f5/src/Carbon/Laravel/ServiceProvider.php#L47

Gets the application from when laravel octane is instantiated, so even if the language is changed, it will continue to return the default language, causing carbon to not update.

A temporary solution to this problem until it is fixed, is to create your own service provider:
(this solution works for me)

namespace App\Providers;

use Carbon\Laravel\ServiceProvider;
use Illuminate\Container\Container;

class CarbonServiceProvider extends ServiceProvider
{
    public function updateLocale()
    {
        $this->app = Container::getInstance();
        parent::updateLocale();
    }
}

And disable the original provider in composer.json

"extra": {
    "laravel": {
        "dont-discover": [
            "nesbot/carbon"
        ]
    }
},

@Cluster2a
Copy link
Author

Thanks for the explanation and the workaround.

@driesvints
Copy link
Member

Thanks for the report @Cluster2a and for the solution @mcolominas. Looks like this indeed needs to be fixed within Carbon itself.

@kylekatarnls
Copy link

We can work around this in Carbon, I think about something like this:
briannesbitt/Carbon#2639

So we don't break support for existing application that might need events to be bound to $this->app and also be resilient if Container::getInstance() is not available.

@driesvints Can you explain why in the first place App::setLocale() is producing a LocaleUpdated events that can be listened in Laravel from within the auto-discovered service provider using $this->app['events'] and in octane suddenly it stop working, IMO this is a feature Laravel that got broken in octane and should be a concern for this repo.

If App is a facade for Container::getInstance() but service provider receive an other app in $this->app it could break many other things and is likely hiding some problem. Don't you think so?

@driesvints
Copy link
Member

I'm probably not the right person to answer this. @nunomaduro might shed some more light here.

@mcolominas
Copy link

@kylekatarnls Here he explains a little how octane works Dependency Injection & Octane

@kylekatarnls
Copy link

Please consider the idea exposed in #556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants