-
Notifications
You must be signed in to change notification settings - Fork 53
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
Dispatch to DispatchSync with non queueable job #182
Dispatch to DispatchSync with non queueable job #182
Conversation
This rule is great 🙌 The return value could also be used in places like |
@GeniJaho I looked at this but it's very difficult to work out all possible contexts of a return value being used The main problem with dispatch returning PendingBatch is that it uses a destructor so in some cases it will be fine. But the worst is when a variable is created so I just focused on that. |
I was thinking, since we're only doing a rename, we could target all use cases by targeting the three nodes directly: public function getNodeTypes(): array
{
return [FuncCall::class, MethoCall::class, StaticCall::class];
} This would also simplify the rule, but I'm not sure if we should touch all cases anyway. |
…to-dispatch_sync-with-non-queueable-job
…b' of github.com:driftingly/rector-laravel into feature/dispatch-to-dispatch_sync-with-non-queueable-job
@GeniJaho docs have been updated 👍 In theory, there's no reason why it can't just be for any call I guess, as I can't see a reason why someone would dispatch an instance that didn't have ShouldQueue implemented. |
@GeniJaho @driftingly I've updated to refactor all calls rather than just assigned ones. |
Thanks @peterfox |
@GeniJaho you okay with the rule in the current state? |
@peterfox I was ok with the rule since 1 week ago 😁 |
Changes
Why
Performs the following:
In Laravel 9 and earlier if you were able to send a non-queue job to dispatch and it would handle it immediately but now it will return a PendingBatch instead of the return value of the job. Even then it doesn't make sense to use dispatch instead of dispatchSync if it's a non queueable job so this should be helpful to a few people.