Why do we need view models?

[Reinmar]

OK, I hate to say that, but I don't remember or I don't know the answer. Either way, it sounds like we overcomplicated this thing...

So, currently we say – the view model (created automatically by each view and used by controllers to push data into the views and get the feedback) is the view's interface. It's how the world communicates with the view.

That's even fine, but:

• It looks like we haven't followed that rule: Why does BalloonPanelView have couple of public methods? ckeditor5-ui-default#70 and I also don't know how to make richer methods (like those attachTo() – I guess its params should be put in the view model...).
• If the view model is the views public API, then what's left in the view itself? What does its public API represent? We've got there methods like init() and destroy() plus couple of other things, but still... why moving the rest outside?

[Reinmar]

OK, I can name one argument for separating the model – clarity of what has to be defined (bound by the controller) in order to render that specific view.

The cons are quite obvious – additional property, additional interfaces, etc.

[pjasiun]

So, currently we say – the view model (created automatically by each view and used by controllers to push data into the views and get the feedback) is the view's interface. It's how the world communicates with the view.

I think, I understand the concept, but I have a strong feeling that it is a class interface what should be... an interface.

OK, I can name one argument for separating the model – clarity of what has to be defined (bound by the controller) in order to render that specific view.

Still, there are simpler ways to define that the interface should contain: base class or simply a documentation. We do not need nested property as a public interface. It looks like the reversed facade pattern ;)

[Reinmar]

Eh... It's so obvious now, but somehow it wasn't when we were introducing VMs. I think that we set correct requirements, but we failed at realising what's the easiest solution.

Anyway, it should be a fairly easy change to do – nothing serious to refactor. Just many automatic changes in the tests and docs.

However, as we're discussing this now – let's take a look at what we'll end up with, to reassure that we're going in the right direction.

So, a controller has a model. It is passed to the controlled by the "world" (so multiple instances can share one model) and that model is binded to the view (currently it's binded to the view model). So a model is component's API, while view defines its API itself.

I know that there are multiple ideas how MVC should be structured, so I hope that this approach is understandable as well. In terms of whether it's going to work for us, we can be pretty sure that it will because it's just a small change comparing to what we have now.

[pjasiun]

It looks like the most classic MVC approach for me, what is nice and easy to understand.

[Reinmar]

One more thing – the result of the current architecture is that our views can contain their runtime state which doesn't have to be exposed in the component model (because it's not a business data of the component).

E.g. a multiple drop-down components can share one model and only their respective views will be able to tell you whether a specific drop-down is opened. In such case other controllers have to access those views to change that state.

That's a bit similar to CSS where you have :hover and can control some things directly on the view layer.

It may, however, be controversial that we divide the state into business and view layers. It is also unclear what should go where. In the drop-down we assumed that "isOpen" goes to the view (and it's unlikely that anyone will need to use those), but how to open a balloon panel (balloon panel is usually used directly by a feature)? Also through the view or should this state be exposed in the model?

[Reinmar]

Also through the view or should this state be exposed in the model?

There's a third option – through a controller's methods which then change the view.

[oskarwrobel]

So, currently we say – the view model (created automatically by each view and used by controllers to push data into the views and get the feedback) is the view's interface. It's how the world communicates with the view.

I didn't know that view model should be the only view interface, I thought it is for storing specific for view, observable data which can be bound to the template. But I didn't know that I couldn't extend view API by adding methods.

So, a controller has a model. It is passed to the controlled by the "world" (so multiple instances can share one model) and that model is binded to the view (currently it's binded to the view model). So a model is component's API, while view defines its API itself.

For me it sounds good, but I don't think that model has to be the only interface.

It may, however, be controversial that we divide the state into business and view layers. It is also unclear what should go where.

It sounds like MVC. If something is only a matter of view it should stay in the view.

(...) how to open a balloon panel (...) through a controller's methods which then change the view?

I was expecting that. It seems to be the easiest way to use components.

const balloonPanel = new BalloonPanel( ... );
balloonPanel.show(); // And I don't have to know component architecture.

[oleq]

There were multiple reason behind separate Controller model and View Model. I'm curious which are still valid:

Interchangeability

The first one was interchangeability, which is that each Controller can work with any compatible View instance without knowing anything about the mechanics of the view.

We wanted to have a such architecture that would allow switching all the views to a different library by a simple injection in the compiler (builder). For instance, as the default library (ckeditor5-ui-default) defines ButtonView, IconView, etc., another library (ckeditor5-ui-bootstrap) defines the same classes (ButtonView, IconView) but with the HTML (Template) structure specific to Bootstrap, and still offering the same VM API. So the whole UI of the editor should simply switch to Bootstrap when all the view files (.js) from ckeditor5-ui-default are physically replaced by those from ckeditor5-ui-bootstrap during compilation.

In fact, it's been a long, long time since we tested such process (and it worked). I wonder if it's still possible.

UI duplication

Another reason for separate VM is reflected in the Dropdown component example. If there's an editor implementation, which has 2 toolbars, and each one duplicates the layout of the other (it's a valid use case), and both Dropdown instances in each toolbar share the same model (let's say a HeadingsModel with #isOpen attribute), and then if the user clicked the Dropdown in the first toolbar, model#isOpen will go true and... the second dropdown will open as well because both DropdownViews are bound to the same model#isOpen attribute in the same model.

So to have 2 components sharing the same application logic (like heading formats) but separate view states, there must be a separate VM for each of the Dropdown instances and such VM is the right place to put the isOpen state attribute.

No magic in component definition

When you define a component, you provide its model, Controller class and View class:

const dropdownModel = new Model( {
    logicAttr: 'foo',
    anotherLogicAttr: 'bar',
} );
const dropdown = new Dropdown( dropdownModel, new DropdownView() );

console.log( dropdownModel );
// logicAttr: 'foo',
// anotherLogicAttr: 'bar',

The DropdownView creates own VM and the Dropdown controller could bind some of them to the dropdownModel if needed, but in general DropdownView#model works with the view layer only and attributes like isOpen belong to DropdownView#model.

Now, if suddenly there was no DropdownView#model, the view would need to operate on dropdownModel. To provide a way to control the DOM, isOpen attribute would land in the dropdownModel

const dropdownModel = new Model( {
    logicAttr: 'foo',
    anotherLogicAttr: 'bar',
} );
const dropdown = new Dropdown( dropdownModel, new DropdownView() );

console.log( dropdownModel );
// logicAttr: 'foo',
// anotherLogicAttr: 'bar',
// isOpen: false <---------------------- Where did that come from, huh? I didn't define it. WTF?

so the developers would face a situation that the models they create are automagically extended by some attributes of unknown origin and purpose. It could be a problem because such model becomes unpredictable and there's a lot of garbage if it's to be serialized, because some attribute like isOpen are not the actual state of the editor.

[oleq]

So, currently we say – the view model (created automatically by each view and used by controllers to push data into the views and get the feedback) is the view's interface. It's how the world communicates with the view.

I didn't know that view model should be the only view interface, I thought it is for storing specific for view, observable data which can be bound to the template. But I didn't know that I couldn't extend view API by adding methods.

Me neither, and I coded it :P It makes sense that Views have their own interfaces IMO, because i.e. BalloonPanelView#attachTo() operates on DOM level only and should not bother BalloonPanel. Or InputTextView#focus() – it's a pure DOM method, it does not belong to InputText.

Please note guys that ATM many Controllers have a very limited role. They mainly extend the VM and bind some of its attributes to the component model. However, in case of more complex components like, let's say Autocompleter, there will be tons of logic in the Controller (async obtaining data, processing etc.) and tons of view logic (displaying, positioning, keyboard navigation, filtering as you type) and both component model and VM will be filled up with their respective states, which will express the idea of the separation much better than what we have now.

[fredck]

The VM could be seen as the container that holds the essential data that drives the View behavior (like a serialization of the View). User interaction on the View is then converted into data changes and whichever change to that data, usually coming from the controller, is reflected in the View.

Views should certainly be able to introduce a custom API with own methods to handle its behavior or even semantically rich shortcuts to manipulate the VM, in a clear View PoV... for example, things like hide() or click().

VMs can have some "dumb" logic, if, for example, a change in one of its attributes results on logic to update other attributes. Nothing more than that though as VMs know nothing about the View nor about the Controller.

[oskarwrobel]

Our view model looks like nothing more as namespace with observable properties. It is not injectable, can't be shared between views, it is just a namespace. Question is do we need namespace for storing properties in the class?

[fredck]

One of the essential features MVC features, in fact, is that a View can receive its Model in the constructor... this is much probably the part that messed things up in our implementation as we don't allow that.

[pjasiun]

Interchangeability

...

I think this could be achieved with the class interface.

UI duplication

....

---

The VM could be seen as the container that holds the essential data that drives the View behavior (like a serialization of the View).

I think that the key question here is: if we need view serialization? In my opinion, it is the model what should be serialized and view should be re-created based on the deserialized model. Then that view-only features link "if the dropdown is open" will not be recreated, only these which are based on the model state: for instance balloon panel might be recreated based on the selection. I do not think that view should be serialised, that anybody expects to have open dropdown after deserialization, but I might not know about some cases we want to handle.

[pjasiun]

Also, as I think about it: we do not create some special property in the model to help with serialization. There is no Model.model. If we want to serialize it we serialize the model class itself. The same way we could serialize view classes with their state if we need it. Base view class could provide proper method and child classes could provide serialize() and deserialize() methods it the class need special treatment. But, as I post before, I do not think view need serialization.

[oleq]

I think this could be achieved with the class interface.

Can you elaborate, @pjasiun ?

As for

In my opinion, it is the model what should be serialized and view should be re-created based on the deserialized model.

when writing

It could be a problem because such model becomes unpredictable and there's a lot of garbage if it's to be serialized

I meant component (Controller) model, not the VM. My message was that if Views share the same model with Controllers, then the model of the controller may get polluted by unexpected stuff created by the View.

And it's the model of the controller which is to be serialised, if any.

[Reinmar]

@oleq pointed out all reasons why we thought that separating VM will be reasonable. However, merging VM into the V won't change anything else than paths and number of interfaces.

Currently, each component instance has a duet of V+VM (they always go together). After the change, each component instance will have an instance of its view.

1. Interchangeability – still the same. By exposing VM we wanted to say "your V must be capable of working with this VM interface". We then defined each VM carefully. However, since methods were added to Vs as well, currently, it's required that also the V itself must have the same API. After the change, we'll only require that V has the same API.
2. UI duplication – exactly the same. There's still component's model which controller binds to its view. You can have multiple views bound to that single model. All that varies between view instances (e.g. their "isOpen" state or URL input value should be kept in the view, because it can't be synced back to the component's model).
3. No magic in component definition – exactly the same. The model passed to a component stays the same.

One of the essential features MVC features, in fact, is that a View can receive its Model in the constructor... this is much probably the part that messed things up in our implementation as we don't allow that.

That couldn't have worked with VMs – we tried that. Such component initialisation was a total mess. And MVC says nothing about how model is connected to the view.

The VM could be seen as the container that holds the essential data that drives the View behavior (like a serialization of the View). User interaction on the View is then converted into data changes and whichever change to that data, usually coming from the controller, is reflected in the View.

That's basically how we see this today, but there's no reason why we have to stick to that model. The discussion here is about simplifying this model, so there's less code and not talk about VM, which is confusing. Of course, I agree that by merging things together we may also introduce some confusion in more complex situations, but I haven't seen yet a model which wouldn't cause any WTFs. So, in this case, I'd prefer simplicity over explicitness, because we don't lose any functionality at the same time.

[oleq]

Alright. I'm for this refactoring, which is merging VMs in to their respective Vs. It should not be a big thing, in fact, it's more about copy and paste than actual coding.

i3?

[pjasiun]

Can you elaborate, @pjasiun ?

Sure. I mean:

mix( View, ObservableMixin );

export default class DropdownView extends View {
    constructor( model, view ) {
        super( model, view );

        this.set( 'isOpen', false );

        this.addCollection( 'content' );
    }
}

Plus we could have a Docs which describe that every DropdownView need to have isOpen property or API class if we need it in the code, then:

export default class DropdownView extends DropdownViewApi

But the isOpen property could be kept in the view.

Then, as @oskarwrobel mention, we could provide nice API in the controller, for other plugins:

export default class Dropdown extends Controller {
    show() {
        this.view.isOpen = true;
    }

    hide() {
        this.view.isOpen = false;
    }
}

[oleq]

Since the scope of the refactoring is known, I decided to move it to iteration 4 because it's likely to take a couple of days to code it and i3 has already lots of pending tasks. Besides, no new components will arrive before i4 so the scope remains the same.