From 022b29b6a0b4e9eb600d38a2e7900f383a05aa8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 3 Oct 2019 10:48:26 +0200 Subject: [PATCH] [IMP] component/qweb: remove t-mounted directive The t-mounted directive main goals can be achieved with hooks, in a better and more intuitive way. closes #308 --- doc/component.md | 19 --------- doc/qweb.md | 1 - src/component/component.ts | 10 ++--- src/hooks.ts | 12 +----- src/qweb/extensions.ts | 36 ----------------- tests/animations.test.ts | 23 ----------- tests/component/component.test.ts | 65 ------------------------------- 7 files changed, 5 insertions(+), 161 deletions(-) diff --git a/doc/component.md b/doc/component.md index 94292e7f..abce1830 100644 --- a/doc/component.md +++ b/doc/component.md @@ -16,7 +16,6 @@ - [Event Handling](#event-handling) - [Form Input Bindings](#form-input-bindings) - [`t-key` Directive](#t-key-directive) - - [`t-mounted` Directive](#t-mounted-directive) - [Semantics](#semantics) - [Props Validation](#props-validation) - [References](#references) @@ -756,24 +755,6 @@ There are three main use cases: - _animations_: give a different identity to a component. Ex: thread id with animations on add/remove message. -### `t-mounted` Directive - -The `t-mounted` directive allows to register a callback to execute whenever the node -is inserted into the DOM. - -```xml -
-``` - -```js -class MyComponent extends owl.Component { - ... - focusMe() { - this.refs.someInput.focus(); - } -} -``` - ### Semantics We give here an informal description of the way components are created/updated diff --git a/doc/qweb.md b/doc/qweb.md index e9c5c4b5..984aae89 100644 --- a/doc/qweb.md +++ b/doc/qweb.md @@ -72,7 +72,6 @@ needs. Here is a list of all Owl specific directives: | `t-key` | [Defining a key (to help virtual dom reconciliation)](component.md#t-key-directive) | | `t-on-*` | [Event handling](component.md#event-handling) | | `t-transition` | [Defining an animation](animations.md#css-transitions) | -| `t-mounted` | [Callback when a node or component is mounted](component.md#t-mounted-directive) | | `t-slot` | [Rendering a slot](component.md#slots) | | `t-model` | [Form input bindings](component.md#form-input-bindings) | diff --git a/src/component/component.ts b/src/component/component.ts index 194487e8..4f4fedcb 100644 --- a/src/component/component.ts +++ b/src/component/component.ts @@ -82,7 +82,7 @@ interface Internal { boundHandlers: { [key: number]: any }; observer: Observer | null; render: CompiledTemplate | null; - mountedHandlers: { [key: number]: Function }; + mountedCB: Function | null; willUnmountCB: Function | null; willPatchCB: Function | null; patchedCB: Function | null; @@ -186,7 +186,7 @@ export class Component { cmap: {}, currentFiber: null, boundHandlers: {}, - mountedHandlers: {}, + mountedCB: null, willUnmountCB: null, willPatchCB: null, patchedCB: null, @@ -480,11 +480,10 @@ export class Component { } } __owl__.isMounted = true; - const handlers = __owl__.mountedHandlers; try { this.mounted(); - for (let key in handlers) { - handlers[key](); + if (__owl__.mountedCB) { + __owl__.mountedCB() } } catch (e) { errorHandler(e, this); @@ -601,7 +600,6 @@ export class Component { vnode = __owl__.render!(this, { promises, handlers: __owl__.boundHandlers, - mountedHandlers: __owl__.mountedHandlers, fiber: fiber }); } catch (e) { diff --git a/src/hooks.ts b/src/hooks.ts index 5ce98901..8e3e5a37 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -29,17 +29,6 @@ export function useState(state: T): T { return __owl__.observer.observe(state); } -/** - * Mounted hook. The callback will be called when the current component is - * mounted. Note that the component mounted method is called first. - */ -let nextID = 1; - -export function onMounted(cb) { - const component: Component = Component._current; - component.__owl__.mountedHandlers[`h${nextID++}`] = cb; -} - function makeLifecycleHook(method: string, reverse: boolean = false) { return function(cb) { const component: Component = Component._current; @@ -66,6 +55,7 @@ function makeLifecycleHook(method: string, reverse: boolean = false) { * willUnmount hook. The callback will be called when the current component is * willUnmounted. Note that the component mounted method is called last. */ +export const onMounted = makeLifecycleHook("mountedCB", true); export const onWillUnmount = makeLifecycleHook("willUnmountCB"); export const onWillPatch = makeLifecycleHook("willPatchCB"); export const onPatched = makeLifecycleHook("patchedCB", true); diff --git a/src/qweb/extensions.ts b/src/qweb/extensions.ts index 0e1c2016..2592d796 100644 --- a/src/qweb/extensions.ts +++ b/src/qweb/extensions.ts @@ -183,42 +183,6 @@ QWeb.addDirective({ } }); -//------------------------------------------------------------------------------ -// t-mounted -//------------------------------------------------------------------------------ -QWeb.addDirective({ - name: "mounted", - priority: 97, - atNodeCreation({ ctx, fullName, value, nodeID, addNodeHook }) { - ctx.rootContext.shouldDefineOwner = true; - const eventName = fullName.slice(5); - if (!eventName) { - throw new Error("Missing event name with t-on directive"); - } - let extraArgs; - let handler = value.replace(/\(.*\)/, function(args) { - extraArgs = args.slice(1, -1); - return ""; - }); - let error = `(function () {throw new Error('Missing handler \\'' + '${handler}' + \`\\' when evaluating template '${ctx.templateName.replace( - /`/g, - "'" - )}'\`)})()`; - if (extraArgs) { - ctx.addLine( - `extra.mountedHandlers[${nodeID}] = (context['${handler}'] || ${error}).bind(owner, ${ctx.formatExpression( - extraArgs - )});` - ); - } else { - ctx.addLine( - `extra.mountedHandlers[${nodeID}] = extra.mountedHandlers[${nodeID}] || (context['${handler}'] || ${error}).bind(owner);` - ); - } - addNodeHook("insert", `if (context.__owl__.isMounted) { extra.mountedHandlers[${nodeID}](); }`); - } -}); - //------------------------------------------------------------------------------ // t-slot //------------------------------------------------------------------------------ diff --git a/tests/animations.test.ts b/tests/animations.test.ts index 7b51cf50..851b3b3d 100644 --- a/tests/animations.test.ts +++ b/tests/animations.test.ts @@ -5,7 +5,6 @@ import { makeDeferred, makeTestFixture, makeTestEnv, - nextTick, patchNextFrame, renderToDOM, unpatchNextFrame @@ -265,28 +264,6 @@ describe("animations", () => { expect(fixture.innerHTML).toBe("
"); }); - test("t-transition combined with t-mounted", async () => { - env.qweb.addTemplate( - "TestWidget", - `
blue
` - ); - class TestWidget extends Widget { - state = useState({ flag: false }); - f() {} - } - const widget = new TestWidget(env); - widget.f = jest.fn(); - await widget.mount(fixture); - - patchNextFrame(cb => cb()); - expect(widget.f).toHaveBeenCalledTimes(0); - - widget.state.flag = true; - await nextTick(); - expect(widget.f).toHaveBeenCalledTimes(1); - unpatchNextFrame(); - }); - test("t-transition, remove and re-add before transitionend", async () => { expect.assertions(11); diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index 378912b7..f8b68b9e 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -2959,71 +2959,6 @@ describe("widget and observable state", () => { }); }); -describe("t-mounted directive", () => { - test("callback is not called when not in DOM", async () => { - env.qweb.addTemplate("TestWidget", `
`); - class TestWidget extends Widget { - f() {} - } - const widget = new TestWidget(env); - widget.f = jest.fn(); - await widget.mount(document.createElement("div")); - expect(widget.f).toHaveBeenCalledTimes(0); - }); - - test("callback is called when in DOM", async () => { - env.qweb.addTemplate("TestWidget", `
`); - class TestWidget extends Widget { - f() {} - } - const widget = new TestWidget(env); - widget.f = jest.fn(); - await widget.mount(fixture); - expect(widget.f).toHaveBeenCalledTimes(1); - }); - - test("callback with args is called when in DOM", async () => { - env.qweb.addTemplate("TestWidget", `
`); - class TestWidget extends Widget { - f() {} - } - const widget = new TestWidget(env); - widget.f = jest.fn(); - await widget.mount(fixture); - expect(widget.f).toHaveBeenCalledTimes(1); - expect(widget.f).toHaveBeenCalledWith(2); - }); - - test("combined with a t-if", async () => { - env.qweb.addTemplate("TestWidget", `
`); - class TestWidget extends Widget { - state = useState({ flag: false }); - f() {} - } - const widget = new TestWidget(env); - widget.f = jest.fn(); - await widget.mount(fixture); - expect(widget.f).toHaveBeenCalledTimes(0); - - widget.state.flag = true; - await nextTick(); - expect(widget.f).toHaveBeenCalledTimes(1); - }); - - test("combined with a t-ref", async () => { - env.qweb.addTemplate("TestWidget", `
`); - class TestWidget extends Widget { - input = useRef("input"); - f() {} - } - const widget = new TestWidget(env); - widget.f = jest.fn(); - await widget.mount(fixture); - expect(widget.input.el).toBeDefined(); - expect(widget.f).toHaveBeenCalledTimes(1); - }); -}); - describe("can deduce template from name", () => { test("can find template if name of component", async () => { class ABC extends Widget {}