laravel / folio

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

Fix /index route replacement #132

Closed satoved closed 8 months ago

satoved commented 8 months ago

Fixes the issue with /index handling when generating routes. Closes https://github.com/laravel/folio/issues/131.

Behavior description There's an existing behavior to handle index.blade.php files, that replaces index in the url with empty string, to create a proper route.

Let's say I have /pages/foo/index.blade.php page with a name('foo').

route('foo') would then generate /foo/index and replace the /index part to an empty string.

The bug Let's say I have /pages/foo/[slug].blade.php page with a name('foo').

And my slug accidentally start with word "index".

route('foo', ['slug' => 'index-shows-growth']) should then generate /foo/index-shows-growth

But, because of the rule above, it'll generate /foo-shows-growth with /index replaced out.

The solution Change replacing function to regex and add an end of line to the end of the pattern.

That way we still replace /index to empty string, but won't replace slugs that start with /index.

driesvints commented 8 months ago

Heya thanks. Best that you add a thorough explanation to your PR description and not just link to an issue (see the PR template when you open a new PR).

satoved commented 8 months ago

Sorry @driesvints, I've updated the PR description.

nunomaduro commented 8 months ago

Decided to add a few more tests and change the implementation of the fix here: https://github.com/laravel/folio/pull/133.