mirror of
https://github.com/odoo/owl.git
synced 2025-10-06 19:59:41 +07:00
[FIX] component, fiber: update props with virtual node should not crash
Have a hierarchy of A, B, C components where:
```xml
<div t-name="A">
<div>
<B t-key="key1"/>
</div>
</div>
<t t-name="B">
<C t-key="key2"/>
</t>
<div t-name="C">
<div><t t-esc="keys_as_props" /></div>
</div>
```
The subtility of the issues lies in B, which doesn't have its own
concrete DOM element, rather, it borrows it from C.
With the sequence of events:
- change key2
C1 is destroyed and replaced by another instance, and another node.
B1 has its props updated and is patched with the C2's node (CRITICAL)
A1 is patched
- change key1 AND key2
C2 is destroyed
B1 is destroyed
A1 is patched replacing B1 by B2, and their nodes too (which at this point should be C2's to C3's)
Before this commit, at the CRITICAL point, the node representing the component itself
(technically its pvnode) was not updated with the new concrete node provided by B1 patch with C2 node
i.e. it held the previous node still
The second array of steps crashed because at A1 patch, the new B2 node would replace B1, which
was out of the DOM (removed because C1 was destroyed long before),
and therefore without a viable parent to insert B2 node.
After this commit, we update the component's pvnode after the patch which elm had possibly changed
There is no crash anymore for this use case.
This commit is contained in:
committed by
Géry Debongnie
parent
9baea2c1cd
commit
81af21a025
@@ -238,6 +238,12 @@ export class Fiber {
|
||||
} else {
|
||||
if (fiber.shouldPatch) {
|
||||
component.__patch(component.__owl__.vnode!, fiber.vnode!);
|
||||
// When updating a Component's props (in directive),
|
||||
// the component has a pvnode AND should be patched.
|
||||
// However, its pvnode.elm may have changed if it is a High Order Component
|
||||
if (component.__owl__.pvnode) {
|
||||
component.__owl__.pvnode.elm = component.__owl__.vnode!.elm;
|
||||
}
|
||||
} else {
|
||||
component.__patch(document.createElement(fiber.vnode!.sel!), fiber.vnode!);
|
||||
component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm;
|
||||
|
||||
@@ -3015,6 +3015,55 @@ describe("random stuff/miscellaneous", () => {
|
||||
expect(env.qweb.templates[Parent.template].fn.toString()).toMatchSnapshot();
|
||||
expect(fixture.innerHTML).toBe("<div><span>42</span></div>");
|
||||
});
|
||||
|
||||
test("update props of component without concrete own node", async () => {
|
||||
class Custom extends Component {
|
||||
static template = xml`
|
||||
<div class="widget-subkey">
|
||||
<t t-esc="props.key"/>__<t t-esc="props.subKey"/>
|
||||
</div>`
|
||||
}
|
||||
class Child extends Component {
|
||||
static components = { Custom };
|
||||
static template = xml`
|
||||
<t t-component="Custom"
|
||||
t-key="props.subKey"
|
||||
key="props.key"
|
||||
subKey="props.subKey"/>`
|
||||
}
|
||||
|
||||
class Parent extends Component {
|
||||
static components = { Child };
|
||||
static template = xml`
|
||||
<div>
|
||||
<Child t-key="childProps.key" t-props="childProps"/>
|
||||
</div>`;
|
||||
childProps = {
|
||||
key: 1,
|
||||
subKey: 1,
|
||||
};
|
||||
}
|
||||
const parent = new Parent(null);
|
||||
await parent.mount(fixture);
|
||||
expect(fixture.textContent!.trim()).toBe('1__1');
|
||||
|
||||
// First step: change the Custom's instance
|
||||
Object.assign(parent.childProps, {
|
||||
subKey: 2,
|
||||
});
|
||||
parent.render();
|
||||
await nextTick();
|
||||
expect(fixture.textContent!.trim()).toBe('1__2');
|
||||
|
||||
// Second step, change both Child's and Custom's instance
|
||||
Object.assign(parent.childProps, {
|
||||
key: 2,
|
||||
subKey: 3,
|
||||
});
|
||||
parent.render();
|
||||
await nextTick();
|
||||
expect(fixture.textContent!.trim()).toBe('2__3');
|
||||
});
|
||||
});
|
||||
|
||||
describe("widget and observable state", () => {
|
||||
|
||||
Reference in New Issue
Block a user