laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.41k stars 10.99k forks source link

Collection::groupBy should not return a new static() because that might be an EloquentCollection #53295

Open rudiedirkx opened 4 hours ago

rudiedirkx commented 4 hours ago

Laravel Version

11.23.5

PHP Version

8.2.14

Description

Currently, a collection groupBy() returns the same type of collection, containing the same type of collection: @return static<array-key, static<array-key, TValue>>. That's good for the base collection, of children that accept any type of TValue, but not for EloquentCollection, because that doesn't accept any type of TValue, but only Model: @template TModel of \Illuminate\Database\Eloquent\Model. So groupBy() should check for that (or EloquentCollection could overwrite groupBy()) to not return new static, but new Illuminate\Support\Collection.

Steps To Reproduce

$users = User::all();
assert($users instanceof EloquentCollection); // 🟩 

$groups = $users->groupBy('status');
assert($users instanceof EloquentCollection); // πŸŸ₯ because EloquentCollection should only contain Model, not EloquentCollection
assert($users->first() instanceof EloquentCollection); // 🟩 

Fixing might be super backward incompatible, but it is correcter. Workaround is pretty simple though: add in custom model collection (which everyone has, right?):

/** @return BaseCollection<array-key, $this> */
public function groupBy($groupBy, $preserveKeys = false) {
    $groups = parent::groupBy($groupBy, $preserveKeys); // temporary 'impossible' EloquentCollection of EloquentCollections
    return BaseCollection::make($groups->all());
}
browner12 commented 3 hours ago

I wonder if this is related to my issue #53283