Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v9.15 has breaking change in route() helper: Unidentified array key 0 #42605

Closed
mortenscheel opened this issue Jun 1, 2022 · 5 comments · Fixed by #42606
Closed

v9.15 has breaking change in route() helper: Unidentified array key 0 #42605

mortenscheel opened this issue Jun 1, 2022 · 5 comments · Fixed by #42606

Comments

@mortenscheel
Copy link

mortenscheel commented Jun 1, 2022

  • Laravel Version: 9.15.0
  • PHP Version: 8.0.18
  • Database Driver & Version: MySQL 8.0

Description:

In v9.15.0 calling the route() helper with model parameters in a non-associative array will result in an ErrorException "Unidentified array key 0" being thrown in Route.php line 539.
I realize that the use of non-associative arrays is no longer documented, but as long as the order of the parameters was correct, it has worked until v9.14.0.
If there have been any warnings in the upgrade guides that this would happen, I must have missed it. And it's especially unexpected in a minor release.

Steps To Reproduce:

route('route.name', [$someModel]); // Throws ErrorException
route('route.name', $someModel); // Also worked up until v9.14.0, now it throws ErrorException
@mortenscheel
Copy link
Author

Closing this issue, because it's not as simple as initially stated. I will post a new issue if I can figure out what exactly causes this to break.

@mortenscheel
Copy link
Author

It turns out that the issue only happens in a rather obscure edge case.
It still works fine, as long as the model is a route parameter.
But if it's not, the route() call will fail as it tries to turn the model into a query param.

Route::get('/foo', function() {
    // ...
})->name('foo');

$url = route('foo', [User::first()]);

This throws an error now.
Up until v9.14, it would produce an URL like http://example.com/foo?1 with 1 being the model's id.

@ksassnowski
Copy link
Contributor

ksassnowski commented Jun 1, 2022

I swear to God, I must have hit every single possible edge case with my initial PR 🙈

The fix is rather straight forward, but I think this behavior is pretty weird to begin with (what does http://example.com/foo?1 even mean?). Since this is technically a breaking change in a non-major version, however, we should still fix this. I'll send a PR for this shortly.

You might want to open this issue again in the meantime.

@mortenscheel
Copy link
Author

@ksassnowski I completely agree it's a weird edge case. I believe it arose because the model used to be a route param, then the route definition was changed to not include the model, without updating the route() call. Like I said, it flew under the radar because the generated URL worked as expected, even though it had a non-sensical ?1 tagged on.
As always, xkcd said it best

I'll re-open the issue as suggested.

@mortenscheel mortenscheel reopened this Jun 2, 2022
@ksassnowski
Copy link
Contributor

The PR already got merged by the way, so you can close the issue again 😅 #42606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants