Skip to content
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

BUGFIX: Accept a string as second parameter of the slot method #43

Closed
wants to merge 2 commits into from

Conversation

daniellienert
Copy link
Member

Fixes: #42

@daniellienert
Copy link
Member Author

@kdambekalns, currently testing against Neos 7. I remember that there have been changes in Signal/Slot information passing. May that be related?

@kdambekalns
Copy link
Member

Hah, this seems to be a prime example of why the "maybe this passes a string" idea was bad. My guess: deactivating the signal information in the connecion is the better fix!

@kdambekalns
Copy link
Member

… which you seem to do in

$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeRemoved', Indexer\NodeIndexingManager::class, 'removeNode', false);
already!?

Also, #42 is weird, the signal information is passed twice?!

@daniellienert
Copy link
Member Author

@kdambekalns, wild guess from looking at it:

there is a loop over the slots in dispatch and signal information is added to the $signalArguments[] array on every loop:
https://github.com/neos/flow-development-collection/blob/master/Neos.Flow/Classes/SignalSlot/Dispatcher.php#L182

As the common nodeAdded signal is used. Might it be that $passSignalInformation is set to true at two other connections to that signal and so they are passed here as well?

@albe
Copy link
Member

albe commented Dec 19, 2020

https://github.com/neos/flow-development-collection/blob/master/Neos.Flow/Classes/SignalSlot/Dispatcher.php#L182

That line is indeed a bug! The $signalArguments array needs to be reset to the initial value passed into the dispatch for every iteration.

Also, this is handled correctly up to 6.3 so it is a regression: https://github.com/neos/flow-development-collection/blob/6.3/Neos.Flow/Classes/SignalSlot/Dispatcher.php#L110

@albe
Copy link
Member

albe commented Dec 19, 2020

Should be fixed with neos/flow-development-collection#2362 so this can then be closed. Sorry that change in behaviour went by unnoticed!

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is wrong. The parameter must be a Workspace (or null), but never something else. So removing the type does not solve anything!

@daniellienert
Copy link
Member Author

Hey @kdambekalns, no worries. Just fixed it here to get the project going. The fix in Flow makes this unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception when deleting a document
3 participants