-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
[5.5] Make Optional@__call really macroable #20898
Conversation
We need to firstly check existence of macro and only then pass it to the value. Otherwise it won't be really macroable (because macros won't work for Optional with value inside).
This is an interesting case. I would think the check should use Though, this way of doing it wouldn't work for objects implementing Do with this what you wish. This is just me putting my two cents in. |
Indeed, but we already have similar Macroable functionality in, for example, |
True but the builder is only delegated to by a model if a method isn't found. Here optional isn't going to define any methods except the ones necessary to delegate to the underlying value. It's mainly about how it's going to be used. ¯\_(ツ)_/¯ |
Tempted to just make it not macroable at this point. |
This reverts commit 979d693.
But you can just move this behavior to your macro: Optional::macro('present', function () {
if (is_object($this->value)) {
return $this->value->present();
}
return new Optional(null);
}); |
You are saying it like you are going to have tons of macros and it is really painful to write 3 lines of code. Do you really think that it is good idea to remove feature just not to write 3 lines of code in your macro? You can checkout similar monads or |
Optional::macro('orElse', function ($value) {
if (is_object($this->value)) {
return $this->value; // nope, I don't want to proxy orElse
}
return value($value);
}); |
@derekmd jfyi 1 year+ passed and there were no PRs. There were no need to be rude. |
We need to firstly check existence of macro and only then proxy
__call
to the value. Otherwise it won't be really macroable (because macros won't work for Optional with value inside).