Skip to content

[5.4] Move "tap" method to BuildsQueries trait.#20384

Merged
taylorotwell merged 2 commits into
laravel:5.4from
ed-fruty:move_tap_method
Aug 4, 2017
Merged

[5.4] Move "tap" method to BuildsQueries trait.#20384
taylorotwell merged 2 commits into
laravel:5.4from
ed-fruty:move_tap_method

Conversation

@ed-fruty
Copy link
Copy Markdown
Contributor

@ed-fruty ed-fruty commented Aug 2, 2017

Method tap was moved from Illuminate\Database\Query\Builder to Illuminate\Database\Concerns\BuildsQueries trait.

Eloquent builder when and tap methods passes different builder instances to the callback.
when passes current eloquent builder to the callback, but tap method uses Illuminate\Database\Query\Builder as callback argument.

For example:

>>> (new User)->newQuery()->tap(function($builder) { dd(get_class($builder)); });
"Illuminate\Database\Query\Builder"

>>> (new User)->newQuery()->when(true, function($builder) { dd(get_class($builder)); });
"Illuminate\Database\Eloquent\Builder"

So, we can't use one type hint callback for when/tap methods, like (class callback case will illustrate this better):

class SomeBuilderCallback
{
   public function __invoke(Illuminate\Database\Eloquent\Builder $builder)
   {
       // do stuff
   }
}

// In one part of application
User::when($condition, new SomeBuilderCallback);

// In other part
User::tap(new SomeBuilderCallback); // will fail

@ed-fruty ed-fruty changed the title Move "tap" method to BuildsQueries trait. [5.4] Move "tap" method to BuildsQueries trait. Aug 2, 2017
@taylorotwell
Copy link
Copy Markdown
Member

I don't understand what bug this is fixing.

@ed-fruty
Copy link
Copy Markdown
Contributor Author

ed-fruty commented Aug 2, 2017

@taylorotwell , so is it normal behavior when i get different builder instances in the similar methods ?

$query->when(true, function ($builder) {

   // Instance of Illuminate\Database\Eloquent\Builder

})->tap(function ($builder) {

  // Another builder instance. Illuminate\Database\Query\Builder

});

Why not one builder instance in the above methods?

@ed-fruty
Copy link
Copy Markdown
Contributor Author

ed-fruty commented Aug 2, 2017

@taylorotwell , in addition, with tap method i can't use eloquent builder features, like

Model::tap(function ($builder) {
   /*
    * This will throw an exception
    * BadMethodCallException with message 'Call to undefined method Illuminate\Database\Query\Builder::with()'
    */
   $builer->with('relation');

});

@ed-fruty ed-fruty mentioned this pull request Aug 4, 2017
@taylorotwell taylorotwell reopened this Aug 4, 2017
@taylorotwell taylorotwell merged commit 4ce8d7a into laravel:5.4 Aug 4, 2017
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