tighten / ziggy

Use your Laravel routes in JavaScript.
MIT License
3.86k stars 247 forks source link

Make slash "not encoding" less restrictive #661

Closed aguingand closed 1 year ago

aguingand commented 1 year ago

The problem

Following up on #500. This feature is great but too restrictive:

Example:

Route::get('/{breadcrumb}/page/{page}')->where('breadcrumb', '.+');

Proposed solution

I move the / replacement after asserting the current param matches the given wheres. At this point if slashes are replaced it's because the RegExp allowed so.

bakerkretzmar commented 1 year ago

@aguingand thanks a lot for this. Great catch about the Regex, you're of course right it might not always be .*.

The slash encoding thing I'm going to dig into more... I see what you mean about the code, but the docs are correct in the sense that routing only works with slashes if they're in the last parameter, encoded or not. See https://github.com/laravel/framework/issues/22125 and https://symfony.com/doc/current/routing.html#slash-characters-in-route-parameters. I'm not certain whether that's relevant or matters here though since that all happens after the URL is generated. I'm going to try to add some tests here to see.

aguingand commented 1 year ago

I have tested in my project, it works well with a single starting param containing slash (both laravel + ziggy). But some js tests may be relevant here.

bakerkretzmar commented 1 year ago

Thanks! Gonna come back to the encoding thing separately asap.