Skip to content

[12.x] Fix typehint for VerifyEmail::toMailUsing() #56454

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

Merged

Conversation

xurshudyan
Copy link
Contributor

Currently, the toMailUsing method in the VerifyEmail notification only documents the $callback parameter as a generic \Closure without specifying the expected input parameters or return type.
This PR improves the docblock by adding a return type declaration to the callback, specifying that it should return either an instance of MailMessage or a Mailable.
This detailed typehint helps IDEs and static analysis tools (like PHPStan or Psalm) better understand the expected usage, improving developer experience and code quality.

@taylorotwell taylorotwell merged commit 2bc2b7e into laravel:12.x Jul 27, 2025
62 checks passed
@@ -21,7 +21,7 @@ class VerifyEmail extends Notification
/**
* The callback that should be used to build the mail message.
*
* @var \Closure|null
* @var (\Closure(mixed, string): \Illuminate\Notifications\Messages\MailMessage|\Illuminate\Contracts\Mail\Mailable)|null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that's the correct return type as toMail() only ever returns \Illuminate\Notifications\Messages\MailMessage?

/**
* Build the mail representation of the notification.
*
* @param mixed $notifiable
* @return \Illuminate\Notifications\Messages\MailMessage
*/
public function toMail($notifiable)
{
$verificationUrl = $this->verificationUrl($notifiable);
if (static::$toMailCallback) {
return call_user_func(static::$toMailCallback, $notifiable, $verificationUrl);
}
return $this->buildMailMessage($verificationUrl);
}

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.

3 participants