From 1337aa01071f5ed831429d96f29c3b2d8d2b8816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Sun, 13 Oct 2019 08:33:51 +0200 Subject: [PATCH] [IMP] component: add current key for easier hook creation closes #319 --- doc/component.md | 5 +++ doc/hooks.md | 70 ++++++++++++++++++++++++++++++++++++++ src/Context.ts | 2 +- src/component/component.ts | 4 +-- src/hooks.ts | 12 +++---- tools/playground/app.js | 2 +- 6 files changed, 85 insertions(+), 10 deletions(-) diff --git a/doc/component.md b/doc/component.md index b12c6995..fb5230b3 100644 --- a/doc/component.md +++ b/doc/component.md @@ -162,6 +162,11 @@ constructor. } ``` +There is another static property defined on the `Component` class: `current`. +This property is set to the currently being defined component (in the constructor). +This is the way [hooks](hooks.md) are able to get a reference to the target +component. + ### Methods We explain here all the public methods of the `Component` class. diff --git a/doc/hooks.md b/doc/hooks.md index ff31b0c7..d9cae5ff 100644 --- a/doc/hooks.md +++ b/doc/hooks.md @@ -15,6 +15,7 @@ - [`useContext`](#usecontext) - [`useRef`](#useref) - [`useSubEnv`](#usesubenv) + - [Making customized hooks](#making-customized-hooks) ## Overview @@ -146,6 +147,10 @@ class SomeComponent extends Component { } ``` +Hooks can get a reference to the currently being defined component with the +`Component.current` static property. This is why they need to be called in the +constructor. + ### `useState` The `useState` hook is certainly the most important hooks for Owl components: @@ -277,3 +282,68 @@ The `useSubEnv` takes one argument: an object which contains some key/value that will be added to the parent environment. Note that it will extend, not replace the parent environment. And of course, the parent environment will not be affected. + +### Making customized hooks + +Hooks are a wonderful way to organize the code of a complex component by feature +instead of by lifecycle methods. They are like mixins, except that they can be +easily composed together. + +But, like every good things in life, hooks should be used with moderation. They are +not the solution to every problem. + +- they may be overkill: if your component needs to perform some action specific + to him (so, the specific code does not need to be shared), there is nothing + wrong with a simple class method: + + ```js + // maybe overkill + class A extends Component { + constructor(...args) { + super(...args); + useMySpecificHook() + } + } + + // ok + class B extends Component { + constructor(...args) { + super(...args); + this.performSpecificTask() + } + } + ``` + + Note that the second solution is easier to extend in sub components. + +- they may be harder to test: if a customized hook inject some external side + effect dependency, then it is harder to test without doing some non obvious + manipulation. For example, assume that we want to give a reference to a + router in a `useRouter` hook. We could do this: + + ```js + const router = new Router(...); + + function useRouter() { + return router; + } + ``` + + As you can see, this does not *hook* into the internal of the component. It + simply returns a global object, which is difficult to mock. + + A better way would be to do something like this: get the reference from the + environment. + + ```js + function useRouter() { + return Component.current.env.router; + } + ``` + + This means that we give control to the application developer to create the + router, which is good, so they can set it up, subclass it, ... And then, to + test our components, we can just add a mock router in the environment. + +Note: the code above makes use of the `Component.current` property. This is the +way hooks are able to get a reference to the component currently being created. \ No newline at end of file diff --git a/src/Context.ts b/src/Context.ts index 7bfe373d..9279c206 100644 --- a/src/Context.ts +++ b/src/Context.ts @@ -59,7 +59,7 @@ export class Context extends EventBus { * to context state changes. The `useContext` method returns the context state */ export function useContext(ctx: Context): any { - const component: Component = Component._current; + const component: Component = Component.current!; const __owl__ = component.__owl__; const id = __owl__.id; const mapping = ctx.mapping; diff --git a/src/component/component.ts b/src/component/component.ts index f45654b1..013fa5fb 100644 --- a/src/component/component.ts +++ b/src/component/component.ts @@ -101,7 +101,7 @@ export class Component { readonly __owl__: Internal; static template?: string | null = null; static _template?: string | null = null; - static _current?: any | null = null; + static current: Component | null = null; static components = {}; static props?: any; static defaultProps?: any; @@ -142,7 +142,7 @@ export class Component { */ constructor(parent: Component | T, props?: Props) { const defaultProps = (this.constructor).defaultProps; - Component._current = this; + Component.current = this; if (defaultProps) { props = this.__applyDefaultProps(props, defaultProps); } diff --git a/src/hooks.ts b/src/hooks.ts index f1ca40ca..b8145e19 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -22,7 +22,7 @@ import { Observer } from "./core/observer"; * trigger a rerendering of the current component. */ export function useState(state: T): T { - const component: Component = Component._current; + const component: Component = Component.current!; const __owl__ = component.__owl__; if (!__owl__.observer) { __owl__.observer = new Observer(); @@ -37,7 +37,7 @@ export function useState(state: T): T { function makeLifecycleHook(method: string, reverse: boolean = false) { if (reverse) { return function(cb) { - const component: Component = Component._current; + const component: Component = Component.current!; if (component.__owl__[method]) { const current = component.__owl__[method]; component.__owl__[method] = function() { @@ -50,7 +50,7 @@ function makeLifecycleHook(method: string, reverse: boolean = false) { }; } else { return function(cb) { - const component: Component = Component._current; + const component: Component = Component.current!; if (component.__owl__[method]) { const current = component.__owl__[method]; component.__owl__[method] = function() { @@ -66,7 +66,7 @@ function makeLifecycleHook(method: string, reverse: boolean = false) { function makeAsyncHook(method: string) { return function(cb) { - const component: Component = Component._current; + const component: Component = Component.current!; if (component.__owl__[method]) { const current = component.__owl__[method]; component.__owl__[method] = function(...args) { @@ -100,7 +100,7 @@ interface Ref { } export function useRef(name: string): Ref { - const __owl__ = Component._current.__owl__; + const __owl__ = Component.current!.__owl__; return { get el(): HTMLElement | null { const val = __owl__.refs && __owl__.refs[name]; @@ -123,6 +123,6 @@ export function useRef(name: string): Ref { * constructor method. */ export function useSubEnv(nextEnv) { - const component = Component._current; + const component = Component.current!; component.env = Object.assign(Object.create(component.env), nextEnv); } diff --git a/tools/playground/app.js b/tools/playground/app.js index 857d578a..100a28c5 100644 --- a/tools/playground/app.js +++ b/tools/playground/app.js @@ -179,7 +179,7 @@ function deleteLocalSample() { function useSamples() { const samples = loadSamples(); - const component = owl.Component._current; + const component = owl.Component.current; let interval; onMounted(() => {