Skip to content

Conversation

gdebrauwer
Copy link
Contributor

@gdebrauwer gdebrauwer commented Jun 3, 2025

Why?

Currently, it is not possible to register a subdomain route that catches all requests to that subdomain (a fallback route). This is only possible if this route is registered before any other routes are registered. That can not be (easily) achieved when packages installed in the project are also registering routes, and if this catch-all subdomain route is registered inside a package itself.

// This 'login' route will still work on the 'link.laravel.test' subdomain
// even if the 'link.laravel.test' subdomain has a catch-all/fallback route
Route::get('/login', LoginController::class);

Route::domain('link.laravel.test')->group(function () {
    Route::get('{dynamicLink?}', DynamicLinkController::class)->where('dynamicLink', '.*');
});
Route::domain('link.laravel.test')->group(function () {
    Route::get('{dynamicLink?}', DynamicLinkController::class)->where('dynamicLink', '.*');
});

// If we register the 'login' route after the 'link.laravel.test' subdomain route
// then the 'login' route can not be hit on 'link.laravel.test' subdomain as
// that subdomain has a catch-all/fallback route.
Route::get('/login', LoginController::class);

You could say: link all root domain routes explicitly to a root domain, but that is a very heavy and big change you would have to make throughout your project (and its installed packages), and won't work if there are multiple root domains.

How?

This PR fixes that. It moves all subdomain routes to the top of the route collection. It still respects the registration order of those subdomain routes. By moving the subdomain routes to the top of the route collection, they will be checked first when a request comes in.

// This 'login' route will not work on the 'link.laravel.test' domain
// because the 'link.laravel.test' domain has a catch-all/fallback route
Route::get('/login', LoginController::class);

Route::domain('link.laravel.test')->group(function () {
    Route::get('{dynamicLink?}', DynamicLinkController::class)->where('dynamicLink', '.*');
});

Implications?

I targeted the master branch, just be sure, although I can't seem to think of a way this breaks existing applications. If there are domain routes registered across multiple domains in a project, then the order of those route registrations is still the preserved.

This would also mean that the following warning on https://laravel.com/docs/12.x/routing#route-group-subdomain-routing can be removed:
Screenshot 2025-06-03 at 18 53 45

@gdebrauwer gdebrauwer changed the title [13.x] Register domain routes before routes that are not linked to a domain [13.x] Register subdomain routes before routes that are not linked to a domain Jun 3, 2025
if ($route->getDomain()) {
$domainRoutes = array_filter($this->routes[$method] ?? [], fn ($route) => $route->getDomain() !== null);

$this->routes[$method] = $domainRoutes + [$domainAndUri => $route] + ($this->routes[$method] ?? []);
Copy link
Member

@taylorotwell taylorotwell Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gdebrauwer is this going to have extraneous routes in the $this->routes array? If we combine domainRoutes and still also include $this->routes doesn't that also include domain routes... since we just filtered them above and didn't partition them?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gdebrauwer mark as ready for review after commenting if you don't mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are associative arrays. If you use the plus sign to merge 2 associative arrays and a key is present in both associative arrays, then only the one in the first associative array is kept. That way, we avoid duplicates.

@taylorotwell taylorotwell marked this pull request as draft June 4, 2025 18:03
@gdebrauwer gdebrauwer marked this pull request as ready for review June 5, 2025 07:25
@taylorotwell taylorotwell merged commit 5ce08d4 into laravel:master Jun 5, 2025
51 checks passed
@taylorotwell
Copy link
Member

Thanks!

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.

2 participants