laravel / folio

Page based routing for Laravel.
MIT License
568 stars 46 forks source link

Wildcard route collision #120

Closed undjike closed 10 months ago

undjike commented 12 months ago

Folio Version

1.1.3

Laravel Version

10.22.0

PHP Version

8.2.3

Description

Hello team, Thanks for the great job. Laravel Folio is really amazing.

Thus, I don't know if I should create a new issue for it has almost the same topic. I still have some routes (files) that are not accessible due to collision with another wildcard route.

image

The route /create is not reachable. When trying to access it, Folio keeps using /{id} and just passes the term create as $id.

Steps To Reproduce

Create a new Laravel project using Folio and the following tree structure (without implicit model binding).

image

nunomaduro commented 12 months ago

Can you create an Laravel application that faces this issue, push it to GitHub, an give me the link so I can try it locally please?

nunomaduro commented 12 months ago

Nevermind. I understand the issue now. So, at the moment, if you are using something like:

create.blade.php
[User].blade.php

When passing /create, Folio will try to see if there is any user with the ID create. If not, it will try to match literal views, and of course, create.blade.php matches the given value, so create.blade.php will be used.

In your use case:

create.blade.php
[id].blade.php

The [id].blade.php does match the given /create, because the create string does match the given wildcard ID. I don't think there is an issue here, because wildcard views do take precedence over literal views at the moment.

@taylorotwell What's your opinion on this?

undjike commented 12 months ago

I can suggest one of the options below:

  1. In FolioServiceProvider, we could introduce a property called $pipelinePriority (as is the case for middlewares in Kernel.php). This will be used to define precedence in the Folio route resolver.

  2. Or, similarly, add a config key to hold this prioritization.

  3. Or there can be just a variable or a config key $prioritizeLiterals. If true, literals take precedence over wildcards, else...

So either way, everyone could configure the priority regarding his use case.

This will concern only these 04 pipes I think.

image

I hope it makes sense !

undjike commented 12 months ago

I can suggest one of the options below:

  1. In FolioServiceProvider, we could introduce a property called $pipelinePriority (as is the case for middlewares in Kernel.php). This will be used to define precedence in the Folio route resolver.
  2. Or, similarly, add a config key to hold this prioritization.

So either way, everyone could configure the priority regarding his use case.

I hope it makes sense !

Of course, this might be fuzzy if in the same project, one uses at times model binding and at times not.

undjike commented 10 months ago

@nunomaduro I think I'm gonna close this issue in favor of a new one... 🤔

I was wondering why I didn't use model binding and then remembered that I didn't because I was facing an issue with the connection used for binding resolution.

The thing is... It seems like the Folio route model binding resolution happens before middlewares are applied 🤔. Thus, in my case, I had a middleware in charge of changing the current database connection dynamically.

I tried using the default routing system in the same project and things started working as expected. Back to Folio, it's no longer working (resolving the suitable database connection).

undjike commented 10 months ago

Closed in favor of https://github.com/laravel/folio/issues/123