[FIX] runtime/fibers: render a parent when self is in error

Have a Component A which instantiate another one (B) which also have some Children (C).
Have B be the one to handle the errors from the Children via the onError hook.
This hook should call a props coming from A. The real error handler is on A then, passed to B via props.

This handler naturally sets a flag on A, and triggers a render.

Before this commit, the mounting of A never resolved as the fiber that was recycled in A
was still part of a root one which was flagged as being in error.

After this commit, the A component mounts correctly.

Closes #1298

doc/reference/reactivity.md

@@ -16,8 +16,8 @@ according to state changes, and to do so in a performant manner.
 
 To this end, Owl provides a proxy-based reactivity system, based on the `reactive` primitive.
 The `reactive` function takes an object as a first argument, and an optional callback as its second
-argument, it will return a proxy of the object. This proxy will track what properties are read
-through the proxy, and call the provided callback whenever one of these properties is changed
+argument, it returns a proxy of the object. This proxy tracks what properties are read
+through the proxy, and calls the provided callback whenever one of these properties is changed
 through any reactive version of the same object. It does so in depth, by returning reactive versions
 of the subobjects when they are read.
 
@@ -103,7 +103,7 @@ only in debug mode in the future.
 
 The `reactive` function is the basic reactivity primitive. It takes an object
 or an array as first argument, and optionally, a function as the second argument.
-The function will be called whenever any tracked value is updated.
+The function is called whenever any tracked value is updated.
 
 ```js
 const obj = reactive({ a: 1 }, () => console.log("changed"));
@@ -180,7 +180,7 @@ rendered output, and so we can ignore it.
 
 The reactivity system has special support built-in for the standard container types `Map` and `Set`.
 They behave like one would expect: reading a key subscribes the observer to that key, adding or
-removing an item to them will notify observers that have used any of the iterators on that reactive
+removing an item to them notifies observers that have used any of the iterators on that reactive
 object, such as `.entries()` or `.keys()`, likewise with clearing them.
 
 ## Escape hatches
@@ -223,9 +223,9 @@ in the template:
 <t t-foreach="items" t-as="item" t-key="item.label" t-esc="item.label + item.value"/>
 ```
 
-Here, on every render, we will go and read one thousand keys from a reactive object, which will
-cause one thousand reactive objects to be created. If we know that the content of these objects
-cannot change, this is wasted work. If instead all of these objects are marked as raw, we will avoid
+Here, on every render, we go and read one thousand keys from a reactive object, which causes
+one thousand reactive objects to be created. If we know that the content of these objects
+cannot change, this is wasted work. If instead all of these objects are marked as raw, we avoid
 all of this work while keeping the ability to lean on the reactivity to track the presence and
 identity of these objects:
 
@@ -303,14 +303,14 @@ export function addNotification(label) {
 Here, the `notifications` variable is a reactive object. Notice how we didn't give `reactive` a
 callback: this is because in this case, all we care about is that adding or removing notifications
 in the `addNotification` function goes through the reactivity system. The `NotificationContainer`
-component reobserves this object with `useState`, and will be updated whenever notifications are
+component reobserves this object with `useState`, and is updated whenever notifications are
 added or removed.
 
 ### Store
 
 Centralizing application state is a pretty common want/need in web applications. Because of the way
 the reactivity system works, you can treat any reactive object as a store, and if you call `useState`
-on it, components will automatically observe only the part of the store that they're interested in:
+on it, components automatically observe only the part of the store that they're interested in:
 
 ```js
 export const store = reactive({

src/compiler/code_generator.ts

@@ -294,7 +294,7 @@ export class CodeGenerator {
       for (let block of this.blocks) {
         if (block.dom) {
           let xmlString = block.asXmlString();
-          xmlString = xmlString.replace(/`/g, "\\`");
+          xmlString = xmlString.replace(/\\/g, "\\\\").replace(/`/g, "\\`");
           if (block.dynamicTagName) {
             xmlString = xmlString.replace(/^<\w+/, `<\${tag || '${block.dom.nodeName}'}`);
             xmlString = xmlString.replace(/\w+>$/, `\${tag || '${block.dom.nodeName}'}>`);

src/runtime/fibers.ts

@@ -25,7 +25,7 @@ export function makeRootFiber(node: ComponentNode): Fiber {
     current.children = [];
     current.childrenMap = {};
     current.bdom = null;
-    if (fibersInError.has(current)) {
+    if (fibersInError.has(root)) {
       fibersInError.delete(current);
       fibersInError.delete(root);
       current.appliedToDom = false;

tests/compiler/__snapshots__/attributes.test.ts.snap

@@ -430,6 +430,19 @@ exports[`attributes static attributes on void elements 1`] = `
 }"
 `;
 
+exports[`attributes static attributes with backslash or backtick 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div foo=\\"\\\\\\\\a\\" bar=\\"\\\\\\\\n\\" baz=\\"\\\\\`\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return block1();
+  }
+}"
+`;
+
 exports[`attributes static attributes with backticks 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

tests/compiler/attributes.test.ts

@@ -12,6 +12,11 @@ describe("attributes", () => {
     expect(renderToString(template)).toBe(`<div foo="a" bar="b" baz="c"></div>`);
   });
 
+  test("static attributes with backslash or backtick", () => {
+    const template = `<div foo="\\a" bar="\\n" baz="\`"/>`;
+    expect(renderToString(template)).toBe(`<div foo="\\a" bar="\\n" baz="\`"></div>`);
+  });
+
   test("two classes", () => {
     const template = `<div class="a b"/>`;
     expect(renderToString(template)).toBe(`<div class="a b"></div>`);

tests/components/__snapshots__/error_handling.test.ts.snap

@@ -1195,6 +1195,131 @@ exports[`can catch errors onError in class inheritance is not called if no rethr
 }"
 `;
 
+exports[`can catch errors re-render parent when self is in error - 2 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Classic\`, true, false, false, false);
+  const comp2 = app.createComponent(\`Classic\`, true, false, false, false);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = comp1({hasBoom: true,state: ctx['reactive']}, key + \`__1\`, node, this, null);
+    const b3 = comp2({hasBoom: false,state: ctx['reactive']}, key + \`__2\`, node, this, null);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error - 2 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`BoomWrapper\`, true, false, false, false);
+  
+  let block3 = createBlock(\`<div><block-text-0/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    if (ctx['props'].hasBoom) {
+      b2 = comp1({state: ctx['props'].state}, key + \`__1\`, node, this, null);
+    } else {
+      let txt1 = ctx['props'].state.safeTree;
+      b3 = block3([txt1]);
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error - 2 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Boom\`, true, false, false, true);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    if (ctx['state'].errorTree==='error') {
+      b2 = comp1({}, key + \`__1\`, node, this, null);
+    } else {
+      b3 = text(ctx['state'].errorTree);
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error - 2 4`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return block1();
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Classic\`, true, false, false, true);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return comp1({}, key + \`__1\`, node, this, null);
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { bind } = helpers;
+  const comp1 = app.createComponent(\`BoomWrapper\`, true, false, false, false);
+  
+  let block3 = createBlock(\`<div/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    if (!ctx['inError']) {
+      b2 = comp1({onError: bind(this, ctx['onErrorAsProps'])}, key + \`__1\`, node, this, null);
+    } else {
+      b3 = block3();
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Boom\`, true, false, false, true);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return comp1({}, key + \`__1\`, node, this, null);
+  }
+}"
+`;
+
+exports[`can catch errors re-render parent when self is in error 4`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return block1();
+  }
+}"
+`;
+
 exports[`errors and promises a rendering error in a sub component will reject the mount promise 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

tests/components/error_handling.test.ts

@@ -9,6 +9,7 @@ import {
   onRendered,
   onWillUnmount,
   useState,
+  reactive,
   xml,
 } from "../../src/index";
 import {
@@ -785,6 +786,131 @@ describe("can catch errors", () => {
     expect(mockConsoleWarn).toBeCalledTimes(0);
   });
 
+  test("re-render parent when self is in error", async () => {
+    class Boom extends Component {
+      static template = xml`<div />`;
+      setup() {
+        onWillStart(() => {
+          throw new Error("Boom Error");
+        });
+      }
+    }
+
+    const steps: string[] = [];
+    class BoomWrapper extends Component {
+      static template = xml`<Boom />`;
+      static components = { Boom };
+      setup() {
+        onError(() => {
+          steps.push("onError in child");
+          this.props.onError();
+        });
+      }
+    }
+
+    class Classic extends Component {
+      static template = xml`<BoomWrapper t-if="!inError" onError.bind="onErrorAsProps"/><div t-else="" />`;
+      static components = { BoomWrapper };
+      inError: Boolean = false;
+      setup() {
+        onMounted(() => {
+          steps.push("mounted");
+        });
+      }
+      onErrorAsProps() {
+        this.inError = true;
+        this.render(true);
+      }
+    }
+
+    class App extends Component {
+      static template = xml`<Classic />`;
+      static components = { Classic };
+    }
+
+    await mount(App, fixture);
+    expect(steps).toEqual(["onError in child", "mounted"]);
+  });
+
+  test("re-render parent when self is in error - 2", async () => {
+    class Boom extends Component {
+      static template = xml`<div />`;
+      setup() {
+        onWillStart(() => {
+          throw new Error("Boom Error");
+        });
+      }
+    }
+
+    const steps: string[] = [];
+    class BoomWrapper extends Component {
+      static template = xml`<Boom t-if="state.errorTree === 'error'" /><t t-else="" t-esc="state.errorTree" />`;
+      static components = { Boom };
+      state: any;
+      setup() {
+        this.state = useState(this.props.state);
+        onError(() => {
+          steps.push("onError");
+          this.state.onError();
+        });
+
+        onWillRender(() => {
+          steps.push(`BoomWrapper willRender`);
+        });
+      }
+    }
+
+    let classicId = 0;
+    class Classic extends Component {
+      static template = xml`
+        <BoomWrapper t-if="props.hasBoom" state="props.state"/>
+        <div t-else="" t-esc="props.state.safeTree" />
+      `;
+      static components = { BoomWrapper };
+      id: Number = 0;
+      state: any;
+      inError: Boolean = false;
+      setup() {
+        this.id = classicId++;
+        onMounted(() => {
+          steps.push(`mounted ${this.id}`);
+        });
+        onWillRender(() => {
+          steps.push(`Classic willRender ${this.id}`);
+        });
+      }
+    }
+
+    class App extends Component {
+      reactive: any;
+      setup() {
+        this.reactive = reactive({
+          errorTree: "error",
+          safeTree: "safe",
+          onError() {
+            this.safeTree = "safe2";
+            this.errorTree = "errorHandled";
+          },
+        });
+      }
+      static template = xml`<Classic hasBoom="true" state="reactive" /><Classic hasBoom="false" state="reactive"/>`;
+      static components = { Classic };
+    }
+
+    await mount(App, fixture);
+    expect(steps).toEqual([
+      "Classic willRender 0",
+      "Classic willRender 1",
+      "BoomWrapper willRender",
+      "onError",
+      "Classic willRender 1",
+      "BoomWrapper willRender",
+      "mounted 1",
+      "mounted 0",
+    ]);
+    expect(fixture.innerHTML).toBe("errorHandled<div>safe2</div>");
+  });
+
   test("can catch an error in the willStart call", async () => {
     class ErrorComponent extends Component {
       static template = xml`<div>Some text</div>`;

tests/components/t_call.test.ts

@@ -316,7 +316,7 @@ describe("t-call", () => {
     expect(fixture.innerHTML).toBe("childaaronchildlucas");
   });
 
-  test.only("t-call-context: ComponentNode is not looked up in the context", async () => {
+  test("t-call-context: ComponentNode is not looked up in the context", async () => {
     let child: any;
     class Child extends Component {
       static template = xml`<t t-slot="default"/>`;