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

[11.x] Add ability to override RouteServiceProvider #51249

Closed
wants to merge 2 commits into from
Closed

[11.x] Add ability to override RouteServiceProvider #51249

wants to merge 2 commits into from

Conversation

ahmedabdel3al
Copy link
Contributor

@ahmedabdel3al ahmedabdel3al commented Apr 30, 2024

I need to override loadCacheRoutes method inside RouteServiceProvider as we did before laravel 11 but when i extend the framework RouteServiceProvider and add it inside the bootstrap/providers.php
the framework RouteServiceProvider always run last
i think we are forcing it here #49732

@taylorotwell
Copy link
Member

Can you describe instead what you are trying to accomplish as your end goal?

@ahmedabdel3al
Copy link
Contributor Author

ahmedabdel3al commented Apr 30, 2024

https://github.com/mcamara/laravel-localization
We are using this package for many projects to translate routes. When we cache routes using their command, it generates many files inbootstrap/cache based on the locales supported by the application.
For example, if we have 3 locales [es, en, ar], this will generate 3 files:

  • routes-v7_es.php
  • routes-v7_en.php
  • routes-v7_ar.php

Based on the current locale of the request, they load the correct cached routes file. So, we have overridden the RouteServiceProvider.
https://github.com/mcamara/laravel-localization?tab=readme-ov-file#caching-routes

https://github.com/mcamara/laravel-localization/blob/master/src/Mcamara/LaravelLocalization/Traits/LoadsTranslatedCachedRoutes.php

@rodrigopedra
Copy link
Contributor

How about making Application@resolveProvider resolve the provider from the container? Instead of new'ing up the class directly from its class name?

public function resolveProvider($provider)
{
return new $provider($this);
}

This way, one could bind their custom provider on their AppServiceProvider@register method.

I would say, as customizing this appears to be very specific, would work better than adding one more parameter to the withRouting() method.

@ahmedabdel3al
Copy link
Contributor Author

@rodrigopedra I don't understand your point fully.

IMO, when anyone open the bootstrap/app.php with this code

Application::configure(basePath: dirname(__DIR__))
    ->withRouting(
        commands: __DIR__.'/../routes/console.php',
        health: '/up',
        provider: \App\Providers\RouteServiceProvider::class
    )

will totally understand this provider will be instead of the framework's one

@rodrigopedra
Copy link
Contributor

Based on the current locale of the request, they load the correct cached routes file.

This is working as intended, right?

So, we have overridden the RouteServiceProvider.

Can you provide your overridden method, so we can better understand what you are trying to accomplish?

@ahmedabdel3al
Copy link
Contributor Author

ahmedabdel3al commented May 3, 2024

@rodrigopedra

Based on the current locale of the request, they load the correct cached routes file.

This is working as intended, right? Yes

So, we have overridden the RouteServiceProvider.

Can you provide your overridden method, so we can better understand what you are trying to accomplish?

https://github.com/mcamara/laravel-localization/blob/master/src/Mcamara/LaravelLocalization/Traits/LoadsTranslatedCachedRoutes.php

/**
     * Load the cached routes for the application.
     *
     * @return void
     */
    protected function loadCachedRoutes()
    {
        $localization = $this->getLaravelLocalization();

        // compute $locale from url.
        // It is null if url does not contain locale.
        $locale = $localization->getInversedLocaleFromMapping(
            $localization->setLocale()
        );

        $localeKeys = $localization->getSupportedLanguagesKeys();

        // First, try to load the routes specifically cached for this locale
        // if they do not exist, write a warning to the log and load the default
        // routes instead. Note that this is guaranteed to exist, because the
        // 'cached routes' check in the Application checks its existence.

        $path = $this->makeLocaleRoutesPath($locale, $localeKeys);

        if ( ! file_exists($path)) {

            Log::warning("Routes cached, but no cached routes found for locale '{$locale}'!");

            $path = $this->getDefaultCachedRoutePath();
        }

        $this->app->booted(function () use ($path) {
            require $path;
        });
    }

    /**
     * Returns the path to the cached routes file for a given locale.
     *
     * @param string   $locale
     * @param string[] $localeKeys
     * @return string
     */
    protected function makeLocaleRoutesPath($locale, $localeKeys)
    {
        $path = $this->getDefaultCachedRoutePath();

        if ( ! $locale || ! in_array($locale, $localeKeys)) {
            return $path;
        }

        return substr($path, 0, -4) . '_' . $locale . '.php';
    }

    /**
     * Returns the path to the standard cached routes file.
     *
     * @return string
     */
    protected function getDefaultCachedRoutePath()
    {
        return $this->app->getCachedRoutesPath();
    }

    /**
     * @return \Mcamara\LaravelLocalization\LaravelLocalization
     */
    protected function getLaravelLocalization()
    {
        return app('laravellocalization');
    }

i prefer if you can also look at https://github.com/mcamara/laravel-localization?tab=readme-ov-file#caching-routes

@rodrigopedra
Copy link
Contributor

Ok, I get it now. Sorry for the insistence.

From that repo's issue tracker, it seems people are finding better results with another package:

mcamara/laravel-localization#899 (comment)

Maybe, it is worth taking a look in the meanwhile.

@ahmedabdel3al
Copy link
Contributor Author

Even if there's another package that can perform the same function,
the issue of overriding this RouteServiceProvider will remain. It was a feature that the framework gave me full permissions to override. After the latest release, this feature has been deprived.

@driesvints driesvints changed the title add ability to override RouteServiceProvider [11.x] Add ability to override RouteServiceProvider May 3, 2024
@taylorotwell
Copy link
Member

I think there is probably a better solution to this. Even something like loadCachedRoutesUsing or some sort of specific solution.

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

Successfully merging this pull request may close these issues.

3 participants