Skip to content

[9.x] fix: bring back old behaviour #40880

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 9, 2022
Merged

[9.x] fix: bring back old behaviour #40880

merged 1 commit into from
Feb 9, 2022

Conversation

ankurk91
Copy link
Contributor

@ankurk91 ankurk91 commented Feb 9, 2022

Hi,

This PR bring back old behavior, since this can be a breaking change for many.

$notifiable->routeNotificationFor('MethodWhichIsNotImplemented', $notification);

// Larval 8.x and before => null
// Larval v9.x => throws UnhandledMatchError

The routeNotificationFor() method should return null if the method is not implemented on Notifiable.

Tip: the match expression does not return null when there is no match.

PS - This PR adds default case to some more match expressions.

@driesvints
Copy link
Member

@ankurk91 would it be possible to go over #39583 and add the other places where this is needed to this PR as well? Seems like there's more of them.

@ankurk91
Copy link
Contributor Author

ankurk91 commented Feb 9, 2022

@driesvints

I can do that, but looks like all of other match expression has a default case in the mentioned PR.
Only src/Illuminate/Notifications/RoutesNotifications.php was missed.

PS. - Added default case to missing places

@crynobone
Copy link
Member

src/Illuminate/Database/Connection.php
src/Illuminate/Redis/RedisManager.php

@ankurk91
Copy link
Contributor Author

ankurk91 commented Feb 9, 2022

@crynobone

Done!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants