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

DISCUSSION: Should property: null be ignored? #41

Open
mhsdesign opened this issue May 25, 2023 · 3 comments
Open

DISCUSSION: Should property: null be ignored? #41

mhsdesign opened this issue May 25, 2023 · 3 comments

Comments

@mhsdesign
Copy link
Contributor

When building a more complex template and a loop, you might encounter the situation, when you only want to set this property on iteration 1 or something and otherwise return null, thinking it will be ignored.

Currently the behaviour with null might not be 100% obvious.

template:
  properties:
    someSimplePropertyWithDefaultValue: null
    someReferencePropery: null
    someReferencesProperty: [null]

in the first case someSimplePropertyWithDefaultValue will be set to null (see $node->getProperties()) (also see somewhat related discussion about properties without default value or not previous value beeing skipped: neos/neos-development-collection#4292)

in the latter cases with the reference properties, the null value will be (rightfully) ignored, but because the setProperty will guard this:

https://github.com/neos/neos-development-collection/blob/1b744c8a6959a4aae735ffc6dd360f33847c98aa/Neos.ContentRepository/Classes/Domain/Model/AbstractNodeData.php#L167-L188C1

the question i asked myself, if it would be generally expected, that when null is returned, that the setProperty operation will be skipped for the property.

This would be helpful for cases, where you dont want to hardcode the default value. But this change might be breaking in unexpected ways...

@mhsdesign
Copy link
Contributor Author

mhsdesign commented Jun 5, 2023

will be solved via: #53

Properties with default values cannot be set to null anymore. This would cause the property to be unset and thus might cause unintended behaviour when null is not handled in the Fusion Rendering.
We know this is a bolder move, but in our test it actually prevented us from writing malformed templates. If you disagree feel free to start and discussion in the issues and explain us your use case.

@mhsdesign
Copy link
Contributor Author

mhsdesign commented Jun 8, 2023

Currently the discussion is to "not try to be smarter than the system" (the system being the neos cr)

We are thinking about introducing a required: true field next to type neos/neos-development-collection#4304, which will work like a not empty validator, except its validated on server side. This feature will make my odd approach in #54 useless and resolve this discussion, as one should just declare properties to be required instead and it will be enforced by the CR.

@mhsdesign
Copy link
Contributor Author

Neos 9 has a bug which initially omits the null values but an explicit set properties will still work: neos/neos-development-collection#5154

As talked with @kitsunet we are unsure what the correct behaviour should be :D

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

No branches or pull requests

1 participant