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

[8.x] Respect custom route key with explicit route model binding #36375

Merged
merged 3 commits into from
Feb 25, 2021

Conversation

erikgaal
Copy link
Contributor

@erikgaal erikgaal commented Feb 24, 2021

Hi there! 🚀

You can use explicit route model binding in a situation where implicit binding might not work. Implicit binding allows you to customize the binding field for each route.

Route::get('/post/{post:slug}', fn (Post $post) => $post->content);
Route::get('/posts-by-id/{post}', fn (Post $post) => $post->content);

Unfortunately, this doesn't work when you're using explicit binding. It always uses the $model->getRouteKeyName().

Route::model('post', Post::class);

This PR makes sure that when explicitly binding models to route paramters, they respect any custom route fields being used.

Additionally, it passes the custom key to custom route binding.

Route::bind('post', function ($value, $route, $key) {
    // do something
});

@erikgaal erikgaal changed the title [8.x] Respect [8.x] Respect custom route key with explicit route model binding Feb 24, 2021
@taylorotwell
Copy link
Member

Does this contain any (even minor) breaking changes?

@GrahamCampbell
Copy link
Member

Breaking in the sense that someone relying on the old behaviour (because they assumed "that's just how it works"), will have their app broken.

@erikgaal
Copy link
Contributor Author

erikgaal commented Feb 24, 2021

Does this contain any (even minor) breaking changes?

I left the order of parameters unchanged so it doesn't break your current routes. But like @GrahamCampbell said, if you rely on the broken behavior, then this will be a breaking change.

@taylorotwell
Copy link
Member

Broken: #36443

Reverting.

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