search

drewroberts / blog

📝 Laravel package for my opinionated style of adding blog posts to Laravel projects
MIT License
2 stars 4 forks source link

Flag generic route as fallback instead of using middleware to inject #70 #83

Closed pdbreen closed 2 years ago

pdbreen commented 3 years ago

Putting this out as an alternative to the middleware injection of the generic route. But - this solution has its own issues. While it leverages the ->fallback() method to flag the route as a fallback, the way laravel implements fallbacks is by simply sorting ALL routes flagged as a fallback so they come last (actually not even a sort - it partitions non fallback and fallback routes then merges the fallback routes back onto the non fallback routes making them last). Because of this, there is no way to control the ordering within the fallbacks themselves. So, if there are multiple packages with fallback routes, OR the app registers its own fallback route (which uses a .* pattern to match everything) this solution could suddenly stop working / stop serving pages and posts because some other fallback route is being matched.

Looking at the classes that implement the fallback sorting logic, it would be difficult to customize the laravel behavior via a modified class override in the container.

drewroberts commented 3 years ago

How would this approach differ from our current approach in regard's to @jasonmccreary's questions (https://github.com/drewroberts/blog/issues/70) here:

  • Are application fallback routes called when package is installed?
  • Can package routes leverage fallback routes in some way?
  • Can package routes be cached?
  • Does any of the above change with or without Nova installed?
pdbreen commented 3 years ago

It deals with the last 3 --- but not the first. That would require some type of predictable sequencing when an app also defines a fallback route - the big issue I'm calling out in the original description. Depending on how the fallback routes end up being ordered, it will either behave the same as the current solution (stealing all 1, 2 or 3 segment routes for blogs) or, it won't work at all and the blog page and post routes stop working completely.

pdbreen commented 3 years ago

I should add - the only route not cached is the single generic route and not caching a single route isn't really a big deal (item 3) and the current solution does require Nova to be installed because the middleware is extending an existing Nova class (item 4). But, nova is a stated dependency for all of these packages.

So, this PR really only addresses item 2 - item 3 and 4 don't need any resolution.