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

[9.x] Widen the type of Collection::unique $key parameter #40903

Merged
merged 3 commits into from
Feb 10, 2022
Merged

[9.x] Widen the type of Collection::unique $key parameter #40903

merged 3 commits into from
Feb 10, 2022

Conversation

NiclasvanEyk
Copy link
Contributor

This was narrowed down to bool, but the docs and implementation
actually support arbitrary values.

As suggested in #40897 I PRed the changes to Collection and other related classes like LazyCollection.

Description:

Currently the $key parameter of the Collection::unique method has the following type annotation:

    /**
     * Return only unique items from the collection array.
     *
     * @param  (callable(TValue, TKey): bool)|string|null  $key
     * @param  bool  $strict
     * @return static
     */
    public function unique($key = null, $strict = false)

So basically it can be

  • a function that returns bool
  • a string
  • nothing at all

I think the first one is a little bit too strict. Looking at https://laravel.com/docs/9.x/collections#method-unique I should be able to return any value to determine uniqueness:

Finally, you may also pass your own closure to the unique method to specify which value should determine an item's uniqueness:

$unique = $collection->unique(function ($item) {
    return $item['brand'].$item['type'];
});
 
$unique->values()->all();
 
/*
    [
        ['name' => 'iPhone 6', 'brand' => 'Apple', 'type' => 'phone'],
        ['name' => 'Apple Watch', 'brand' => 'Apple', 'type' => 'watch'],
        ['name' => 'Galaxy S6', 'brand' => 'Samsung', 'type' => 'phone'],
        ['name' => 'Galaxy Gear', 'brand' => 'Samsung', 'type' => 'watch'],
    ]
*/

This was narrowed down to bool, but the docs and implementation
actually support arbitrary values.
@nunomaduro nunomaduro merged commit e76a1ec into laravel:9.x Feb 10, 2022
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 this pull request may close these issues.

2 participants