Framework: [6.x, 8.x] Laravel 8 support PR broke auto method dependency resolving when string keys are used

[stancl]

• Laravel Version: 8.11.0
• PHP Version: 7.4

Description:

https://github.com/laravel/framework/pull/34884 broke Livewire (and presumably other things too).

Steps To Reproduce:

@livewire('my-component', ['foo' => 'bar'])

causes the result of static::getMethodDependencies($container, $callback, $parameters) to be ['foo' => 'bar']

Which results in:

Cannot unpack array with string keys

[patrickbrouwers]

Can confirm this issue with routes with string values (like a slug or uuid):

Route::get('articles/{article:slug}', [ArticlesController::class, 'show'])->name('articles.show');

[driesvints]

@stancl why are you passing arrays with string keys there?

[driesvints]

@stancl @patrickbrouwers do your both use cases work again if we revert this one single change?

https://github.com/laravel/framework/pull/34884#pullrequestreview-512974186

[patrickbrouwers]

@driesvints reverting that line indeed fixes the routing issue

[stancl]

@driesvints That way the values get matched with the parameters in mount() by their name.

Do you want me to try upgrading to the last version and reverting this one change in vendor?

[driesvints]

@stancl yes

[stancl]

@driesvints Yeah, fixes it completely 👍🏻

[timrspratt]

Wrapping it in array_values() appears to solve it.

$callback(...array_values(static::getMethodDependencies($container, $callback, $parameters)))

From my quick test, the livewire component properties resolve, even if the order doesn't match that in the mount() method.

[stancl]

@timrspratt How can it work when the order doesn't match and there are no names? Are you using typehints maybe?

[timrspratt]

@stancl try it? I think the getMethodDependencies uses reflection and puts them in the correct order. I am using typehints, but it should go off the property names.

[stancl]

Yeah, that works.

[taylorotwell]

Fixing and releasing a patch.

[driesvints]

Thanks all

[GrahamCampbell]

causes the result of static::getMethodDependencies($container, $callback, $parameters) to be ['foo' => 'bar']

I don't see how this can be the case. Where in the code does a string key get used?

[GrahamCampbell]

I don't see how this can be the case. Where in the code does a string key get used?

That should be fixed, rather than hacking an array_values in.

[driesvints]

@GrahamCampbell this was fixed. See taylor's comment above.

[GrahamCampbell]

Fixed with an array_values hack that shouldn't be needed. We need to work out what actually went wrong and fix the underlying issue.

[driesvints]

The method was first using call_user_func_array where the second argument expects an indexed array. All examples in the PHP manual are with non-named array keys and they don't matter for that function: https://www.php.net/manual/en/function.call-user-func-array.php

Changing this to directly calling the callback made this incorrect usage more explicit: https://github.com/laravel/framework/commit/82ffe3cd4a72887ba3e6f8f4bf8fa28fda1f3691#r43417881

Therefor this isn't a bug and calling array_values is a good workaround to the problem. Please don't re-open this.

[driesvints]

I found what's causing the issue in Livewire. It's because Livewire is explicitly overriding the getMethodDependencies in its extended class and altering the behavior of this method:

https://github.com/livewire/livewire/blob/master/src/ImplicitlyBoundMethod.php#L11-L23

I think it's best that Livewire wraps the result of that method in an array_values call unless it explicitly needs the named parameters (which I don't see a reason for).

[driesvints]

Ping @calebporzio ^