-
Notifications
You must be signed in to change notification settings - Fork 11k
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
Unbinding $this of closure is deprecated #29411
Comments
Hmm. We should try to get this fixed before v6.0 is out since PHP 7.4 drops in fall this year. |
@driesvints The option I took for next Carbon release is muting the errors with |
@kylekatarnls yeah I'd opt to actually provide a fix already if we can. |
Here is for the quick fix: #29418 But some testing utils throws other deprecation notices with PHP 7.4 in Travis-CI. I will try to fix them. No fix for Laravel 5.8? |
Using error control symbol
IMO, and from PR #29418 you shouldn't use |
@kylekatarnls I've closed your temp fix pr. Let's try to fix the actual problem before we release 6.0 |
No, we can't make breaking changes here. |
Okay so here's the gist: everything will still work as is except for static macro calls. When you perform a static macro call which is a closure you'll need to make sure it's also marked the closure as static. Only the unbinding of $this for non-static closures is deprecated. The error message above is actually a bit misleading. So basically the following won't work anymore: FooClass::macro('tryStatic', function () {
return 'foo';
});
FooClass::tryStatic(); Because the closure is non-static you can't unbind FooClass::macro('tryStatic', static function () {
return 'foo';
});
FooClass::tryStatic(); Notice the static keyword. The closure is marked as static now and I've sent in a PR here: #29482 |
In Macroable.php:110 we're inside __call, I don't see how we'd get that error there: Maybe the error comes from line 84? That'd make more sense unless I'm missing something big. The way I see it, there are a few options:
The great solution would be to be able to change the context (for static::) without touching the $newthis. However, it seems like they didn't think of that when deprecating unbinding non-static Closures... |
This will be fixed in PHP 7.4.0 RC1. |
#1 /be/vendor/laravel/framework/src/Illuminate/Macroable/Traits/Macroable.php(85): Closure->bindTo() #2 /be/routes/api.php(35): Illuminate\\Http\\Request::__callStatic() ` when visiting /stat(u)s route, related to laravel/framework#29411 @ routes/api.php * revert f7c2753, remove ctor param $startPage since it's hardcoded, the same as the ReplyQueue @ Jobs\Crawler\SubReplyQueue.php * remove outdated comments and invoke getContents() on the body stream @ Tieba\Cralwer\(Thread|Reply|SubReply)Crawler.php @ fe
#1 /be/vendor/laravel/framework/src/Illuminate/Macroable/Traits/Macroable.php(85): Closure->bindTo() #2 /be/routes/api.php(35): Illuminate\\Http\\Request::__callStatic() ` when visiting /stat(u)s route, related to laravel/framework#29411 @ routes/api.php * revert f7c2753, remove ctor param $startPage since it's hardcoded, the same as the ReplyQueue @ Jobs\Crawler\SubReplyQueue.php * remove outdated comments and invoke getContents() on the body stream @ Tieba\Cralwer\(Thread|Reply|SubReply)Crawler.php @ fe
Description:
PHP deprecate closure unbinding, so the following line:
https://github.com/laravel/framework/blob/master/src/Illuminate/Support/Traits/Macroable.php#L110
throws:
Unbinding $this of closure is deprecated
since PHP 7.4.0
As having a similar macro system in Carbon, I'm working on possible alternatives.
Do you plan to support PHP 7.4 and 8? And to still support mixin/macro in those new versions?
The text was updated successfully, but these errors were encountered: