Compare commits

..

8 Commits

Author SHA1 Message Date
Géry Debongnie 144b323d2b [REL] v1.2.0
# Owl v1.2.0

## Changes

- translation fix for terms surrounded by spaces
- fix: properly remount components with shouldUpdate=false
- add: add two new generic hooks:  and
- fix: do not skip rendering in components using store and local state (in some cases)

Note that the last change is a pretty significant change: component connected to
a store should be very careful if the data that they represent is deleted, because
they will always be rendered with the current state of the store.
2020-12-14 13:23:46 +01:00
Géry Debongnie bb64e87634 [FIX] store: properly render, even if shouldupdate is implemented
Before this commit, an unwanted behaviour happened when using components
with shouldUpdate implemented, and store/state.

The actual problem is the following: the sub component is using a store,
and shouldupdate.  Whenever the component is rendered, it checks if there is an incoming
rendering from the context.  If that is the case, it skips the
rendering, because we actually only want to be rendered by the store
rendering (otherwise, we may run into issue with inconsistent data (more
recent data from the context, older data in the component).

However, if shouldUpdate is implemented, then the rendering coming from
the context simply does not arrive.

To fix this, we tried to just force these renderings to go through all children,
regardless of their shouldUpdate status. This actually works, but then
shouldUpdate is ignored, which is an issue in Odoo discuss.

Then, after discussing this situation, we noticed that the context/store
system is actually unsafe: the protection given by the check mentioned
above is in fact fundamentally insufficient: there are other perfectly
valid situations where a rendering can be triggered on the component,
which will bypass the check (for example, an explicit call to
this.render() or a rendering initiated by some parent component) and
cause a crash if the component is not properly defensively written.

Therefore, it is currently mandatory for all components using
context/store to be aware of that, and to protect themselves against
such situations. So, in that regard, the check is not really a
protection, it just helps hiding an unsafe situation anyway and we
decided to remove it.

Note that this is a potentially breaking change: components using a
store and some local state will now be rendered twice in some cases...

closes #799
2020-12-14 13:19:36 +01:00
Géry Debongnie 9a87b9a4a0 [IMP] hooks: add some building blocks for hooks
This commit introduces two new hooks: useComponent, and
useEnv.
2020-12-14 11:53:01 +01:00
Géry Debongnie 2a53a9592e [FIX] component: force mounting subcomponents with shouldUpdate
Components can implement shouldUpdate to return false.  In that case,
renderings coming from above should be ignored.

However, if the component was unmounted and is remounted, we actually
need to force a rerendering in that case, so it is mounted, otherwise
the subcomponent is left in unmounted state, which means that rendering
are ignored.

closes #800
2020-12-11 15:19:37 +01:00
Ivan Yelizariev f54b9a4a0c [FIX] properly translate terms surrounded by spaces
This reimplements logic from Odoo v13 [1], i.e. if we have a text that follows
by space e.g. "This database will expire in " [2], then we want get translation
for the term without that space and add the space manually. This way we can
export trimmed terms for translations and don't miss the space, when atranslator
forget to add it at the end of translation

[1] https://github.com/odoo/odoo/blob/26927417e2957acba6fb79446c2520107daa7eea/addons/web/static/src/js/core/qweb.js#L46
[2] https://github.com/odoo/enterprise/blob/ab0e893493f909ec3033e8a1cf6c141ea9375587/web_enterprise/static/src/xml/base.xml#L21

---

opw-2410708
2020-12-03 13:36:17 +01:00
Géry Debongnie abb9d0b364 [REL] v1.1.1
# Owl v1.1.1

## Changes

- export  object to make utility functions testable
- minor doc improvements and corrections
- fix: make sure owl does not crash in iframe in private mode
2020-11-20 11:02:19 +01:00
Géry Debongnie 8483cc805e [FIX/IMP] export browser object, prevent crash in private mode
This commit performs two tasks:

1. it exports the internal browser object (in owl.browser), for ease of
testing
2. it makes sure that Owl does not crash inside iframes in incognito
mode, because window.localStorage raises an exception in that case

closes #791
2020-11-20 10:55:08 +01:00
Simon Genin 142f47ac82 [DOC] Fix animation documentation (#783)
The documentation was a bit missleading around which and when css
classes were added to the DOM.
Also added a scss mixin usage exemple.

Co-authored-by: Simon Genin (ges) <ges@odoo.com>
2020-11-18 09:41:50 +01:00
23 changed files with 355 additions and 80 deletions
-32
View File
@@ -1,32 +0,0 @@
name: "CLA Assistant"
on:
issue_comment:
types: [created]
pull_request_target:
types: [opened,closed,synchronize]
jobs:
CLAssistant:
runs-on: ubuntu-latest
steps:
- name: "CLA Assistant"
if: (github.event.comment.body == 'cla check' || github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA') || github.event_name == 'pull_request_target'
uses: cla-assistant/github-action@v2.0.1-alpha
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# the below token should have repo scope and must be manually added by you in the repository's secret
PERSONAL_ACCESS_TOKEN : ${{ secrets.PERSONAL_ACCESS_TOKEN }}
with:
path-to-signatures: 'signatures/cla.json'
path-to-document: 'https://github.com/odoo/odoo/blob/master/doc/cla/icla-1.0.md' # e.g. a CLA or a DCO document
branch: 'master'
allowlist: SimonGenin,brboi,kebeclibre,*-odoo,bot*
use-dco-flag: false #'Set this to true if you want to use a dco instead of a cla'
#below are the optional inputs - If the optional inputs are not given, then default values will be taken
#remote-organization-name: enter the remote organization name where the signatures should be stored (Default is storing the signatures in the same repository)
#remote-repository-name: enter the remote repository name where the signatures should be stored (Default is storing the signatures in the same repository)
#create-file-commit-message: 'For example: Creating file for storing CLA Signatures'
#signed-commit-message: 'For example: $contributorName has signed the CLA in #$pullRequestNo'
#custom-notsigned-prcomment: 'pull request comment with Introductory message to ask new contributors to sign'
#custom-allsigned-prcomment: 'pull request comment when all contributors has signed, defaults to **CLA Assistant Lite bot** All Contributors have signed the CLA.'
+1 -1
View File
@@ -120,7 +120,7 @@ npm install @odoo/owl
If you want to use a simple `<script>` tag, the last release can be downloaded here:
- [owl-1.0.13](https://github.com/odoo/owl/releases/tag/v1.0.13)
- [owl-1.2.0](https://github.com/odoo/owl/releases/tag/v1.2.0)
## License
+1
View File
@@ -17,6 +17,7 @@ You will find here a complete reference of every feature, class or object
provided by Owl.
- [Animations](reference/animations.md)
- [Browser](reference/browser.md)
- [Component](reference/component.md)
- [Content](reference/content.md)
- [Concurrency Model](reference/concurrency_model.md)
+41 -9
View File
@@ -52,21 +52,20 @@ sequence of events will happen:
At node insertion:
- the css classes `name-enter` and `name-enter-active` will be added directly
when the node is inserted into the DOM,
when the node is inserted into the DOM.
- on the next animation frame: the css class `name-enter` will be removed and the
class `name-enter-to` will be added (so they can be used to trigger css
transition effects),
- the css class `name-enter-active` will be removed whenever a css transition
ends.
transition effects).
- at the end of the transition, `name-enter-to` and `name-enter-active` will be removed.
At node destruction:
- the css classes `name-leave` and `name-leave-active` will be added before the
node is removed to the DOM,
- the css class `name-leave` will be removed on the next animation frame (so it
can be used to trigger css transition effects),
- the css class `name-leave-active` will be removed whenever a css transition
ends. Only then will the element be removed from the DOM.
node is removed to the DOM.
- on the next animation frame: the css class `name-leave` will be removed and the
class `name-leave-to` will be added (so they can be used to trigger css
transition effects).
- at the end of the transition, `name-leave-to` and `name-leave-active` will be removed.
For example, a simple fade in/out effect can be done with this:
@@ -93,3 +92,36 @@ Notes:
Owl does not support more than one transition on a single node, so the
`t-transition` expression must be a single value (i.e. no space allowed).
## SCSS Mixins
If you use SCSS, you can use mixins to make generic animations. Here is an exemple with a fade in / fade out animation:
```scss
@mixin animation-fade($time, $name) {
.#{$name}_fade-enter-active,
.#{$name}_fade-active {
transition: all $time;
}
.#{$name}_fade-enter {
opacity: 0;
}
.#{$name}_fade-leave-to {
opacity: 0;
}
}
```
Usage:
```scss
@include animation-fade(0.5s, "o_notification");
```
You can now have in your template:
```xml
<SomeTag t-transition="o_notification_fade"/>
```
+33
View File
@@ -0,0 +1,33 @@
# 🦉 Browser 🦉
## Content
- [Overview](#overview)
- [Browser Content](#browser-content)
## Overview
The browser object contains some browser native APIs, such as `setTimeout`, that
are used by Owl and its utility functions. They are exposed with the intent of
making them mockable if necessary.
```js
owl.browser.setTimeout === window.setTimeout; // return true
```
For now, this object contains some functions that are not used by Owl. They
will eventually be removed in Owl 2.0.
## Browser Content
More specifically, the `browser` object contains the following methods and objects:
- `setTimeout`
- `clearTimeout`
- `setInterval`
- `clearInterval`
- `requestAnimationFrame`
- `random`
- `Date`
- `fetch`
- `localStorage`
+3
View File
@@ -7,6 +7,7 @@ For example, `Component` is available at `owl.Component` and `EventBus` is
exported as `owl.core.EventBus`.
```
browser
Component misc
Context AsyncRoot
QWeb Portal
@@ -28,6 +29,8 @@ hooks utils
useContext
useState
useRef
useComponent
useEnv
useSubEnv
useStore
useDispatch
+1 -14
View File
@@ -133,17 +133,4 @@ the `QWeb` instance and a `browser` object:
- `qweb` will be set to an empty `QWeb` instance. This is absolutely necessary
for Owl to be able to render anything
- `browser`: this is an object that contains some common access points to the
browser methods with a side effect. This is particularly useful when one want
to test more advanced components, and be able to mock those methods.
More specifically, the `browser` object contains the following methods and objects:
- `setTimeout`
- `clearTimeout`
- `setInterval`
- `clearInterval`
- `requestAnimationFrame`
- `random`
- `Date`
- `fetch`
- `localStorage`
browser methods with a side effect. See [browser](browser.md) for more information. Note that the browser object will be removed from the environment in Owl 2.0.
+14 -4
View File
@@ -21,6 +21,8 @@
- [`useStore`](#usestore)
- [`useDispatch`](#usedispatch)
- [`useGetters`](#usegetters)
- [`useComponent`](#usecomponent)
- [`useEnv`](#useenv)
- [Making customized hooks](#making-customized-hooks)
## Overview
@@ -381,6 +383,16 @@ The `useDispatch` hook is the way for components to get a reference to the store
The `useGetters` hook is the way for components to get a reference to the store
getters. See the [store documentation](store.md) for more information.
### `useComponent`
The `useComponent` hook is useful as a building block for some customized hooks,
that may need a reference to the component calling them.
### `useEnv`
The `useEnv` hook is useful as a building block for some customized hooks,
that may need a reference to the env of the component calling them.
### Making customized hooks
Hooks are a wonderful way to organize the code of a complex component by feature
@@ -435,13 +447,11 @@ not the solution to every problem.
```js
function useRouter() {
return Component.current.env.router;
const env = useEnv();
return 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.
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "@odoo/owl",
"version": "1.0.13",
"version": "1.2.0",
"description": "Odoo Web Library (OWL)",
"main": "dist/owl.cjs.js",
"browser": "dist/owl.iife.js",
+1 -1
View File
@@ -1,6 +1,6 @@
# 🦉 OWL Roadmap 🦉
- Current version: 1.0.13
- Current version: 1.2.0
- Status: stable
This roadmap is only an attempt at predicting Owl's future. Everything may
+8 -1
View File
@@ -10,6 +10,8 @@ export interface Browser {
localStorage: Window["localStorage"];
}
let localStorage: Window["localStorage"] | null = null;
export const browser: Browser = {
setTimeout: window.setTimeout.bind(window),
clearTimeout: window.clearTimeout.bind(window),
@@ -19,5 +21,10 @@ export const browser: Browser = {
random: Math.random,
Date: window.Date,
fetch: (window.fetch || (() => {})).bind(window),
localStorage: window.localStorage,
get localStorage() {
return localStorage || window.localStorage;
},
set localStorage(newLocalStorage: Window["localStorage"]) {
localStorage = newLocalStorage;
},
};
+2 -1
View File
@@ -159,6 +159,7 @@ export class Component<Props extends {} = any, T extends Env = Env> {
if (!this.env.qweb) {
this.env.qweb = new QWeb();
}
// TODO: remove this in owl 2.0
if (!this.env.browser) {
this.env.browser = browser;
}
@@ -332,7 +333,7 @@ export class Component<Props extends {} = any, T extends Env = Env> {
message += `\nMaybe the DOM is not ready yet? (in that case, you can use owl.utils.whenReady)`;
throw new Error(message);
}
const fiber = new Fiber(null, this, false, target, position);
const fiber = new Fiber(null, this, true, target, position);
fiber.shouldPatch = false;
if (!__owl__.vnode) {
this.__prepareAndRender(fiber, () => {});
-10
View File
@@ -115,16 +115,6 @@ export function useContextWithCB(ctx: Context, component: Component, method): an
__owl__.observer = new Observer();
__owl__.observer.notifyCB = component.render.bind(component);
}
const currentCB = __owl__.observer.notifyCB;
__owl__.observer.notifyCB = function () {
if (ctx.rev > mapping[id]) {
// in this case, the context has been updated since we were rendering
// last, and we do not need to render here with the observer. A
// rendering is coming anyway, with the correct props.
return;
}
currentCB();
};
mapping[id] = 0;
const renderFn = __owl__.renderFn;
+21 -1
View File
@@ -1,4 +1,4 @@
import { Component } from "./component/component";
import { Component, Env } from "./component/component";
import { Observer } from "./core/observer";
/**
@@ -118,6 +118,26 @@ export function useRef<C extends Component = Component>(name: string): Ref<C> {
};
}
// -----------------------------------------------------------------------------
// "Builder" hooks
// -----------------------------------------------------------------------------
/**
* This hook is useful as a building block for some customized hooks, that may
* need a reference to the component calling them.
*/
export function useComponent<P, E extends Env>(): Component<P, E> {
return Component.current as any;
}
/**
* This hook is useful as a building block for some customized hooks, that may
* need a reference to the env of the component calling them.
*/
export function useEnv<E extends Env>(): E {
return Component.current.env as any;
}
// -----------------------------------------------------------------------------
// useSubEnv
// -----------------------------------------------------------------------------
+1
View File
@@ -22,6 +22,7 @@ import { Router } from "./router/router";
export { Component, mount } from "./component/component";
export { QWeb };
export { config };
export { browser } from "./browser";
export const Context = _context.Context;
export const useState = _hooks.useState;
+3 -1
View File
@@ -70,6 +70,7 @@ const TRANSLATABLE_ATTRS = ["label", "title", "placeholder", "alt"];
const lineBreakRE = /[\r\n]/;
const whitespaceRE = /\s+/g;
const translationRE = /^(\s*)([\s\S]+?)(\s*)$/;
const NODE_HOOKS_PARAMS = {
create: "(_, n)",
@@ -496,7 +497,8 @@ export class QWeb extends EventBus {
}
if (this.translateFn) {
if ((node.parentNode as any).getAttribute("t-translation") !== "off") {
text = this.translateFn(text);
const match = translationRE.exec(text);
text = match[1] + this.translateFn(match[2]) + match[3];
}
}
if (ctx.parentNode) {
+1 -2
View File
@@ -1,5 +1,4 @@
import { Component } from "./component/component";
import { Env } from "./component/component";
import { Component, Env } from "./component/component";
import { Context, useContextWithCB } from "./context";
import { onWillUpdateProps } from "./hooks";
+47
View File
@@ -600,4 +600,51 @@ describe("unmounting and remounting", () => {
await parent.render();
expect(fixture.textContent).toBe("fixedsome text");
});
test("remounting component tree where a component implement shouldupdate", async () => {
let state: any;
const steps = [];
class Child extends Component {
static template = xml`<div><t t-esc="state.word"/><t t-esc="props.name"/></div>`;
state = useState({ word: "hello" });
constructor(parent, props) {
super(parent, props);
state = this.state;
}
patched() {
steps.push("patched");
}
mounted() {
steps.push("mounted");
}
willUnmount() {
steps.push("willUnmount");
}
shouldUpdate() {
return false;
}
}
class Parent extends Component {
static template = xml`<div><Child name="state.name"/></div>`;
static components = { Child };
state = useState({ name: "World" });
}
const parent = await mount(Parent, { target: fixture });
expect(fixture.innerHTML).toBe("<div><div>helloWorld</div></div>");
parent.unmount();
expect(fixture.innerHTML).toBe("");
await parent.mount(fixture);
expect(fixture.innerHTML).toBe("<div><div>helloWorld</div></div>");
state.word = "test";
await nextTick();
expect(fixture.innerHTML).toBe("<div><div>testWorld</div></div>");
expect(steps).toEqual(["mounted", "willUnmount", "mounted", "patched"]);
});
});
+20 -1
View File
@@ -289,7 +289,26 @@ describe("Context", () => {
expect(testContext.subscriptions.update.length).toBe(0);
});
test("concurrent renderings", async () => {
test.skip("concurrent renderings", async () => {
/**
* Note: this test is interesting, but sadly just an incomplete attempt at
* protecting users against themselves. With the context API, it is not
* possible for the framework to protect completely against crashes. Maybe
* like in this case, when a component is in a simple hierarchy where all
* renderings come from the context changes, but in a real case, where some
* code can trigger a rendering independently, it is insufficient.
*
* The main problem is that the sub component depends on some external state,
* which may be modified, and then incompatible with the component actual
* state (for example, if the sub component has an id key related to some
* object that has been removed from the context).
*
* For now, sadly, the only solution is that components that depends on external
* state should guarantee their own integrity themselves. Then maybe this
* could be solved at the level of a state management solution that has a
* more advanced API, to let components determine if they should be updated
* or not (so, something slightly more advanced that the useStore hook).
*/
const testContext = new Context({ x: { n: 1 }, key: "x" });
const def = makeDeferred();
let stateC;
+28
View File
@@ -9,8 +9,10 @@ import {
onWillPatch,
onWillStart,
onWillUpdateProps,
useEnv,
useSubEnv,
useExternalListener,
useComponent,
} from "../src/hooks";
import { xml } from "../src/tags";
@@ -520,6 +522,19 @@ describe("hooks", () => {
});
});
test("can use useEnv", async () => {
expect.assertions(1);
class TestComponent extends Component {
static template = xml`<div><t t-esc="env.val"/></div>`;
constructor() {
super();
expect(useEnv()).toBe(env);
}
}
const component = new TestComponent();
await component.mount(fixture);
});
test("can use sub env", async () => {
class TestComponent extends Component {
static template = xml`<div><t t-esc="env.val"/></div>`;
@@ -535,6 +550,19 @@ describe("hooks", () => {
expect(component.env).toHaveProperty("val");
});
test("can use useComponent", async () => {
expect.assertions(1);
class TestComponent extends Component {
static template = xml`<div></div>`;
constructor() {
super();
expect(useComponent()).toBe(this);
}
}
const component = new TestComponent();
await component.mount(fixture);
});
test("parent and child env", async () => {
class Child extends Component {
static template = xml`<div><t t-esc="env.val"/></div>`;
@@ -3924,6 +3924,18 @@ exports[`translation support some attributes are translated 1`] = `
}"
`;
exports[`translation support translation is done on the trimmed text, with extra spaces readded after 1`] = `
"function anonymous(context, extra
) {
// Template name: \\"test\\"
let h = this.h;
let c1 = [], p1 = {key:1};
let vn1 = h('div', p1, c1);
c1.push({text: \` mot \`});
return vn1;
}"
`;
exports[`whitespace handling consecutives whitespaces are condensed into a single space 1`] = `
"function anonymous(context, extra
) {
+11
View File
@@ -2154,6 +2154,17 @@ describe("translation support", () => {
'<div><p label="mot">mot</p><p title="mot">mot</p><p placeholder="mot">mot</p><p alt="mot">mot</p><p something="word">mot</p></div>'
);
});
test("translation is done on the trimmed text, with extra spaces readded after", () => {
const translations = {
word: "mot",
};
const translateFn = jest.fn((expr) => translations[expr] || expr);
const qweb = new QWeb({ translateFn });
qweb.addTemplate("test", "<div> word </div>");
expect(renderToString(qweb, "test")).toBe("<div> mot </div>");
expect(translateFn).toHaveBeenCalledWith("word");
});
});
describe("t-key tests", () => {
+105 -1
View File
@@ -1,4 +1,4 @@
import { Component, Env } from "../src/component/component";
import { Component, Env, mount } from "../src/component/component";
import { Store, useStore, useDispatch, useGetters, EnvWithStore } from "../src/store";
import { useState } from "../src/hooks";
import { xml } from "../src/tags";
@@ -1241,4 +1241,108 @@ describe("various scenarios", () => {
await nextTick();
expect(fixture.innerHTML).toMatchSnapshot();
});
test("component with store, useState and shouldUpdate=false", async () => {
let state: any;
const store = new Store({ state: { rev: 0 } });
class Child extends Component {
static template = xml`<div><t t-esc="state.word"/><t t-esc="props.name"/></div>`;
state = useState({ word: "hello" });
constructor(parent, props) {
super(parent, props);
state = this.state;
useStore((props) => {
return 1;
});
}
shouldUpdate() {
return false;
}
}
class Parent extends Component {
static template = xml`<div><Child name="state.name"/></div>`;
static components = { Child };
state = useState({ name: "World" });
constructor(parent, props) {
super(parent, props);
useStore((props) => store.state.rev);
}
}
(env as any).store = store;
await mount(Parent, { target: fixture, env });
expect(fixture.innerHTML).toBe("<div><div>helloWorld</div></div>");
store.state.rev++;
// this is the key to the bug, it makes Parent be in "render" state but not
// yet rendered while the change of state happens
await Promise.resolve();
state.word = "test";
await nextTick();
expect(fixture.innerHTML).toBe("<div><div>testWorld</div></div>");
});
test("component with store, useState, shouldUpdate=false and child with shouldupdate false", async () => {
let state: any;
const store = new Store({ state: { rev: 0 } });
class ChildChild extends Component {
static template = xml`<div><t t-esc="props.value"/></div>`;
shouldUpdate() {
return false;
}
}
class Child extends Component {
static template = xml`<div><t t-esc="state.word"/><t t-esc="props.name"/><ChildChild value="state.value"/></div>`;
static components = { ChildChild };
state = useState({ word: "hello", value: 3 });
constructor(parent, props) {
super(parent, props);
state = this.state;
useStore((props) => {
return 1;
});
}
shouldUpdate() {
return false;
}
}
class Parent extends Component {
static template = xml`<div><Child name="state.name"/></div>`;
static components = { Child };
state = useState({ name: "World" });
constructor(parent, props) {
super(parent, props);
useStore((props) => store.state.rev);
}
}
(env as any).store = store;
await mount(Parent, { target: fixture, env });
expect(fixture.innerHTML).toBe("<div><div>helloWorld<div>3</div></div></div>");
store.state.rev++;
// this is the key to the bug, it makes Parent be in "render" state but not
// yet rendered while the change of state happens
await Promise.resolve();
state.word = "test";
state.value = 44;
await nextTick();
expect(fixture.innerHTML).toBe("<div><div>testWorld<div>3</div></div></div>");
});
});