A short proposal since we had a discussion about this in our latest standup.
Currently all editors are bundled, thus, no extensibility point is currently available. We should keep it as is, since extensibility should be our last concern, but still we can discuss some concepts and thoughts, so here we go.
I think this is the first feature which would heavily demand the global Neos API. Since we don't want to force users to bundle up their code with the application code of Neos, we are left with only one solution - <script> tags. But thats not a bad thing actually, this way users are free to use whatever they like as their build process/module system, maybe even vanilla js if they want to do so.
Currently developers describe a nodeType with a custom editor reference like this.
'Namespace.Name:NodeType':
properties:
myProperty:
ui:
inspector:
editor: 'Namespace.Name/Inspector/Editors/MyCustomEditor'
Since we should aim at making the transition from the old UI to the new UI as smooth as possible, I would stick to this and use this path as the basis for resolving the .js bundle to Namespace.Name/Resources/Public/JavaScript/Inspector/Editors/MyCustomEditor.js.
_One thing to note here, we should aim to remove the Inspector/ here, since the editors should also work in other scenarios such as abstract editing - So basically lets make this a convention._
So how would this be implemented? Basically we need some kind of a 'store' in the global API(Note that I don't talk about the redux store here) for property editors, and if the editor identifier isn't loaded/registered already we will create a <script>-tag, append it to the DOM and load the bundle.
Inside the bundle we have a IIFE which is executed as soon as the bundle got loaded, f.e.:
// Editor-Bundle.js
(function () {
// This will be the factory function, it will be handed over the React framework
// as the first argument, and a `context ` object containing f.e. a set of our React components
// which then can be re-used.
//
// At first I was against passing the components throughout the inspectors,
// since they can change as well and this would result in a major release
// each time we change the name of a prop for example.
// But don't passing them would also lead to duplicate code being loaded,
// stylesheets which needed to be distributed & included as well and so on.
//
// So I would agree on this and just pass them, if people prefer to use a version
// distributed over npm, they can easily do so and so on.
function factory(React, context) {
// The factory will return the boostrapped constructor,
// This could be either a React element (or a plain JS function in future).
//
// The constructor will get passed an API object containing f.e. a function like
// `onPropertyValueChanged` which needs to be called by the custom inspector
// each time the value has changed. This function would then internally update
// the redux store.
//
// More functions are definitely needed, this is just an example :-)
return function (api) {
// Do some magic, return React markup, whatever...
};
}
// We can also assign an optional(?) semver version constraint for the required Neos UI version
// in that the property editor can and should be used. This is kind of a fail safe for us
// so we can change our API or components with making sure that the UI
// does not break completely for our users, and at least the developers
// see that they need to change something after an update.
//
// F.e. we could check the required version against the current app's version, and if it is
// out of the constraint, we can render a error message in place of the inspector with a
// error that the given inspector is not suitable for the Neos UI version.
factory.__requiresNeosUi = '^2.1.9';
// Finally, we just register the property editor factory to the editor identifier,
// and our app will bootstrap the factory and store the bootstraped function.
window.neos.registerPropertyEditor('Namespace.Name/Inspector/Editors/MyCustomEditor', factory);
}());
Our application then would include something like this in a abstract editor component:
// Neos Core
window.neos.awaitPropertyEditor('Namespace.Name/Inspector/Editors/MyCustomEditor')
// awaitPropertyEditor will handle the script loading if necessary,
// so it returns a promise.
.then(Constructor => {
// Check if the Constructor is an React element
// * In case it is, render it
// * In case it is a pure function, render a dummy wrapper component and
// create a new instance of the Constructor on the rendered DOM node.
});
One of the things that @grebaldi suggested, was the ability to use one big bundle which registers multiple editors. I think this can be easily achieved if really needed if we add another key/value to the .yaml configuration below the editor identifier like source: 'Namespace.Name/Inspector/Editors/Bundle'which will be loaded if existing.
One last thing to note, I think we should load the inspector editors asynchronously, since we only have control over synchronous code if it is loaded before our app starts, thus, blocking each and every render of the whole UI, which is not cool since our users will think that the UI is slow, especially on bad connections. Remember, our aim should be to incorporate performance and make this a goal, the UI should be rendered as fast as possible.
Also one thing we should keep in mind, with HTTP2 we will benefit from more smaller bundles and less larger composed ones like we do currently, and since the official ES6 module API is also all about async smaller bundles, we should also steer in this direction.
One last note: This async loading in separate bundles does not apply for the default property editors, I think we would be better off if they are part of the application bundle, but registered the same as third party editors to reduce code and have only one way to go really.
wdyt @skurfuerst @dimaip @grebaldi @GerDner @mgoldbeck @aertmann
Hey,
I really like the idea!
I'd only make the part at:
function factory(React, components) {
more extensible to use (React, context), where "components" is inside "context". Just to be able to extend this, whether this is needed in the longer run.
All the best,
Sebastian
Closing this, because we have the registry and manifest architecture in place now.
Most helpful comment
Hey,
I really like the idea!
I'd only make the part at:
function factory(React, components) {more extensible to use
(React, context), where "components" is inside "context". Just to be able to extend this, whether this is needed in the longer run.All the best,
Sebastian