Neos Best Practices Discussion


(Christian Müller) #1

Note this text is originally written by @rolandschuetz just putting it here for review and discussion. It will end up in our documentation finally. This has been discussed at the sprint with a big group of people so it’s pretty much decided on, but at last round of comments is probably a good idea :slight_smile:

At the Neos-sprint in Salzburg the team discussed the best practices we discovered in over the last years. In a friendly and very detailed discussion a lot of patterns were introduced and we decided on which recommendations we want to give. The recommendation we are giving here do not mean that things are wrong if done differently, there are valid reasons to deviate from those guidelines. However with no specific reasons speaking against we strongly recommend to obey to the following rules.

The key words “MUST”, “MUST NOT”, “REQUIRED”, “SHALL”, “SHALL NOT”, “SHOULD”, “SHOULD NOT”, “RECOMMENDED”, “MAY”, and “OPTIONAL” in this document are to be interpreted as described in RFC 2119.

Note: These rules will eventually end up in the Neos-documentation where they will be reevaluated and extended in regular intervals. We will use semantic versioning for these best practises.

NodeTypes

  1. Each NodeType SHOULD be defined in a dedicated yaml-file and the file-name MUST represent the namespace of the contained NodeType/s. This helps finding the definition of a node type and get an understanding of the existing NodeTypes by looking at the configuration folder.
  2. The namespace of the NodeTypes SHOULD be structured and nested. It is RECOMMEND to start with one of the prefixes “Document”, “Content”, “Mixin”, “Collection” or “Constraint”.
  3. Sub-NodeTypes that are bound to a specific parent NodeType SHOULD have a name matching the parent, e.g. “Vendor:Content.Slider” and “Vendor:Content.Slider.Item”
  4. Properties SHOULD only be editable by a single editor – either inline or in the inspector. Editing the same property with different editors like inspector and inline easily causes problems when settings are only slightly off.
  5. Each NodeType property MUST be valid after creating a new node. This can be done by defining the defaultValue or by allowing the property being empty. Especially SelectBoxEditors caused trouble in the past if they neither allowed being empty nor had good defaults.
  6. The Neos.Neos.NodeTypes package SHOULD NOT be used directly. You RECOMMEND to create NodeTypes in the project namespace. You MAY use the Mixins from Neos.Neos.NodeTypes.Mixins instead.
  7. A NodeType SHOULD only inherit from a single non-abstract superType. All other superTypes SHOULD be Mixins, or Constraints. This helps keeping the inheritance chain understandable.
  8. NodeType-files that modify NodeTypes from other Packages MUST have “Override” in the filename.

Fusion

  1. The Root.Fusion in a project MUST not contain anything but includes.
  2. Fusion files SHOULD only contain a single prototype. The folder and filenames MUST reflect the names of the contained prototypes.
  3. The Fusion prototype-namespace SHOULD be structured and nested. It is RECOMMENDED to start with one of the prefixes “Content”, “Document”, “Component”, “Helper” “Presentation” and “Integration”.
  4. Each non-abstract NodeType SHOULD have a matching Fusion prototype. This is the default way of Neos to find a matching Renderer and makes the code easy to understand. We RECOMMEND to abstract code into Fusion prototypes that are not bound to NodeTypes and reuse them across the project.
  5. Fusion prototypes with the prefixes “Content” and “Document” SHOULD implement the rendering of a Content or Document NodeType
  6. Fusion prototypes with the “Component” Prefix SHOULD implement rendering aspects only without fetching data from the ContentRepository.
  7. The rendering of content SHOULD be defined in Fusion-AFX. Fluid-Templates are still supported but no longer considered best-practice.
  8. Fusion-files that modify prototypes from other packages MUST have “Override” in the file- or folder name.
  9. For larger projects the presentainal prototypes SHOULD be separated from Integration. This can be done with seperate packages or folders “Presentation”, “Integration”. Those presentational Fusion prototypes MUST implement rendering aspects only without fetching data from the ContentRepository. You MAY use Sitegeist.Monocle.
  10. Visual differentiations between Documents SHOULD be implemented via separate Document-NodeTypes. Especially the layout property from Neos.Neos:NodeTypes.Page layout property SHOULD NOT be used.

Distributions

  1. Neos projects SHOULD be managed as a single git repository that contains all packages that are specific to this project. This gives you a shared git history for all project-specific code.
  2. The “Packages” folder SHOULD only be controlled by composer. All your own project-specific packages MUST be be stored in special directory like “DistributionPackages” that is referenced as a repository of type path in the main composer file.

Public Packages

  1. We RECOMMEND to separate reusable Mixins from concrete NodeTypes definitions. This makes it easy for users of your package to only use specific parts of your package.
  2. You MAY provide presentational components without a direct cumpling to the NodeTypes. This makes your package even more flexible other developers.

Team Unicorn Meeting | 2018-12-13
(Daniel Lienert) #2

Thanks guys for discussing this and writing it down! Great to have that. I think this document should function as a guideline for beginners, although it mentions some advanced techniques. Maybe some explanations / examples could help to clear things up.
Here are my detailed remarks:

NodeTypes:
1., 2. and 8. handle the naming of prototypes and files. I would bring them closer together. Eventually integrate 8. in 1.

2 Document and Content are root concepts of Neos, “Mixin”, “Collection” or “Constraint” are just terms or part of concepts. Seems a bit mixed and unclear. Shouldn’t we either explain what these files contain or just recommend to use prefixes, that structure files of same purpose?

5 What about required properties? Should we mention the use of the creation dialog / node templates here?

6 “You RECOMMEND”: shouldn’t it be “We RECOMMEND” or “It is RECOMMENDED to”

Fusion:
9 Recommending a third-party package in this basic paper seems odd… Especially as it is the only package mentioned and as it is not explained what that package does and why it belongs to that topic. I would remove that.


(Sebastian Kurfuerst) #3

Hey Christian and Docs Sprinters,

that’s a great initiative! I very much like this; and I’d suggest we put it twice into documentation:

  • in the short form (RFC-style) as you have written above as some kind of cheat sheet
  • a fully-fledged more expanded version which gives in-depth explanations about each of the points.

I have two suggestions, one formal and one content-wise:

Formal: I’d suggest that at every time where we introduce a new name (“Mixin”, “Helper”, “Integration”, …), we add a nested bullet point with a one-sentence explanation of the concept of it.

Content-wise: I’d like to add one point about Node Type Constraints, though I am still a bit unsure how to phrase it:

Node Type Constraints SHOULD only use positive logic, i.e. you should not write “MyNodeType: FALSE”, but instead "MyNodeType: null / ~ ".
I am unsure about your experiences, but in my experience if you use BOTH positive and negative logic for node types; you will soon get a big mess where you “enable the default, exclude a certain type, and add another subtype again” (which is really hard to debug if it is spread across the node type inheritance chain).

Problem is that we in the core are at some places using negative logic; which might be a problem.

WDYT?

All the best,
Sebastian


(Martin Ficzel) #4

Hey Sebastian,

the rules were discussed in Salzburg by a large group. I would try to publish those rules and extend them later. The RFC is for finding kinks where Roland and i made errors transcribing the notes. We can always publish version 1.1 of the best practices once we extend those.

Same goes for the full description of the reasoning behind the rules. I agree this would be nice but we can publish those anytime and it should not stop us from releasing the Rules 1.0.0 once those are ready.

We originally planned to publish the rules as a blogpost and later add them to the new docs once those are ready.


(Roland Schütz) #5

@daniellienert Thanks for the feedback.

To give a bigger picture, this document does not aim to be a beginners guide, for that we will have the new documentation.

This document aims to standardize best practices and to encourage developers of all levels to adopt a standard best-practice code. In the long run, this could lead to more similar code basis across the Neos community. @martoro and I discussed, if we should add examples and Martin made the good point that this document be short and precise – while documentation and tutorials can have a long form.

NodeTypes:

  1. I agree. I moved 8. right below 2.
  2. Ok, see below.
  3. We did not have any decision in the sprint discussion on that topic. What rule would you propose?
  4. Fixed the typo

Fusion:
9. I had the impression that Monocle is the only solution right now and that everyone in the core team likes it - even though many don’t use it because their projects are “too small” for it. Would you want to recommend an alternative solution as well or just remove it?

@sebastian I will add descriptions of the NodeType namespaces below. I think your point on NodeType Constraints isn’t clear enough yet. Also, is this really a best-practice? I think things like the example below could not be done without negative logic:

'Vendor.Site:Document.PageWithoutSubpages':
  superTypes:
    'Neos.Neos:Document': true
  constraints:
    nodeTypes:
      'Neos.Neos:Document': false

@all I also noticed another error, Fusion point 6. should be “Fusion prototypes with the “Component” Prefix SHOULD implement reusable rendering aspects.”, since many people do fetch data e.g. in a MainNavigation component. For larger projects, see point 9.


Recommended NodeType Namespaces:

I think the Fusion namespaces are explained sufficiently.

Let me know your thoughts,
Roland


(Dmitri Pisarev) #6

Really awesome work, guys!

Just to add, I think we should move https://github.com/code-q-web-factory/Neos-Skeleton under Neos account and link to relevant parts of it from each point of these guidelines. People usually learn best by example.


(Sebastian Kurfuerst) #7

Hey @rolandschuetz,

you can just do the following:

  superTypes:
    'Neos.Neos:Document': true
  constraints:
    nodeTypes:
      'Neos.Neos:Document': ~

(Daniel Lienert) #8

I’m pretty sure, Monocle is a great piece of software! But it servers a meta task of building living style guides and it is only one way of doing this. It is not necessary for configuring node types or build fusion code, also for big projects. So I would not mention that package without further context.
The downside I see personally, is that the @styleguide of Monocle definitions vastly bloat your Fusion code. So I don’t think it is a recommendation per se, as long as you are not absolutely sure what you are doing and what it is needed for.


(Martin Ficzel) #10

I think we should remove the explicit name and mention that third party packages exist to render presentational components as a styleguide. Indeed it would be weird to mention a single third party package in the standard-definition.

In the same manner we can mention that third party packages exist that make prefilling NodeTypes with valid properties during creation easier without mentioning Flowpack.NodeTemplates.


(Bastian Waidelich) #11

As much as I love Monocle, I agree that a Best Practice Guide referencing 3rd party packages is somewhat odd.
Hopefully we can integrate the functionality of the NodeTemplates (and maybe the CreationDialogProperties and/or even Monocle to the core distribution at some point and extend the guide then)


(Roland Schütz) #12

Ok, Monocle is removed.

@sebastian Ahhh… now I understand your post. Makes sense. Not sure if we have a consensus on that though…


(Bastian Waidelich) #13

@sebastian I have to admit: I wouldn’t know the difference (effective) between

  constraints:
    nodeTypes:
      'Neos.Neos:Document': ~

and

  constraints:
    nodeTypes:
      'Neos.Neos:Document': false

Can you quickly elaborate?

But apart from that I think we should state that:
Reverting a super type or constraint configuration in a sub type might be a sign of poor abstraction so it SHOULD be avoided in general.


(Martin Ficzel) #14

Not sure i would not rule out removing constraints and i still think the wording is not that clear yet.

We should to a best-practice session at least once a year on one of the big sprints. It worked very well in salzburg and i doubt that this really works asynchronously. Maybe next time we should collect ideas a bit beforehand. After such session we can always publish a new version and raise the number so there is no reason to rush in additional points now.

I suggest focus strictly on getting the kinks out of the initial version.


(Bastian Waidelich) #15

You’re right, we could also just insert that image:

:slight_smile:

Sure, I didn’t mean to slow down the initial version of this in any way!