PHP 8: Fatal error when using Hybrid Template Manager

[brettsmason]

I've just come to use Hybrid Template Manager on a PHP 8 site and am receiving this fatal error:

Call to undefined method ReflectionUnionType::getName() in /home/poshsheds/gitrepo-beta/vendor/themehybrid/hybrid-core/src/Container/Container.php:385

I'm assuming this is related to #180 in some way. Posted here rather than that repo as assumed by the error it was a Hybrid Core package issue.

[saas786]

@brettsmason

I thought we have resolved this issue.

Will take a look ASAP.

[justintadlock]

Notes here are probably relevant: https://www.php.net/manual/en/class.reflectiontype.php

[justintadlock]

$dependency in the code is a ReflectionParameter instance: https://www.php.net/manual/en/class.reflectionparameter.php

Basically, the goal with that bit of code (where the error is) is to first find out whether the dependency/parameter is a class. Then, check if that class exists before resolving it.

We need to support PHP 7 - 8+.

[justintadlock]

Also, more notes on PHP 8 here: https://www.php.net/manual/en/reflectionparameter.gettype.php

[saas786]

Its alpha version and not tested.

I have taken an initiative and did more than just PHP 8 compatibility. Added additional toolkit to container.

See: laravel container for additional tools added.

https://github.com/themehybrid/hybrid-core/pull/191

So @brettsmason if you can pull this and test out if your existing code works (PHP 7.4+).

Your testing and feedback will make it easier for me to improve it.

[saas786]

Related issue: https://github.com/themehybrid/hybrid-core/issues/190

[justintadlock]

Can we separate PHP 8 compat code from a possible overhaul of the container? One is a v.6.x fix and the other is a v.7.0 feature if we go that direction.

[saas786]

Can we separate PHP 8 compat code from a possible overhaul of the container? One is a v.6.x fix and the other is a v.7.0 feature if we go that direction.

Certainly, but PHP 8 compat code is major part of this PR. It no longer only just check for Reflection class etc, code also checks for contextual binding etc. I tried to remove the contextual binding code to keep it minimal, but there was a potential to break the logic, so rolled back the changes. Because required some monkey patching with protentional to break.

We can drop:

• tag, tagged & RewindableGenerator class
• flush
• scoped, scopedIf & forgetScopedInstances
• rebinding
• refresh
• call, callClass, wrap & BoundMethod class
• forgetInstance, forgetInstances, getInstance & setInstance

I think we can do v6.1, v7 is not on my scope, because most of dev's are still using v5 and only few v6, so v7 might cause some more friction.

Its not a fix, but more of a compatibility update, so can land in v6.1.

[justintadlock]

@brettsmason - Can you give this branch a try? https://github.com/themehybrid/hybrid-core/tree/php-8-container

[brettsmason]

@justintadlock Really sorry for the delay.

I've finally given that branch a proper test, and can confirm it fixes my issues with PHP8/8.1 in both V5 and V6 of Hybrid Core. Would be be possible to get a point version released for both? This will fix headaches on dozens of sites I have at the moment 😄

[justintadlock]

@brettsmason - I think we should be able to do that.

[saas786]

@justintadlock Really sorry for the delay.

I've finally given that branch a proper test, and can confirm it fixes my issues with PHP8/8.1 in both V5 and V6 of Hybrid Core. Would be be possible to get a point version released for both? This will fix headaches on dozens of sites I have at the moment 😄

Sure @brettsmason I will file PR against v5 too. Thanks for testing, much appreciated. Note: Apologies, I thought you tested this: https://github.com/themehybrid/hybrid-core/pull/191

[saas786]

@justintadlock

I think its safe to close now, since:

• https://github.com/themehybrid/hybrid-core/releases/tag/5.2.1
• https://github.com/themehybrid/hybrid-core/releases/tag/6.0.1