This repository was archived by the owner on Jul 10, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Jul 10, 2023. It is now read-only.
9.0.0-beta.4 PHP 7.4 class ordering bug resulting in Fatal Error #140
Description
In 9.0.0-beta.4, in Sober\Controller\Loader::setInstance() there appears to be an issue when using PHP 7.4. The code assumes that the last class returned by get_declared_classes() is the one that should be mapped to the template, however, in PHP 7.4 the final class returned is Sober\Controller\Controller. This works correctly under PHP 7.3.
For example, under PHP 7.3, the results of a var_dump on get_declared_classes() produces:
...
[1738]=>
string(44) "Yoast\WP\SEO\Conditionals\XMLRPC_Conditional"
[1739]=>
string(23) "Sober\Controller\Loader"
[1740]=>
string(27) "Sober\Controller\Controller"
[1741]=>
string(20) "App\TemplateVocation"
Under PHP 7.4 we are seeing:
...
[1906]=>
string(44) "Yoast\WP\SEO\Conditionals\XMLRPC_Conditional"
[1907]=>
string(23) "Sober\Controller\Loader"
[1908]=>
string(16) "App\TemplateNews"
[1909]=>
string(27) "Sober\Controller\Controller"
Since an upgrade to 2.x.x would be fairly significant a suggested quick fix patch to 9.0.0-beta.4 would be:
diff --git a/src/Loader.php b/src/Loader.php
index 11bc8c4..0b13911 100644
--- a/src/Loader.php
+++ b/src/Loader.php
@@ -77,8 +77,12 @@ class Loader
*/
protected function setInstance()
{
- $class = get_declared_classes();
- $class = '\\' . end($class);
+ $classes = get_declared_classes();
+ $class = array_pop($classes);
+ if (strpos($class, "Sober") === 0) {
+ $class = array_pop($classes);
+ }
+ $class = '\\' . $class;
$template = pathinfo($this->instance, PATHINFO_FILENAME);
// Convert camel case to match template
$template = strtolower(preg_replace('/(?<!^)[A-Z]/', '-$0', $template));
I appreciate the 9.0.0-beta code is no longer being worked on but it great if this could be included in a tag to help some of us that are still using it.
Thanks
Activity
LiahMartens commentedon Apr 14, 2021
Was this ever picked up? Running into this issue
totallyben commentedon Apr 14, 2021
I don't believe so. It's a shame as it would be great to have this minor fix in place to allow us to upgrade to php 7.4 without a more significant update.
darrenjacoby commentedon Apr 14, 2021
Hey @LiamMartens and @totallyben , I can look into getting it implemented.
However, if it's an option, I would recommend trying to upgrade to Sage 10 as their implementation of Blade Components and Composers is a step up from Sage 9+Controller. Have you tried upgrading to Controller 2.x.x? That is the more recent version vs 9.0.0-beta. Initially it was versioned to match Sage 9.
totallyben commentedon Apr 15, 2021
Hi @darrenjacoby
Thanks, that would be greatly appreciated! We have quite a few projects using Sage 9 and it's just going to take some time to update them to 10; it's also not always easy to persuade clients into making upgrades to working sites :-)
LiahMartens commentedon Apr 15, 2021
@darrenjacoby I'm in the same boat as @totallyben; don't have the time or resources to go and upgrade the projects.
I made a suggestion PR #142 - uses a similar namespace filter like in the new version so it can filter out which class is relevant.
Let me know what you think
totallyben commentedon May 20, 2021
Hey @darrenjacoby
Just wondering if you were able to look into this?
darrenjacoby commentedon May 27, 2021
Hey all,
Reviewing this, but I would recommend an upgrade to 2.x.x, which solves the issue, as opposed to tagging another beta. Is upgrading not an option at all? It should have little impact on compatibility.