-
-
Notifications
You must be signed in to change notification settings - Fork 239
BUGFIX: Handle null values in node cache helper #5608
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
base: 9.0
Are you sure you want to change the base?
Conversation
…ersions This is hard to get right in a clean way in Fusion, so it’s better if the helper deals with it.
|
IIRC we did that on purpose to be type safe. I'm struggling a bit with soften this, but I also see the point. Question is, if we should allow null values on all node parameter in this helper class? On the other hand, you will not be able to find errors in your cache tag building, when we simply swollow these empty nodes. |
bwaidelich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it's fine to be less strict in Eel helpers. We already do that in other places
|
@dlubitz I compared the old and new helper, and in the past the EDIT: But for consistency we should have a single behaviour. I agree. But also for debugging purposes, neos-debug would of course show the issue, but in general I would prefer some kind of log for these cases like the i18n log, which tells about our mistakes but is not as rude. |
kitsunet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I suggested this, I am always for having EelHelpers be a bit more forgiving as otherwise stringing them together is hindered.
|
But the open question: should I adjust all methods to the same behaviour in this helper? |
|
I think we should adapt all helper methods to allow null. |
This was supported in previous Neos versions and is hard to solve in a clean way in Fusion, so it’s better if the helper deals with it.
Example from neos.io: