From 8b6ce52cd73d64a8d5de1dd7b313048c490cc802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Sun, 27 Jan 2019 13:31:02 +0100 Subject: [PATCH] update readme.md to add design/architeture notes --- README.md | 116 ++++++++++++++++++++++++++- web/static/src/ts/main.ts | 2 +- web/static/src/ts/registry.ts | 21 +++-- web/static/src/ts/services/router.ts | 10 ++- 4 files changed, 131 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 3ae1139e..3bf40b4d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # Core Utility for Odoo Web Client -This is a POC, not at all production ready code!!! +This is a POC, not at all production ready code!!! This application is a proof of concept of how the Odoo web client could be +redesigned. ## Installation @@ -35,11 +36,120 @@ npm run dev This will: build the app (without minification), then start a live-server with hot-reloading, and watch the file system to make sure that assets are properly rebuilt if necessary, and then reloaded. -## Notes +## Design goals: + +- _declarative UI code_: we want to be able to declare the UI state in a template + and trust the framework to do the job as expected +- _possibility to have 'imperative' ui code_: Odoo comes from an imperative stand + point, and it will be easier to migrate to this new design if it is still + possible to create and mount subwidgets manually. Also, for performance + reasons, it makes it possible to 'drop down' an abstraction layer if necessary + (thinking about large one2many lists for example) +- _minimalist_: less code is easier to maintain. Also, we do not want to depend + on external libraries too much (so, for example, core code does not use jquery + or underscore) +- _simple to reason_: if we need to choose between the ultimate design and a + simpler design, but which requires a few more lines of code, we will go for + the second option. For example, I think that 'observables values' could be + better than our simple EventBus. But this would introduce more concepts that + will be needed to be understood by our ecosystem. We want as few concepts/communications primitives as possible for our needs. +- _testable_: each part of the code should be easy to test. For example, the + action manager takes in its constructor parameters which implements the + IRouter and IAjax interfaces. This makes it easy to test. +- _typesafe_: the current implementation uses Typescript. I believe that using + well defined interfaces will make our code + + - safer: no more _undefined_ is not a function + - simpler/faster: once we are aware of the exact structure of what we are + dealing with, we can make simpler code. + +## Architecture notes + +This POC revolves around two main ideas: + +- an Environment +- a generic Widget class + +**Environment**: The environment is an object which gives access to many +important global partsof the application. For example, the rendering engine +QWeb, translation functions, router, services, ... The environment is supposed +to be unique (otherwise, some strange things could happen if there are two +different active routers for example), accessible to every widget (via its +parent). Also, even though it will probably happen, it is not supposed to be +imported directly by a piece of code. If some code needs access to the +environment, it should be given to him. This makes our code easier to test, +and more generic (could be reused in a different environment). + +**Widget**: Widgets are UI building blocks. In this POC, we want to experiment +with a different Widget API which has the following properties: + +- async hook: the willStart hook is asynchronous. We like that +- composability: we want to be able to create subwidget in a simple declarative + way +- async rendering: because of the two previous properties, the rendering + operation of a widget (not of QWeb) is asynchronous. This introduces a new + class of challenges. +- mounted/willUnmount hooks: they really are missing from current Odoo Widgets +- uses a virtual dom: not really a goal, but it seems like this is necessary. + Because of this, we had to reimplement QWeb to make it output a vnode + representation of a widget. +- has a reference to the application environment: it is given to the root widget + and propagated to each sub widgets. This is the key to make this component + generic and useful. + +We have 3 main folders and 3 main files: + +- _core/_: this contains main building blocks for the rest of the application. + For example, Widget. Everything in _core/_ should be generic and independant + of any other files. This is why the Widget class is not aware of anything + specific to Odoo: it does not know about the action manager (via its + environment). There are two reasons for this restriction: + + 1. this prevents code coupling. If no code in core depends on something + external, this means that the abstraction is self sufficient + 2. this makes it easier to reason. One can read the code from core and + understand it. Then the rest of Odoo builds on top of these abstractions. + 3. this forces us to have a better design. This is not easy to prove, but I + think that generic code forces us to think more about its API. + +- _services_/: here, we put classes that will be available in the application + environment. For example, the router, the action manager, the crash manager, + the notification system, and so on. Code here can import core files, the Env + type, or code from other services. However, no code from widgets/ should be + necessary. + +- _widgets_/: this contains every widgets required by the Odoo web client. Code + in this folder can import the Env type from the main env file, can import + code from ../core/, can import other widgets, but should not import anything from the service/ folder. Also, it should not import any registry. + +- _env.ts_: the global odoo environment. In this file, we define two different + things: the Env interface (which can be imported by pretty much everything + else), and the makeEnvironment function (which should only be imported by the + main bootstrap file). The Env interface is a description of what each odoo + widget will be able to access. Also, it will be accessible by every widgets. + This will basically remove the need for trigger_up. + +* _registry.ts_: this is the main registry where actions, fields, ... should be + added. In a more normal application, this should not be necessary, but Odoo + code is designed to be extended from the outside. Note that the registry is + not a (direct) part of the environment. This is a conscious decision: if it + was part of the application environment, external code that would want to add + something to the registry would be forced to import the main environment. This + would encourage code to mix 'declaration code', such as defining a widget and + 'control code'. Also, this would be slightly more awkward for testing. And + finally, a stronger argument is that the environment will most likely be + started in an asynchronous way, so it does seems weird to wait for it before + adding something to the registry. + +* _main.ts_: the main bootstrap file. This is the part that takes everything + else, and make sure they are properly connected. Then, it make sure that the + root widget is mounted to its desired location. + +## Random Notes Before even thinking about using this in a real scenario: -- check qweb tests and see if it is reasonable +- check qweb tests and see if it is reasonable (escaping? safety/security?) - Note: the compilation of a template should have a unique node (but sub templates can have multiple roots) - remove the "if (${exprID} || ${exprID} === 0) {" diff --git a/web/static/src/ts/main.ts b/web/static/src/ts/main.ts index ad8e9bdf..a004f1e4 100644 --- a/web/static/src/ts/main.ts +++ b/web/static/src/ts/main.ts @@ -8,7 +8,7 @@ import { RootWidget } from "./widgets/root_widget"; //------------------------------------------------------------------------------ // Prepare application registry //------------------------------------------------------------------------------ -registry.addAction("discuss", Discuss); +registry.add("action", "discuss", Discuss); //------------------------------------------------------------------------------ // Application bootstrapping diff --git a/web/static/src/ts/registry.ts b/web/static/src/ts/registry.ts index a81a2ea4..e2cc42b3 100644 --- a/web/static/src/ts/registry.ts +++ b/web/static/src/ts/registry.ts @@ -14,23 +14,20 @@ type ActionWidget = Type>; // Registry code //------------------------------------------------------------------------------ export class Registry { - actions: { [key: string]: ActionWidget } = {}; + registries: { [key: string]: { [key: string]: any } } = { + action: {} + }; - addAction(name: string, action: ActionWidget): Registry { - return this.addToRegistry(this.actions, name, action); - } - - private addToRegistry( - map: { [k: string]: T }, - name: string, - elem: T - ): Registry { - if (name in map) { + add(type: "action", name: string, action: ActionWidget): Registry { + if (name in this.registries[type]) { throw new Error(`Key ${name} already exists!`); } - map[name] = elem; + this.registries[type][name] = action; return this; } } +//------------------------------------------------------------------------------ +// Main registry instance +//------------------------------------------------------------------------------ export const registry = new Registry(); diff --git a/web/static/src/ts/services/router.ts b/web/static/src/ts/services/router.ts index 9f554cca..631d64d7 100644 --- a/web/static/src/ts/services/router.ts +++ b/web/static/src/ts/services/router.ts @@ -1,5 +1,8 @@ import { EventBus, Callback } from "../core/event_bus"; +//------------------------------------------------------------------------------ +// Types and helpers +//------------------------------------------------------------------------------ export type Query = { [key: string]: string }; function clearSlashes(s: string): string { @@ -9,12 +12,15 @@ function clearSlashes(s: string): string { type RouterEvent = "query_changed"; export interface IRouter { - navigate(query: Query); - on(event: RouterEvent, owner: any, callback: Callback); + navigate(query: Query): void; + on(event: RouterEvent, owner: any, callback: Callback): void; getQuery(): Query; formatURL(path: string, query: Query): string; } +//------------------------------------------------------------------------------ +// Router +//------------------------------------------------------------------------------ export class Router extends EventBus implements IRouter { currentQuery: Query;