Skip to content

Commit

Permalink
[IMP] component: wait for parent rendering to be complete before doin…
Browse files Browse the repository at this point in the history
…g child

This is a breaking semantic change.  With this commit, the UI is frozen
whenever owl is waiting for a parent to change

Also, this allows Owl not to render components that will be removed
later.
  • Loading branch information
ged-odoo authored and sdegueldre committed Mar 29, 2022
1 parent 828be28 commit e3b1566
Show file tree
Hide file tree
Showing 7 changed files with 696 additions and 60 deletions.
30 changes: 7 additions & 23 deletions src/component/component_node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,16 @@ import type { App, Env } from "../app/app";
import { BDom, VNode } from "../blockdom";
import {
clearReactivesForCallback,
getSubscriptions,
NonReactive,
Reactive,
reactive,
TARGET,
NonReactive,
getSubscriptions,
} from "../reactivity";
import { batched, Callback } from "../utils";
import { Component, ComponentConstructor } from "./component";
import { fibersInError, handleError } from "./error_handling";
import {
Fiber,
makeChildFiber,
makeRootFiber,
MountFiber,
MountOptions,
RootFiber,
} from "./fibers";
import { Fiber, makeChildFiber, makeRootFiber, MountFiber, MountOptions } from "./fibers";
import { applyDefaultProps } from "./props_validation";
import { STATUS } from "./status";

Expand Down Expand Up @@ -126,6 +119,7 @@ export function component<P extends object>(

node.initiateRender(new Fiber(node, parentFiber));
}
parentFiber.root!.reachedChildren.add(node);
return node;
}

Expand Down Expand Up @@ -193,7 +187,7 @@ export class ComponentNode<P extends object = any, E = any> implements VNode<Com
return;
}
if (this.status === STATUS.NEW && this.fiber === fiber) {
this._render(fiber);
fiber.render();
}
}

Expand Down Expand Up @@ -238,17 +232,7 @@ export class ComponentNode<P extends object = any, E = any> implements VNode<Com
// embedded in a rendering coming from above, so the fiber will be rendered
// in the next microtick anyway, so we should not render it again.
if (this.fiber === fiber && (current || !fiber.parent)) {
this._render(fiber);
}
}

_render(fiber: Fiber | RootFiber) {
try {
fiber.bdom = this.renderFn();
const root = fiber.root!;
root.setCounter(root.counter - 1);
} catch (e) {
handleError({ node: this, error: e });
fiber.render();
}
}

Expand Down Expand Up @@ -292,7 +276,7 @@ export class ComponentNode<P extends object = any, E = any> implements VNode<Com
return;
}
component.props = props;
this._render(fiber);
fiber.render();
const parentRoot = parentFiber.root!;
if (this.willPatch.length) {
parentRoot.willPatch.push(fiber);
Expand Down
58 changes: 58 additions & 0 deletions src/component/fibers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ export function makeChildFiber(node: ComponentNode, parent: Fiber): Fiber {
if (current) {
cancelFibers(current.children);
current.root = null;
if (current instanceof RootFiber && current.delayedRenders.length) {
let root = parent.root!;
root.delayedRenders = root.delayedRenders.concat(current.delayedRenders);
}
}
return new Fiber(node, parent);
}
Expand All @@ -19,6 +23,9 @@ export function makeRootFiber(node: ComponentNode): Fiber {
root.setCounter(root.counter + 1 - cancelFibers(current.children));
current.children = [];
current.bdom = null;
if (current === root) {
root.reachedChildren = new WeakSet();
}
if (fibersInError.has(current)) {
fibersInError.delete(current);
fibersInError.delete(root);
Expand Down Expand Up @@ -81,6 +88,46 @@ export class Fiber {
this.root = this as any;
}
}

render() {
// if some parent has a fiber => register in followup
let prev = this.root!.node;
let current = prev.parent;
while (current) {
if (current.fiber) {
let root = current.fiber.root!;
if (root.counter) {
root.delayedRenders.push(this);
return;
} else {
if (!root.reachedChildren.has(prev)) {
// is dead
this.node.app.scheduler.shouldClear = true;
return;
}
current = root.node;
}
}
prev = current;
current = current.parent;
}

// there are no current rendering from above => we can render
this._render();
}

_render() {
const node = this.node;
const root = this.root;
if (root) {
try {
this.bdom = node.renderFn();
root.setCounter(root.counter - 1);
} catch (e) {
handleError({ node, error: e });
}
}
}
}

export class RootFiber extends Fiber {
Expand All @@ -94,6 +141,9 @@ export class RootFiber extends Fiber {
// i.e.: render triggered in onWillUnmount or in willPatch will be delayed
locked: boolean = false;

delayedRenders: Fiber[] = [];
reachedChildren: WeakSet<ComponentNode> = new WeakSet();

complete() {
const node = this.node;
this.locked = true;
Expand Down Expand Up @@ -148,6 +198,14 @@ export class RootFiber extends Fiber {
setCounter(newValue: number) {
this.counter = newValue;
if (newValue === 0) {
if (this.delayedRenders.length) {
for (let f of this.delayedRenders) {
if (f.root) {
f.render();
}
}
this.delayedRenders = [];
}
this.node.app.scheduler.flush();
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/component/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export class Scheduler {
tasks: Set<RootFiber> = new Set();
requestAnimationFrame: Window["requestAnimationFrame"];
frame: number = 0;
shouldClear: boolean = false;

constructor() {
this.requestAnimationFrame = Scheduler.requestAnimationFrame;
Expand All @@ -31,6 +32,14 @@ export class Scheduler {
this.frame = this.requestAnimationFrame(() => {
this.frame = 0;
this.tasks.forEach((fiber) => this.processFiber(fiber));
if (this.shouldClear) {
this.shouldClear = false;
for (let task of this.tasks) {
if (task.node.status === STATUS.DESTROYED) {
this.tasks.delete(task);
}
}
}
});
}
}
Expand Down
178 changes: 178 additions & 0 deletions tests/components/__snapshots__/concurrency.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,184 @@ exports[`delay willUpdateProps with rendering grandchild 4`] = `
}"
`;

exports[`delayed rendering, but then initial rendering is cancelled by yet another render 1`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
return component(\`B\`, {value: ctx['state'].value}, key + \`__1\`, node, ctx);
}
}"
`;

exports[`delayed rendering, but then initial rendering is cancelled by yet another render 2`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
return component(\`C\`, {value: ctx['state'].someValue+ctx['props'].value}, key + \`__1\`, node, ctx);
}
}"
`;

exports[`delayed rendering, but then initial rendering is cancelled by yet another render 3`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

let block3 = createBlock(\`<p><block-text-0/></p>\`);

return function template(ctx, node, key = \\"\\") {
const b2 = component(\`D\`, {}, key + \`__1\`, node, ctx);
let txt1 = ctx['props'].value;
const b3 = block3([txt1]);
return multi([b2, b3]);
}
}"
`;

exports[`delayed rendering, but then initial rendering is cancelled by yet another render 4`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

let block1 = createBlock(\`<button block-handler-0=\\"click\\"><block-text-1/></button>\`);

return function template(ctx, node, key = \\"\\") {
let hdlr1 = [ctx['increment'], ctx];
let txt1 = ctx['state'].val;
return block1([hdlr1, txt1]);
}
}"
`;

exports[`delayed rendering, reusing fiber and stuff 1`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
return component(\`B\`, {value: ctx['state'].value}, key + \`__1\`, node, ctx);
}
}"
`;

exports[`delayed rendering, reusing fiber and stuff 2`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
const b2 = text(ctx['props'].value);
const b3 = component(\`C\`, {}, key + \`__1\`, node, ctx);
return multi([b2, b3]);
}
}"
`;

exports[`delayed rendering, reusing fiber and stuff 3`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

let block1 = createBlock(\`<button block-handler-0=\\"click\\"><block-text-1/></button>\`);

return function template(ctx, node, key = \\"\\") {
let hdlr1 = [ctx['increment'], ctx];
let txt1 = ctx['state'].val;
return block1([hdlr1, txt1]);
}
}"
`;

exports[`delayed rendering, reusing fiber then component is destroyed and stuff 1`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
let b2,b3;
b2 = text(\`A\`);
if (ctx['state'].value<15) {
b3 = component(\`B\`, {value: ctx['state'].value}, key + \`__1\`, node, ctx);
}
return multi([b2, b3]);
}
}"
`;

exports[`delayed rendering, reusing fiber then component is destroyed and stuff 2`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
const b2 = text(ctx['props'].value);
const b3 = component(\`C\`, {}, key + \`__1\`, node, ctx);
return multi([b2, b3]);
}
}"
`;

exports[`delayed rendering, reusing fiber then component is destroyed and stuff 3`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

let block1 = createBlock(\`<button block-handler-0=\\"click\\"><block-text-1/></button>\`);

return function template(ctx, node, key = \\"\\") {
let hdlr1 = [ctx['increment'], ctx];
let txt1 = ctx['state'].val;
return block1([hdlr1, txt1]);
}
}"
`;

exports[`delayed rendering, then component is destroyed and stuff 1`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
return component(\`B\`, {value: ctx['state'].value}, key + \`__1\`, node, ctx);
}
}"
`;

exports[`delayed rendering, then component is destroyed and stuff 2`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

return function template(ctx, node, key = \\"\\") {
let b2,b3;
b2 = text(ctx['props'].value);
if (ctx['props'].value<10) {
b3 = component(\`C\`, {}, key + \`__1\`, node, ctx);
}
return multi([b2, b3]);
}
}"
`;

exports[`delayed rendering, then component is destroyed and stuff 3`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;

let block1 = createBlock(\`<button block-handler-0=\\"click\\"><block-text-1/></button>\`);

return function template(ctx, node, key = \\"\\") {
let hdlr1 = [ctx['increment'], ctx];
let txt1 = ctx['state'].val;
return block1([hdlr1, txt1]);
}
}"
`;

exports[`destroying/recreating a subcomponent, other scenario 1`] = `
"function anonymous(bdom, helpers
) {
Expand Down
Loading

0 comments on commit e3b1566

Please sign in to comment.