[RFC] Added breadcrumbs in route

[tabuna]

Description of the problem

I am always a little upset when you need to find and change bread crumbs for several routes. Due to the fact that they are indifferent files and are not related to each other. Therefore, you must first open the file with the routes to see the name of the route, then open another and find a match in it.

// routes/web.php
Route::get('/')->name('home');
Route::get('/about')->name('about');

// routes/breadcrumbs.php
Breadcrumbs::for('home', function ($trail) {
    $trail->push('Home', route('home'));
});

Breadcrumbs::for('about', function ($trail) {
    $trail->parent('home');
    $trail->push('About', route('about'));
});

This is not a problem when there are few routes, but when there are many in your application and there are breadcrumbs for everyone, it starts to bother.

Proposed solution

My suggestion is very simple - combine the announcement of routes and breadcrumbs.

Route::get('/')->name('home')->crumbs(function ($trail) {
    $trail->push('Home', route('home'));
});

Route::get('/about')->name('about')->crumbs(function ($trail) {
    $trail->parent('home');
    $trail->push('About', route('about'));
});

As you can see, we have lost the duplicate required parameter name.

Questions and additions:

Will we break backward compatibility?

Not at all. This should not bring any breaking changes to the code. Instead, we add middleware that you just need to enable.

Routes are cached, can I use them?

Of course, the definition of breadcrumbs will also be cached. And when you change it will be necessary to flush the cache.

Will there be performance issues?

I guess not. Because we will only transfer the registration of breadcrumbs from the service provider, at the time the routes are completed. This can reduce performance, but only if you use tools like spiral/roadrunner.

Other

What do you think about this? The repository has disabled issues, so I attached a draft implementation.

[d13r]

I like it. 😃

I would name it breadcrumbs() not crumbs().

I'd probably remove && !Breadcrumbs::exists($route->getName()), unless there's a good reason for it. (If it's to prevent the middleware being run twice, I think that should be a global flag, maybe on the breadcrumbs manager class, not checked for every route separately.)

I'm not sure I particularly like the hack of storing the closures in a default parameter and then removing it again... But I can see why it may be necessary to deal with route caching. I wonder if it would be better to write our own cache file instead when using route caching; not sure how doable that is though. I think it would be better to not serialise the closures at all when not using route caching - more efficient, and (I assume - haven't tried it) easier to debug because the line numbers will be correct.

Thanks!

[tabuna]

I would name it breadcrumbs() not crumbs().

I have no preference, I chose this name because it is short :sob:

I'd probably remove && !Breadcrumbs::exists($route->getName()), unless there's a good reason for it. (If it's to prevent the middleware being run twice, I think that should be a global flag, maybe on the breadcrumbs manager class, not checked for every route separately.)

I use this in my application since not all the application has been translated to record in routes yet. I think this will not be necessary.

But I can see why it may be necessary to deal with route caching. I wonder if it would be better to write our own cache file instead when using route caching; not sure how doable that is though.

Caching is not an option, but a requirement. Otherwise, any of my attempts to load routes simply will not be executed. Since the real file, for example, /routes/web.php are not executed, and the data is taken from the cache. If you know this method, then I would really like to know about this (Without using substitution and reflection :smirk:)

I like it.

I'm not sure I particularly like the hack of storing the closures in a default parameter and then removing it again...

I do not understand. You like this, but at the same time you do not approve of the temporary default value. Should I continue to work on this?

[ghost]

I do not understand. You like this, but at the same time you do not approve of the temporary default value. Should I continue to work on this?

I like the idea of adding a macro to the routes, but not sure about the implementation of storing the data in the route default parameters.

My idea is to (1) if caching is disabled, just pass the closure straight to Breadcrumbs::register() without serializing it, and (2) if it is enabled, generate our own bootstrap/cache/breadcrumbs.php when route:cache is run, and load the data back from there.

[tabuna]

My idea is to (1) if caching is disabled, just pass the closure straight to Breadcrumbs::register() without serializing it

If caching is enabled, the code that is specified in the route files will not be executed. (A cache file will be loaded instead bootstrap/cache/routes-v7.php). To do this, you will have to listen to the execution of the command artisan route:cache

if it is enabled, generate our own bootstrap/cache/breadcrumbs.php when route:cache is run, and load the data back from there.

I am not ready to work on this. Since there are no events in the command I will continue my journey separately.

[ghost]

Well, like I said, it's not a deal-breaker, just not ideal.

I wonder if it would be better to write our own cache file instead when using route caching; not sure how doable that is though.