Routes are stil registered even after caching them

[rjorel]

Hello,

I'm not sure it's an "issue" strictly saying, but it would be nice to avoid re-parsing controller files when routes are cached. I don't know how fast registering routes is, but I think it's always a gain to avoid re-parsing.

In service provider class, this method is defined to register routes:

protected function registerRoutes(): void
{
    if (! config('route-attributes.enabled')) {
        return;
    }

    $routeRegistrar = (new RouteRegistrar(app()->router))
        ->useRootNamespace(app()->getNamespace())
        ->useMiddleware(config('route-attributes.middleware') ?? []);

    collect($this->getRouteDirectories())->each(fn (string $directory) => $routeRegistrar->registerDirectory($directory));
}

Laravel Application class offers public method routesAreCached() to find out if route registration is required, so I think it's not difficult to add this check. I'd like to do it and make a pull request, but I don't know what kind of test can ensure this behavior.

So I prefer let you have a look, to avoid an useless pull request without any test.

Regards.
Raphael Jorel

[rjorel]

I suppose something like that may fulfill this suggestion, but how to test it?

protected function registerRoutes(): void
{
    if (! config('route-attributes.enabled') || $this->app->routesAreCached()) {
        return;
    }

    $routeRegistrar = (new RouteRegistrar(app()->router))
        ->useRootNamespace(app()->getNamespace())
        ->useMiddleware(config('route-attributes.middleware') ?? []);

    collect($this->getRouteDirectories())->each(fn (string $directory) => $routeRegistrar->registerDirectory($directory));
}

[freekmurze]

Send a PR for this, and I'll try to add a test for this. Make sure all the current tests pass.

[rjorel]

Hello,

Thanks for merging. I have seen your modifications and I just wonder why preferred a method with two if and three return rather than returning one boolean expression. As it seems to me better to make boolean expressions in such cases, your opinion on this should be interesting for me.

About my check with CachesRoutes, it was inspired by Laravel framework (https://github.com/laravel/framework/blob/master/src/Illuminate/Support/ServiceProvider.php#L143) because Application interface does not provide routesAreCached method.

[freekmurze]

To me, my refactor makes the code more readable
Another example: https://spatie.be/videos/readable-laravel/refactor-complex-conditionals

[rjorel]

Thank you for replying 😃