tighten / ziggy

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

Fix parameter validation when generating relative URLs to routes with domain parameters #755

Open bakerkretzmar opened 3 months ago

bakerkretzmar commented 3 months ago

Laravel requires that domain parameters always be passed to the route() helper, even if the user is generating a relative URL and the domain parameters will not appear in the final output. Omitting the parameter results in an error. Depending how the parameters are passed the error will be different, but the underlying problem is the same:

Route::domain('{team}.ziggy.dev')->get('users/{user}', fn () => '')->name('users.index');

// Desired output: "/users/1"

route('users.index', ['team' => 'tighten', 'user' => 1], absolute: false);
// Produces desired output

route('users.index', ['user' => 1], absolute: false);
// Error, missing required route parameter 'team' - only one param value was passed, and it was specifically assigned to the `user` param

route('users.index', ['tighten', 1], absolute: false);
// Produces desired output

route('users.index', [1], absolute: false);
// Error, missing required route parameter 'user' - because param values were passed in order, `1` was handled as the `team` parameter

Ziggy currently behaves exactly the opposite way—it effectively prohibits domain parameters from being passed to the route() helper and handled as such, if the user is generating a relative URL:

// Desired output: "/users/1"

route('users.index', { user: 1 }, false);
// Produces desired output, unlike Laravel, which errors in this case

route('users.index', { team: 'tighten', user: 1 }, false);
// Produces "/users/1?team=tighten", incorrectly adding the `tighten` domain parameter to the query

route('users.index', [1], false);
// Produces desired output, unlike Laravel, which errors

route('users.index', ['tighten', 1], false);
// Produces "/users/tighten?1", incorrectly handling `'tighten'` as the `user` parameter and adding `1` to the query

So Laravel requires that all required parameters be provided, and Ziggy requires that only the required parameters that will appear in the output be provided.

This was almost certainly unintentional, and a side effect of how we replace parameters in the URL. Ziggy generates a template of the final output of the route() helper, which, if the relative option was passed, only contains the route path, and then replaces all the parameters in that template. Laravel generates a template of the entire URL, replaces all the parameters in that template, and then returns only the path if the relative option was passed.

I'm going to propose that we update Ziggy to match Laravel's behaviour, which is a large change, but justified in my opinion since it fixes what I believe is a large bug.

For now this PR includes a failing test demonstrating the issue.