Open kirlat opened 6 years ago
These comments are based on the above recommendation and the prototype changes to the code in https://github.com/alpheios-project/components/commit/f8eeb403e5c258bfe8ca4eade94183795f1a3987
I am in general in favor of the changes to the UI Controller. I think the builder approach makes sense. I'm a little worried about whether we are over-engineering the DOM Events handling though. It seems to me that where we really need events right now are in the interaction between the components, e.g. so that we can decouple the UIController from the LexicalQuery and from the individual Vue Component methods. I understand (I think) why you have abstracted the DOM Events to use the same mechanism, but I'm having a little trouble right now understanding the value we get for that. My biggest concern with the DOM Events right now is making sure that everywhere we depend upon them, including within the Vue Components for interaction with Alpheios UI elements, that we can be flexible enough to handle both desktop and mobile devices (i.e. mouse and non-mouse).
I am in favor of the UIController changes being merged to master as soon as possible, and to use them to try to eliminate the duplicate controller code in the PWA (which would help verify that the design is solid) but would like unit tests for the new EventController and EventEmitter classes added first. I would probably prefer postponing the DOM Event changes until we are a bit further along with the events for inter-component interaction, because I think we might know more then about the api requirements.
As for whether or not to use a 3rd party library - I think I would be fine with using the simple eventemitter3 library right now, if it adds anything over the code @kirlat has already written, because it is a simple clean library with no dependencies. I am not yet convinced of the need for rxJs - I think we will only be able to evaluate that once we begin to use events for intercomponent communication. We should pay close attention to how much custom event handling code we are having to write, and at each step re-evaluate whether a fuller featured solution is needed. For the moment RxJs adds more dependencies than I am willing to take on until we know the cost/benefit is there.
I agree with Bridget, I think we should move step by step as you suggested, @kirlat . And first to test events approach in different environments we have now on some communications. We should check it for the memory load in browsers and nodes, for may be some other aspects. And it would be better to start from some variant with less dependencies (to avoid disunderstood where the source of bugs if they will appear) - it could be our cutom variant or some other light library.
So what if we:
How's that all sound?
Sounds like a very good plan.
I think we have more work to do with events and inter-component communication so will leave this one open for now.
It seems that if we want UI Controller to be modular, one of the solutions is an architecture painted at https://github.com/alpheios-project/documentation/blob/master/development/app-architecture.md#ui-controller-architecture (module role descriptions are still WIP).
It also seems that if UI Controller to operate in different configurations with or without view components, user input event handler and other modules, those modules have to be as loosely coupled as possible. For this, we should avoid calling methods of other modules directly. It looks like an ideal solution would be an event-based architecture, something similar to an Observer pattern.
I've started with decoupling a UI Controller event listener from a UI Controller. It allows to tie certain UI events (i.e. mouse double click) to UI controller method calls. At the same time, it eliminates dependency of a UI Controller on event listener completely. UI Controller can work with any configuration of an event listener or even without it. Moreover, it allows to detach a user input event listener and replace it with some other event listener dynamically.
I think such architecture would be really beneficial for other modules too. However, it seems that for implementing this the same way we have to use some global messaging solutions between components. So I would like to link to the previous discussion of this: https://github.com/alpheios-project/documentation/issues/3#issuecomment-436046927. I think it's better to move discussion here so we can focus on discussing an implementation of such architecture in greater details as avoid growing the original issue too much.
So the question is whether we should use and develop our own solution or use some third-party libraries. Befits and drawbacks of each were already laid out in the discussion linked above.
There are many great libraries, including the one offered by Irina (https://github.com/primus/eventemitter3). On my opinion, if we choose to use an existing library and we want to to be well supported for a long time, an interesting solution can be RxJS (https://rxjs-dev.firebaseapp.com/, GitHub: https://github.com/ReactiveX/rxjs). This is a JavaScript implementation of Rx (http://reactivex.io/), a well known library used by Airbnb, Netflix, GitHub and large companies alike. Except JavaScript, it is implemented in Java, .NET, Scala, Clojure, and even Swift. It does integrate well with Vue.js with the help of https://github.com/vuejs/vue-rx. Here are some examples of such integration: https://egghead.io/courses/build-async-vue-js-apps-with-rxjs (even though it's for version 5 and the current version is 6). It can integrate with native DOM events easily: https://rxjs-dev.firebaseapp.com/guide/overview#flow. It also offers support for functional programming with things like piping which is a good thing. Additional benefit is that it works well with Webpack and is tree shakable.
So, @balmas, @irina060981, what do you think about it? Which way will we go? I think we can make a great step toward an even more modular architecture with this.