From aa95149997340a961f3dc98d358722370a24b6f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 24 Jan 2022 14:39:12 +0100 Subject: [PATCH] [IMP] hooks: introduce useChildSubEnv and change useSubEnv --- CHANGELOG.md | 17 ++-- doc/readme.md | 3 +- doc/reference/environment.md | 2 +- doc/reference/hooks.md | 25 ++++-- src/hooks.ts | 16 +++- src/index.ts | 2 +- .../__snapshots__/hooks.test.ts.snap | 77 +++++++++++++++++-- tests/components/hooks.test.ts | 72 ++++++++++++++++- 8 files changed, 183 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8e1c857..8e2f1df4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,8 @@ All changes are documented here in no particular order. - new `useEffect` hook ([doc](doc/reference/hooks.md#useeffect)) - breaking: `Context` is removed ([details](#15-context-is-removed)) - breaking: `env` is now totally empty ([details](#16-env-is-now-totally-empty)) +- breaking: `env` is now frozen ([details](#28-env-is-now-frozen)) +- new hook: `useChildSubEnv` (only applies to child components) ([details](#27-usechildsubenv-only-applies-to-child-components)) - breaking: most exports are exported at top level ([details](#18-most-exports-are-exported-at-top-level)) - breaking: properties are no longer set as attributes ([details](#19-properties-are-no-longer-set-as-attributes)) - breaking: `EventBus` api changed: it is now an `EventTarget` ([details](#21-eventbus-api-changed-it-is-now-an-eventtarget)) @@ -81,8 +83,6 @@ All changes are documented here in no particular order. - breaking: transition system is removed ([details](#24-transition-system-is-removed)) - breaking: no more global components or templates ([details](#25-no-more-global-components-or-templates)) - breaking: `AsyncRoot` utility component is removed ([details](#26-asyncroot-utility-component-is-removed)) -- breaking: `useSubEnv` only applies to child components ([details](#27-usesubenv-only-applies-to-child-components)) -- breaking: `env` is now frozen ([details](#28-env-is-now-frozen)) - breaking: `renderToString` function on qweb has been removed ([details](#32-rendertostring-on-qweb-has-been-removed)) - breaking: `debounce` utility function has been removed ([details](#34-debounce-utility-function-has-been-removed)) - breaking: `browser` object has been removed ([details](#39-browser-object-has-been-removed)) @@ -565,15 +565,12 @@ either a fallback when the data is not ready, or the actual component with data as props. If there is no escape, and `AsyncRoot` is needed, please reach out to us so we can study this usecase. -### 27. `useSubEnv` only applies to child components +### 27. `useChildSubEnv` (only applies to child components) -In Owl 1, a call to `useSubEnv` would define a new environment for the children -AND the component. It now only defines an environment for the children. - -Rationale: This was a subtle cause for bugs: some code had to be rrun -before the call to `useSubEnv`, otherwise it could interfere with the sub environment. - -Documentation: [Hooks](doc/reference/hooks.md#usesubenv) +In Owl, a call to `useSubEnv` would define a new environment for the children +AND the component. It is very useful, but in some cases, one only need to update +the children component environment. This can now be done with a new hook: +[`useChildSubEnv`](doc/reference/hooks.md#usesubenv-and-usechildsubenv) ### 28. `env` is now frozen diff --git a/doc/readme.md b/doc/readme.md index 56e86824..90337156 100644 --- a/doc/readme.md +++ b/doc/readme.md @@ -36,7 +36,8 @@ Other hooks: - [`useEnv`](reference/hooks.md#useenv): return a reference to the current env - [`useExternalListener`](reference/hooks.md#useexternallistener): add a listener outside of a component DOM - [`useRef`](reference/hooks.md#useref): get an object representing a reference (`t-ref`) -- [`useSubEnv`](reference/hooks.md#usesubenv): extend the current env with additional information for child components +- [`useChildSubEnv`](reference/hooks.md#usesubenv-and-usechildsubenv): extend the current env with additional information (for child components) +- [`useSubEnv`](reference/hooks.md#usesubenv-and-usechildsubenv): extend the current env with additional information (for current component and child components) Utility/helpers: diff --git a/doc/reference/environment.md b/doc/reference/environment.md index ee17f294..50069d72 100644 --- a/doc/reference/environment.md +++ b/doc/reference/environment.md @@ -52,7 +52,7 @@ It is sometimes useful to add one (or more) specific keys to the environment, from the perspective of a specific component and its children. In that case, the solution presented above will not work, since it sets the global environment. -There is a hook for this situation: [`useSubEnv`](hooks.md#usesubenv). +There are two hooks for this situation: [`useSubEnv` and `useChildSubEnv`](hooks.md#usesubenv-and-usechildsubenv). ```js class SomeComponent extends Component { diff --git a/doc/reference/hooks.md b/doc/reference/hooks.md index 77c173c4..0e544faa 100644 --- a/doc/reference/hooks.md +++ b/doc/reference/hooks.md @@ -8,7 +8,7 @@ - [Other hooks](#other-hooks) - [`useState`](#usestate) - [`useRef`](#useref) - - [`useSubEnv`](#usesubenv) + - [`useSubEnv` and `useChildSubEnv`](#usesubenv-and-usechildsubenv) - [`useExternalListener`](#useexternallistener) - [`useComponent`](#usecomponent) - [`useEnv`](#useenv) @@ -152,14 +152,14 @@ this.ref2 = useRef("component_2"); References are only guaranteed to be active while the parent component is mounted. If this is not the case, accessing `el` or `comp` on it will return `null`. -### `useSubEnv` +### `useSubEnv` and `useChildSubEnv` The environment is sometimes useful to share some common information between all components. But sometimes, we want to _scope_ that knowledge to a subtree. For example, if we have a form view component, maybe we would like to make some `model` object available to all sub components, but not to the whole application. -This is where the `useSubEnv` hook may be useful: it lets a component add some +This is where the `useChildSubEnv` hook may be useful: it lets a component add some information to the environment in a way that only its children can access it: @@ -167,15 +167,26 @@ can access it: class FormComponent extends Component { setup() { const model = makeModel(); + // model will be available on this.env for this component and all children useSubEnv({ model }); + // someKey will be available on this.env for all children + useChildSubEnv({ someKey: "value" }); } } ``` -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. +The `useSubEnv` and `useChildSubEnv` hooks take one argument: an object which +contains some key/value that will be added to the current environment. These hooks +will create a new env object with the new information: + +- `useSubEnv` will assign this new `env` to itself and to all children components +- `useChildSubEnv` will only assign this new `env` to all children components. + +As usual in Owl, [environments](environment.md) created with these two hooks are +frozen, to prevent unwanted modifications. + +Note that both these hooks can be called an arbitrary number of times. The `env` +will then be updated accordingly. ### `useExternalListener` diff --git a/src/hooks.ts b/src/hooks.ts index b3758dad..ff259cc8 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -32,6 +32,12 @@ export function useEnv(): E { return getCurrent()!.component.env as any; } +function extendEnv(currentEnv: Object, extension: Object): Object { + const env = Object.create(currentEnv); + const descrs = Object.getOwnPropertyDescriptors(extension); + return Object.freeze(Object.defineProperties(env, descrs)); +} + /** * This hook is a simple way to let components use a sub environment. Note that * like for all hooks, it is important that this is only called in the @@ -39,11 +45,15 @@ export function useEnv(): E { */ export function useSubEnv(envExtension: Env) { const node = getCurrent()!; - const env = Object.create(node.childEnv); - const descrs = Object.getOwnPropertyDescriptors(envExtension); - node.childEnv = Object.freeze(Object.defineProperties(env, descrs)); + const newEnv = extendEnv(node.component.env as any, envExtension); + node.component.env = extendEnv(node.component.env as any, envExtension); + node.childEnv = newEnv; } +export function useChildSubEnv(envExtension: Env) { + const node = getCurrent()!; + node.childEnv = extendEnv(node.childEnv, envExtension); +} // ----------------------------------------------------------------------------- // useEffect // ----------------------------------------------------------------------------- diff --git a/src/index.ts b/src/index.ts index f4bcd9eb..11a3ed3f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -43,7 +43,7 @@ export { status } from "./component/status"; export { Memo } from "./memo"; export { xml } from "./app/template_set"; export { useState, reactive, markRaw, toRaw } from "./reactivity"; -export { useEffect, useEnv, useExternalListener, useRef, useSubEnv } from "./hooks"; +export { useEffect, useEnv, useExternalListener, useRef, useChildSubEnv, useSubEnv } from "./hooks"; export { EventBus, whenReady, loadFile, markup } from "./utils"; export { onWillStart, diff --git a/tests/components/__snapshots__/hooks.test.ts.snap b/tests/components/__snapshots__/hooks.test.ts.snap index 7554cd6c..d5edf3cb 100644 --- a/tests/components/__snapshots__/hooks.test.ts.snap +++ b/tests/components/__snapshots__/hooks.test.ts.snap @@ -103,7 +103,7 @@ exports[`hooks mounted callbacks should be called in reverse order from willUnmo }" `; -exports[`hooks parent and child env 1`] = ` +exports[`hooks parent and child env (with useChildSubEnv) 1`] = ` "function anonymous(bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; @@ -116,7 +116,34 @@ exports[`hooks parent and child env 1`] = ` }" `; -exports[`hooks parent and child env 2`] = ` +exports[`hooks parent and child env (with useChildSubEnv) 2`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let txt1 = ctx['env'].val; + return block1([txt1]); + } +}" +`; + +exports[`hooks parent and child env (with useSubEnv) 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + return function template(ctx, node, key = \\"\\") { + let b2 = text(ctx['env'].val); + let b3 = component(\`Child\`, {}, key + \`__1\`, node, ctx); + return multi([b2, b3]); + } +}" +`; + +exports[`hooks parent and child env (with useSubEnv) 2`] = ` "function anonymous(bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; @@ -144,7 +171,7 @@ exports[`hooks two different call to willPatch/patched should work 1`] = ` }" `; -exports[`hooks use sub env does not pollute user env 1`] = ` +exports[`hooks useChildSubEnv does not pollute user env 1`] = ` "function anonymous(bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; @@ -158,7 +185,7 @@ exports[`hooks use sub env does not pollute user env 1`] = ` }" `; -exports[`hooks use sub env supports arbitrary descriptor 1`] = ` +exports[`hooks useChildSubEnv supports arbitrary descriptor 1`] = ` "function anonymous(bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; @@ -169,7 +196,7 @@ exports[`hooks use sub env supports arbitrary descriptor 1`] = ` }" `; -exports[`hooks use sub env supports arbitrary descriptor 2`] = ` +exports[`hooks useChildSubEnv supports arbitrary descriptor 2`] = ` "function anonymous(bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; @@ -287,3 +314,43 @@ exports[`hooks useRef hook: basic use 1`] = ` } }" `; + +exports[`hooks useSubEnv modifies user env 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let txt1 = ctx['env'].val; + return block1([txt1]); + } +}" +`; + +exports[`hooks useSubEnv supports arbitrary descriptor 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + return function template(ctx, node, key = \\"\\") { + return component(\`Child\`, {}, key + \`__1\`, node, ctx); + } +}" +`; + +exports[`hooks useSubEnv supports arbitrary descriptor 2`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let txt1 = ctx['env'].someVal; + let txt2 = ctx['env'].someVal2; + return block1([txt1, txt2]); + } +}" +`; diff --git a/tests/components/hooks.test.ts b/tests/components/hooks.test.ts index a7df5584..fc08ba42 100644 --- a/tests/components/hooks.test.ts +++ b/tests/components/hooks.test.ts @@ -14,6 +14,7 @@ import { useExternalListener, useRef, useState, + useChildSubEnv, useSubEnv, xml, } from "../../src/index"; @@ -180,7 +181,7 @@ describe("hooks", () => { expect(fixture.innerHTML).toBe("
1
"); }); - test("use sub env does not pollute user env", async () => { + test("useSubEnv modifies user env", async () => { class Test extends Component { static template = xml`
`; setup() { @@ -190,11 +191,25 @@ describe("hooks", () => { const env = { val: 3 }; const component = await mount(Test, fixture, { env }); expect(fixture.innerHTML).toBe("
3
"); + expect(component.env).toHaveProperty("val2"); + expect(component.env).toHaveProperty("val"); + }); + + test("useChildSubEnv does not pollute user env", async () => { + class Test extends Component { + static template = xml`
`; + setup() { + useChildSubEnv({ val2: 1 }); + } + } + const env = { val: 3 }; + const component = await mount(Test, fixture, { env }); + expect(fixture.innerHTML).toBe("
3
"); expect(component.env).not.toHaveProperty("val2"); expect(component.env).toHaveProperty("val"); }); - test("use sub env supports arbitrary descriptor", async () => { + test("useSubEnv supports arbitrary descriptor", async () => { let someVal = "maggot"; let someVal2 = "brain"; @@ -213,6 +228,40 @@ describe("hooks", () => { }); } } + + const env = { + get someVal() { + return someVal; + }, + }; + const component = await mount(Test, fixture, { env }); + expect(fixture.innerHTML).toBe("
maggot brain
"); + someVal = "brain"; + someVal2 = "maggot"; + component.render(); + await nextTick(); + expect(fixture.innerHTML).toBe("
brain maggot
"); + }); + + test("useChildSubEnv supports arbitrary descriptor", async () => { + let someVal = "maggot"; + let someVal2 = "brain"; + + class Child extends Component { + static template = xml`
`; + } + + class Test extends Component { + static template = xml``; + static components = { Child }; + setup() { + useChildSubEnv({ + get someVal2() { + return someVal2; + }, + }); + } + } someVal = "maggot"; const env = { get someVal() { @@ -239,7 +288,7 @@ describe("hooks", () => { await mount(Test, fixture); }); - test("parent and child env", async () => { + test("parent and child env (with useSubEnv)", async () => { class Child extends Component { static template = xml`
`; } @@ -253,6 +302,23 @@ describe("hooks", () => { } const env = { val: 3 }; await mount(Parent, fixture, { env }); + expect(fixture.innerHTML).toBe("5
5
"); + }); + + test("parent and child env (with useChildSubEnv)", async () => { + class Child extends Component { + static template = xml`
`; + } + + class Parent extends Component { + static template = xml``; + static components = { Child }; + setup() { + useChildSubEnv({ val: 5 }); + } + } + const env = { val: 3 }; + await mount(Parent, fixture, { env }); expect(fixture.innerHTML).toBe("3
5
"); });