mirror of
https://github.com/odoo/owl.git
synced 2025-10-06 19:59:41 +07:00
[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
This commit is contained in:
@@ -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;
|
||||
|
||||
+1
-2
@@ -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";
|
||||
|
||||
|
||||
Reference in New Issue
Block a user