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

!!! TASK: Remove leftovers from magic underscore access for internal node properties #5015

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Apr 26, 2024

We have the _* underscore access syntax to access actual php object fields of the Node. Due to the ESCR rewrite the Node object looks completely different and has way less getters / fields. See #4208 (comment) for a comparison.

The only overlap of fields to the old node are those few:

  • _name
  • _nodeTypeName (unknown as .nodeType was always used)
  • _properties (useless as q(node).property(...) is used)

Keeping the underscore access would expose a lot of new node fields like _contentRepositoryId which has no gain over to node.contentRepositoryId.
Adding a full runtime migration layer for fields of 8.3 like _identifier, _depth, _nodeType and _hiddenAfterDateTime was decided against as there is a rector migration in place.

So to avoid confusion about which magic underscore properties still exist in 9.0 - as rule of thumb - None, and the syntax will be removed from fusion. One can use an underscore now in consumer land but it is never any more special than any other character.

The sort operation previously also handled _creationDateTime, _lastModificationDateTime and _lastPublicationDateTime. For Neos 9.0 the new sortByTimestamp(created|lastModified|originalCreated|originalLastModified) flowquery can be used.

Related: #4921
Resolves: #4208

@mhsdesign
Copy link
Member Author

mhsdesign commented May 4, 2024

one todo is: how to sort by creation date in Neos 9?? Something like node.timestamps.originalCreated
#4208 (comment)

in our weekly from the 26. we discussed that instead of adding a new flowquery or a magic parameter, we probably want to support the old legacy 8.3 syntax _creationDateTime or _lastModificationDateTime or even _lastPublicationDateTime.

The question is if these modifiers should only work for the sort operation or also for property and filter as well as part of a fizzle query [_creationDateTime = 123]. (Fizzle is almost impossible to archive with the optimisation #4712 in place)

@mhsdesign mhsdesign marked this pull request as draft May 4, 2024 11:27
@mhsdesign
Copy link
Member Author

mhsdesign commented May 9, 2024

todos

mhsdesign added a commit that referenced this pull request May 9, 2024
@kitsunet
Copy link
Member

kitsunet commented May 9, 2024

I do not understand why you did this?

@mhsdesign
Copy link
Member Author

mhsdesign commented May 9, 2024

That was what i have been discussing in #4208 or in the meetings i thought but it seems we have talked past each other?

My thesis in short (now part of the main description):

We have the _* underscore access syntax to access actual php object properties of the Node. Due to the ESCR rewrite the Node object looks completely different and has way less getters / properties. These are the new properties:

  • _subgraphIdentity
  • _nodeAggregateId
  • _originDimensionSpacePoint
  • _classification
  • _nodeTypeName
  • _nodeName
  • _timestamps

While these are the ones known from 8.3

The actual important and ones used in 8.3 like _identifier _depth _parent _hiddenAfterDateTime _hiddenBeforeDateTime etc (see #4208 (comment)) will not work and have to migrated with rector.

To avoid confusion about which magic underscore properties still exist in 9.0 a simple way is - it thought we agreed upon this in the meeting from the 19.01 link - to NOT have any underscore property logic and remove any magic from this syntax. Like one can use an underscore now in consumer land but it is never any more special than any other character.

Thats why this pr remove partially implemented legacy support for _identifier as well as _depth as we have rector migrations for this to properly use the respective eel helper or eel property access directly.

There is a little but as discussed in on the 26.04 we concluded to still allow sorting nodes by date via the q().sort() we will likely have to make one exception regarding underscore properties: The sort operation will translate _lastPublicationDateTime and respective things to the 9.0 equivalent logic internally instead of having to introduce another sort operation or a special parameter or another special syntax like .sort(['$$lastModified']) or such.

@kitsunet
Copy link
Member

kitsunet commented May 9, 2024

This is all clear, but

Extracted everything i though this pr was originally meant to be into #5015 and added the things i found missing.

why, why didn't you just add the things missing to my original PR?

@mhsdesign
Copy link
Member Author

Because i thought it would get too complex and the _hiddenInIndex thing is somehow related but a different task and should imo deserve its own pr. (Also i asked you in slack if im allowed to do this :D)

@kitsunet
Copy link
Member

kitsunet commented May 9, 2024

Also i asked you in slack if im allowed to do this :D

Too much going on LOL, seems still a bit weird but 🤷

neos-bot pushed a commit to neos/neos that referenced this pull request May 11, 2024
@mhsdesign mhsdesign force-pushed the task/4208-remove-magic-underscore-access-for-internal-node-properties branch from ef940a0 to 5f8579c Compare July 3, 2024 15:50
@mhsdesign mhsdesign marked this pull request as ready for review July 3, 2024 18:03
We should not need to load the node type for every fetch, only if we want to fetch references
@mhsdesign
Copy link
Member Author

mhsdesign commented Sep 24, 2024

The migration to use the herby introduced Neos.Node.isDisabled(node) helper can be reviewed here: neos/rector#83

This pr is ready for final reviews.

If i get feedback regarding "9.0 proposed getter and documentation" from above i can also add that documentation to the node.

…-underscore-access-for-internal-node-properties
@mhsdesign mhsdesign changed the title !!! TASK: Remove magic underscore access for internal node properties !!! TASK: Remove leftovers from magic underscore access for internal node properties Sep 24, 2024
Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

Looking good! Just a remark regarding the new sort operation

…-underscore-access-for-internal-node-properties
Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

Lovely, thanks!

@bwaidelich bwaidelich merged commit bc4de7f into neos:9.0 Oct 11, 2024
9 checks passed
@mhsdesign mhsdesign deleted the task/4208-remove-magic-underscore-access-for-internal-node-properties branch October 11, 2024 11:53
mhsdesign added a commit to neos/Neos.Demo that referenced this pull request Oct 14, 2024
neos/neos-development-collection#5015

`nodePath` also doesnt accept absolute node paths anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"internal" node properties in Fusion/UI
4 participants