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

"internal" node properties in Fusion/UI #4208

Open
kitsunet opened this issue Apr 23, 2023 · 12 comments · May be fixed by #5015
Open

"internal" node properties in Fusion/UI #4208

kitsunet opened this issue Apr 23, 2023 · 12 comments · May be fixed by #5015
Assignees

Comments

@kitsunet
Copy link
Member

kitsunet commented Apr 23, 2023

Currently there is this in Neos.Neos/NodeTypes/Mixin/Node.yaml

  properties:
    #TODO: IS IT BREAKING TO DISABLE THESE NODE PROPERTIES BELOW WITH ES CR?
    #_creationDateTime:
    #  type: DateTime
    #_lastModificationDateTime:
    #  type: DateTime
    #_lastPublicationDateTime:
    #  type: DateTime
    #_path:
    #  type: string
    #_name:
    #  type: string

And yes unsurprisingly this is breaking in multiple ways. These are assumed ot be there at least in the NodeInfoView but probably in other places, also userland code might use those ala q(node).property('_path') which is perfectly "legal" at the moment. So we need to adapt internal usage and create migrations for userland code if we want to get rid of those (as well as create an accessor for them I guess?)

@kitsunet kitsunet self-assigned this Apr 23, 2023
@kitsunet
Copy link
Member Author

This is related to #3434 as that is using them. However I have a fix prepared for that.

@mhsdesign
Copy link
Member

mhsdesign commented Jun 2, 2023

in 7.3 for example these are the node properties which can be accessed via the internal property syntax:

properties which are declared in the original NodeInterface:

_accessRestrictions
_accessRoles
_accessible
_autoCreated
_childNodes
_contentObject
_context
_contextPath
_depth
_dimensions
_hidden
_hiddenAfterDateTime
_hiddenBeforeDateTime
_hiddenInIndex
_identifier
_index
_label
_name
_node
_nodeData
_nodeType
_nodeTypeAllowedAsChildNode
_otherNodeVariants
_parent
_parentPath
_path
_primaryChildNode
_properties
_property
_propertyNames
_removed
_visible
_workspace

for completion (unrelated but i just wanted to document it) these are the settable properties a la NodeInterface:

_accessRoles
_contentObject
_hidden
_hiddenAfterDateTime
_hiddenBeforeDateTime
_hiddenInIndex
_index
_name
_nodeType
_property
_removed
_workspace

additionally accessible properties because of the TraversableNodeInterface and Node.

_cacheEntryIdentifier
_contentStreamIdentifier
_creationDateTime
_dimensionSpacePoint
_lastModificationDateTime
_lastPublicationDateTime
_nodeAggregateIdentifier
_nodeName
_nodeTypeName
_numberOfChildNodes
_originDimensionSpacePoint
_root
_tethered

_path and _identifier are still supported in the new 9.0 cr:

https://github.com/neos/neos-development-collection/blob/8d7acb506d87c76b47bf9114dddd696bee15fdab/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/PropertyOperation.php#L93C1-L100

but since we kept the dynamic internal access in 9.0

if ($propertyName[0] === '_') {
    return ObjectAccess::getPropertyPath($element, substr($propertyName, 1));
}

we will also allow in the future some magic wizardry for these (new) 9.0 properties:

_subgraphIdentity
_nodeAggregateId
_originDimensionSpacePoint
_classification
_nodeTypeName
_nodeType
_properties
_nodeName
_timestamps

im against that. Keeping a legacy layer: Ok but i would refrain from allowing new magic accessors.

also there are futher cases like here:

return ObjectAccess::getPropertyPath($element, substr($propertyPath, 1));

@kitsunet
Copy link
Member Author

Suggestion mentioned in daily today is to actually fully abandon these as it will just lead to confusion with people when some still work. Lets rather migrate them and offer alternatives where it makes sense.

@mhsdesign
Copy link
Member

100% agree.

Additionally we should discuss if _hiddenInIndex - which in 8.3 was a magic property and now in 9.0 is a normal but untypical property due to the underscore - should be renamed to hiddenInIndex. Otherwise we could say this is the last core property which requires an underscore.

@dlubitz
Copy link
Contributor

dlubitz commented Jan 3, 2024

Please check also the already created rector migrations:
neos/rector#39

@mhsdesign
Copy link
Member

To avoid confusion, and since we have rector migrations in place, we decided in the weekly that we wanted to drop the legacy layer: https://github.com/neos/neos-development-collection/blob/8d7acb506d87c76b47bf9114dddd696bee15fdab/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/PropertyOperation.php#L93C1-L100 and not further support the underscore _ syntax anymore.

We should especially look at some node types as the underscore _ syntax is there still used for example to define an editor for the _nodeType but that should rather be either hardcoded in the ui or declared elsewhere. I think neos 9 doesnt need the concept of underscores for untypical properties anymore, and they are opened for consumer land to be used freely.

@kitsunet
Copy link
Member Author

Unfortunately we actually added _hidden and _hiddenInIndex into the property declaration and use them as regular properties now, they are just in the json like anything else. Seems a bit big break to change that still?

@dlubitz
Copy link
Contributor

dlubitz commented Jan 19, 2024

What about a NodeMigration to rename the property?

@kitsunet
Copy link
Member Author

But rename it to what? 😆 The thing is ... complicated, I can explain in the weekly... or write it out here later.

@mhsdesign
Copy link
Member

_hidden and _nodeType must go from the nodeType yaml imo.

@bwaidelich
Copy link
Member

As decided in our weekly today, the plan is to:

  • Rename _hiddenInIndex to hiddenInMenu
  • Remove _hidden
  • Replace all remaining underscore properties in the core
  • Remove special handling for underscores in properties

kitsunet added a commit to kitsunet/neos-development-collection that referenced this issue Mar 6, 2024
Provides a helper to access hidden status as well as renaming
`_hiddenInIndex` to `hiddenInMenu`.

Fixes: neos#4208
@mhsdesign
Copy link
Member

Todo: explicit eel ordering support via time stamp fields: https://neos-project.slack.com/archives/C050C8FEK/p1709887830143079?thread_ts=1709403062.357079&cid=C050C8FEK

currently in 9.0-dev its sort('_timestamps.originalCreated')

mhsdesign pushed a commit to kitsunet/neos-development-collection that referenced this issue Apr 26, 2024
Provides a helper to access hidden status as well as renaming
`_hiddenInIndex` to `hiddenInMenu`.

Fixes: neos#4208
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
4 participants