TASK: Adjust types of Neos' `FusionView::assign`

neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.

https://www.neos.io/

GNU General Public License v3.0

260 stars 221 forks source link

TASK: Adjust types of Neos' `FusionView::assign` #5012

Closed gjwnc closed 2 months ago

gjwnc commented 2 months ago

Upgrade instructions

With this commit the interface of FusionView changed. Thus, currently the Neos 9.0 rendering breaks with latest flow 9.0.

@mhsdesign Would you be so kind to merge this?

Checklist

[ ] Code follows the PSR-2 coding style
[ ] Tests have been created, run and adjusted as needed
[ ] The PR is created against the lowest maintained branch
[ ] Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
[ ] Reviewer - The first section explains the change briefly for change-logs
[ ] Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

mhsdesign commented 2 months ago

öhm yes sure i will take care of this. But interestingly i dont need this fix on my mashine, the demo works locally just fine?

What is your php version? (im on 8.2.10)

gjwnc commented 2 months ago

I'm using 8.2.4 in my docker container and the error message in the php error log was

[25-Apr-2024 13:25:45 Europe/Vienna] PHP Fatal error:  Declaration of Neos\Neos\View\FusionView_Original::assign($key, $value): Neos\Flow\Mvc\View\AbstractView must be compatible with Neos\Flow\Mvc\View\AbstractView_Original::assign(string $key, mixed $value): Neos\Cache\Backend\SimpleFileBackend in /application/Data/Temporary/Development/Cache/Code/Flow_Object_Classes/Neos_Neos_View_FusionView.php on line 232

gjwnc commented 2 months ago

öhm yes sure i will take care of this. But interestingly i dont need this fix on my mashine, the demo works locally just fine?

Unrelated to this issue, I also have the following error, when creating a new node in demo.

Exception in line 168 of /application/Data/Temporary/Development/Cache/Code/Flow_Object_Classes/Neos_Neos_Ui_Domain_Model_Changes_AbstractCreate.php: Unknown named parameter $elementValues

56 Neos\Neos\Ui\Domain\Service\NodePropertyConversionService_Original::convertNodeCreationElements()
55 Neos\Neos\Ui\Domain\Model\Changes\AbstractCreate_Original::createNode()
54 Neos\Neos\Ui\Domain\Model\Changes\Create_Original::apply()

But I didn't report this yet, because I don't understand why. The related line seems fine, also see.

mhsdesign commented 2 months ago

About this https://github.com/neos/neos-development-collection/pull/5012#issuecomment-2077289978 it seems flow is not up to date as i guess it creates a proxy class for the dto (spoiler it shouldnt)

And is your ui also on the newest state?

gjwnc commented 2 months ago

About this #5012 (comment) it seems flow is not up to date as i guess it creates a proxy class for the dto (spoiler it shouldnt)

And is your ui also on the newest state?

I did a composer update today and it shows that I'm using the current latest commit from flow

neos/flow-development-collection   9.0.x-dev 00fa8ff

and also latest from neos-ui

neos/neos-ui                       9.0.x-dev ae55434

I also deleted all DB Tables and recreated demo with

./flow doctrine:migrate
./flow cr:setup
./flow site:create neosdemo Neos.Demo Neos.Demo:Document.Homepage

So, the code itself seems current latest 9.0

gjwnc commented 2 months ago

About this #5012 (comment) it seems flow is not up to date as i guess it creates a proxy class for the dto (spoiler it shouldnt)

Hmm, you're right. Flow creates proxy classes for these. But I don't see why these should not be part of proxy class building?
Anyway, this is unrelated. I'll try to investigate tomorrow and create a new issue/pr if needed. Thanks for your insights!