Skip to content

Conversation

@inxilpro
Copy link
Contributor

This is a continuation of #38800:

  • Removes the custom method DecoratesQueryBuilder::forwardCallToQueryBuilder in favor of forwardDecoratedCallTo
  • Updates NullDispatcher so that fluent calls never accidentally leak the underlying dispatcher
  • Updates DelegatesToResource so that fluent calls return the JsonResource rather than the underlying model instance
  • Updates Mail\Message so that fluent calls return the message object rather than the underlying Swift/Symfony mailer object

@taylorotwell
Copy link
Member

What are the possible breaking changes here?

@inxilpro
Copy link
Contributor Author

What are the possible breaking changes here?

DelegatesToResource

This is the only place where I can imagine an actual breaking change, although I think it's quite unlikely. If someone implements a JsonResource for a model and calls a model method on the resource expecting to get the model back, they'll now get the resource back instead.

For example:

class UserResource extends JsonResource
{
    public function toArray($request)
    {
        // If User::doSomething() returns $this, 8.x would return the underlying User model
        // while 9.x would return the UserResource object
        $user = $this->doSomething();

        $this->functionThatTypeHintsUserModel($user); // This would break
    }
}

That said, I feel like this is mis-using resources, and is incredibly unlikely to ever actually impact anyone. And because resource objects are meant as a transformation layer, method calls that are for anything other than data access are unlikely to be used.

NullDispatcher

In 8.x, this incredibly unlikely scenario would actually dispatch an event because the NullDispatcher's __call method would return the original dispatcher from the setQueueResolver() call. So while it's a breaking change, I think it's really a bug fix:

Model::withoutEvents(function() {
  Model::getEventDispatcher()
    ->setQueueResolver(/* ... */)
    ->dispatch($event);
});

Mail\Message

As for Mail\Message, any breaking changes are overshadowed by the fact that we're moving from SwiftMailer to Symfony Mailer, since the underlying implementation is a completely different package.

DecoratesQueryBuilder

This is not a breaking change. It's just swapping out the custom method for the generic method.

@taylorotwell
Copy link
Member

I really want to minimize as many breaking changes as we can. Maybe we can tweak this to not be breaking?

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