Add PHP 8.0 support #10
Conversation
| @@ -376,6 +381,7 @@ public function testCanDisplayListOfCurrentMessagesCustomisedByConfigSeparator() | |||
| $helperPluginManager = $services->get('ViewHelperManager'); | |||
| $helper = $helperPluginManager->get('flashmessenger'); | |||
|
|
|||
| $helper->setView(new PhpRenderer()); | |||
There was a problem hiding this comment.
Resolves
TypeError: method_exists(): Argument #1 ($object_or_class) must be of type object|string, null given
/usr/src/myapp/vendor/laminas/laminas-view/src/Helper/FlashMessenger.php:344
/usr/src/myapp/vendor/laminas/laminas-view/src/Helper/FlashMessenger.php:165
/usr/src/myapp/vendor/laminas/laminas-view/src/Helper/FlashMessenger.php:128
/usr/src/myapp/test/View/Helper/FlashMessengerTest.php:384
Please advise if another solution is preferred.
There was a problem hiding this comment.
@bfoosness
Please check #11
There was a problem hiding this comment.
I updated to reflect a similar change to https://github.com/laminas/laminas-view/blob/3.0.x/src/Helper/FlashMessenger.php#L344
Currently tests are failing when we use the lowest dependencies for PHP 8 (https://github.com/laminas/laminas-mvc-plugin-flashmessenger/pull/10/checks?check_run_id=2313255852) due to the same line above at version 2.10 of laminas-view: https://github.com/laminas/laminas-view/blob/2.10.0/src/Helper/FlashMessenger.php#L344
There was a problem hiding this comment.
Is the solution to bump the minimum version of laminas-view to 2.12, which is when it was fixed? https://github.com/laminas/laminas-view/blob/2.12.0/src/Helper/FlashMessenger.php#L344
There was a problem hiding this comment.
Why does this class live in both this repository and laminas-view?
There was a problem hiding this comment.
BC reasons. It will be removed when next major is released
There was a problem hiding this comment.
Yes, bump the laminas-view but also open issue about FlashMessager helper from laminas-view being preferred over the one provided by this package. This is either a test setup issue or a legit problem that needs to be investigated for a potential fix.
There was a problem hiding this comment.
I thought as much. Unfortunately it looks like laminas-view 2.12 requires PHP 8, so by making that the minimum we'd be forcing PHP 8 to use this package?
There was a problem hiding this comment.
Never mind; it allows ^7.3, too.
There was a problem hiding this comment.
It looks like laminas/laminas-view#16 already exists. Does that cover us here?
|
can it be merge ? |
laminas-json has just been updated (which is an indirect dependency of this module via laminas-view), so I believe this is now ready. |
|
@bfoosness |
Signed-off-by: Brent Foosness <[email protected]>
Signed-off-by: Brent Foosness <[email protected]>
|
I also updated all Composer dependencies to latest so the new CI check against the lock file would pass. |
Signed-off-by: Brent Foosness <[email protected]>
…test. Signed-off-by: Brent Foosness <[email protected]>
Description
Close #9