[REL] v2.5.2

# v2.5.2

 - [FIX] runtime: make error recovery more robust

docs/owl.js

@@ -1625,6 +1625,13 @@ function makeRootFiber(node) {
             fibersInError.delete(current);
             fibersInError.delete(root);
             current.appliedToDom = false;
+            if (current instanceof RootFiber) {
+                // it is possible that this fiber is a fiber that crashed while being
+                // mounted, so the mounted list is possibly corrupted. We restore it to
+                // its normal initial state (which is empty list or a list with a mount
+                // fiber.
+                current.mounted = current instanceof MountFiber ? [current] : [];
+            }
         }
         return current;
     }
@@ -1741,6 +1748,7 @@ class RootFiber extends Fiber {
         const node = this.node;
         this.locked = true;
         let current = undefined;
+        let mountedFibers = this.mounted;
         try {
             // Step 1: calling all willPatch lifecycle hooks
             for (current of this.willPatch) {
@@ -1760,7 +1768,6 @@ class RootFiber extends Fiber {
             node._patch();
             this.locked = false;
             // Step 4: calling all mounted lifecycle hooks
-            let mountedFibers = this.mounted;
             while ((current = mountedFibers.pop())) {
                 current = current;
                 if (current.appliedToDom) {
@@ -1781,6 +1788,15 @@ class RootFiber extends Fiber {
             }
         }
         catch (e) {
+            // if mountedFibers is not empty, this means that a crash occured while
+            // calling the mounted hooks of some component. So, there may still be
+            // some component that have been mounted, but for which the mounted hooks
+            // have not been called. Here, we remove the willUnmount hooks for these
+            // specific component to prevent a worse situation (willUnmount being
+            // called even though mounted has not been called)
+            for (let fiber of mountedFibers) {
+                fiber.node.willUnmount = [];
+            }
             this.locked = false;
             node.app.handleError({ fiber: current || this, error: e });
         }
@@ -4934,10 +4950,10 @@ function parseTCustom(node, ctx) {
                 throw new OwlError(`Custom directive "${directiveName}" is not defined`);
             }
             const value = node.getAttribute(attr);
-            const modifier = attr.split(".").length > 1 ? attr.split(".")[1] : undefined;
+            const modifiers = attr.split(".").slice(1);
             node.removeAttribute(attr);
             try {
-                customDirective(node, value, modifier);
+                customDirective(node, value, modifiers);
             }
             catch (error) {
                 throw new OwlError(`Custom directive "${directiveName}" throw the following error: ${error}`);
@@ -5604,7 +5620,7 @@ function compile(template, options = {
 }
 
 // do not modify manually. This file is generated by the release script.
-const version = "2.5.0";
+const version = "2.5.2";
 
 // -----------------------------------------------------------------------------
 //  Scheduler
@@ -6082,6 +6098,6 @@ TemplateSet.prototype._compileTemplate = function _compileTemplate(name, templat
 export { App, Component, EventBus, OwlError, __info__, batched, blockDom, loadFile, markRaw, markup, mount, onError, onMounted, onPatched, onRendered, onWillDestroy, onWillPatch, onWillRender, onWillStart, onWillUnmount, onWillUpdateProps, reactive, status, toRaw, useChildSubEnv, useComponent, useEffect, useEnv, useExternalListener, useRef, useState, useSubEnv, validate, validateType, whenReady, xml };
 
 
-__info__.date = '2024-11-25T09:30:45.930Z';
-__info__.hash = '6b24864';
+__info__.date = '2024-12-02T15:51:07.157Z';
+__info__.hash = '1c5b6f2';
 __info__.url = 'https://github.com/odoo/owl';

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@odoo/owl",
-  "version": "2.5.0",
+  "version": "2.5.2",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@odoo/owl",
-  "version": "2.5.0",
+  "version": "2.5.2",
   "description": "Odoo Web Library (OWL)",
   "main": "dist/owl.cjs.js",
   "module": "dist/owl.es.js",

src/common/types.ts

@@ -1,4 +1,4 @@
 export type customDirectives = Record<
   string,
-  (node: Element, value: string, modifier?: string) => void
+  (node: Element, value: string, modifier: string[]) => void
 >;

src/compiler/parser.ts

@@ -299,10 +299,10 @@ function parseTCustom(node: Element, ctx: ParsingContext): AST | null {
         throw new OwlError(`Custom directive "${directiveName}" is not defined`);
       }
       const value = node.getAttribute(attr)!;
-      const modifier = attr.split(".").length > 1 ? attr.split(".")[1] : undefined;
+      const modifiers = attr.split(".").slice(1);
       node.removeAttribute(attr);
       try {
-        customDirective(node, value, modifier);
+        customDirective(node, value, modifiers);
       } catch (error) {
         throw new OwlError(
           `Custom directive "${directiveName}" throw the following error: ${error}`

src/runtime/fibers.ts

@@ -30,6 +30,13 @@ export function makeRootFiber(node: ComponentNode): Fiber {
       fibersInError.delete(current);
       fibersInError.delete(root);
       current.appliedToDom = false;
+      if (current instanceof RootFiber) {
+        // it is possible that this fiber is a fiber that crashed while being
+        // mounted, so the mounted list is possibly corrupted. We restore it to
+        // its normal initial state (which is empty list or a list with a mount
+        // fiber.
+        current.mounted = current instanceof MountFiber ? [current] : [];
+      }
     }
     return current;
   }
@@ -152,6 +159,7 @@ export class RootFiber extends Fiber {
     const node = this.node;
     this.locked = true;
     let current: Fiber | undefined = undefined;
+    let mountedFibers = this.mounted;
     try {
       // Step 1: calling all willPatch lifecycle hooks
       for (current of this.willPatch) {
@@ -173,7 +181,6 @@ export class RootFiber extends Fiber {
       this.locked = false;
 
       // Step 4: calling all mounted lifecycle hooks
-      let mountedFibers = this.mounted;
       while ((current = mountedFibers.pop())) {
         current = current;
         if (current.appliedToDom) {
@@ -194,6 +201,15 @@ export class RootFiber extends Fiber {
         }
       }
     } catch (e) {
+      // if mountedFibers is not empty, this means that a crash occured while
+      // calling the mounted hooks of some component. So, there may still be
+      // some component that have been mounted, but for which the mounted hooks
+      // have not been called. Here, we remove the willUnmount hooks for these
+      // specific component to prevent a worse situation (willUnmount being
+      // called even though mounted has not been called)
+      for (let fiber of mountedFibers) {
+        fiber.node.willUnmount = [];
+      }
       this.locked = false;
       node.app.handleError({ fiber: current || this, error: e });
     }

src/version.ts

@@ -1,2 +1,2 @@
 // do not modify manually. This file is generated by the release script.
-export const version = "2.5.0";
+export const version = "2.5.2";

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

@@ -14,7 +14,7 @@ exports[`t-custom can use t-custom directive on a node 1`] = `
 }"
 `;
 
-exports[`t-custom can use t-custom directive with modifier on a node 1`] = `
+exports[`t-custom can use t-custom directive with modifiers on a node 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
   let { text, createBlock, list, multi, html, toggler, comment } = bdom;

tests/compiler/t_custom.test.ts

@@ -31,25 +31,27 @@ describe("t-custom", () => {
     expect(steps).toEqual(["clicked"]);
   });
 
-  test("can use t-custom directive with modifier on a node", async () => {
+  test("can use t-custom directive with modifiers on a node", async () => {
     const steps: string[] = [];
     class SomeComponent extends Component {
-      static template = xml`<div t-custom-plop.mouse="click" class="my-div"/>`;
+      static template = xml`<div t-custom-plop.mouse.stop="click" class="my-div"/>`;
       click() {
         steps.push("clicked");
       }
     }
     const app = new App(SomeComponent, {
       customDirectives: {
-        plop: (node, value, modifier) => {
+        plop: (node, value, modifiers) => {
           node.setAttribute("t-on-click", value);
-          steps.push(modifier || "");
+          for (let mod of modifiers) {
+            steps.push(mod);
+          }
         },
       },
     });
     await app.mount(fixture);
     expect(fixture.innerHTML).toBe(`<div class="my-div"></div>`);
     fixture.querySelector("div")!.click();
-    expect(steps).toEqual(["mouse", "clicked"]);
+    expect(steps).toEqual(["mouse", "stop", "clicked"]);
   });
 });

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

@@ -1191,6 +1191,135 @@ exports[`can catch errors error in mounted on a component with a sibling (proper
 }"
 `;
 
+exports[`can catch errors error in onMounted, graceful recovery 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(null, false, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const Comp1 = ctx['component'];
+    return toggler(Comp1, comp1({}, (Comp1).name + key + \`__1\`, node, this, Comp1));
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, []);
+  const comp2 = app.createComponent(\`Boom\`, true, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`parent\`);
+    const b3 = comp1({}, key + \`__1\`, node, this, null);
+    const b4 = comp2({}, key + \`__2\`, node, this, null);
+    return multi([b2, b3, b4]);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`abc\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 4`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`boom\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 5`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`def\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(null, false, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2, b3;
+    b2 = text(\`R\`);
+    if (ctx['state'].gogogo) {
+      const Comp1 = ctx['component'];
+      b3 = toggler(Comp1, comp1({}, (Comp1).name + key + \`__1\`, node, this, Comp1));
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, []);
+  const comp2 = app.createComponent(\`Boom\`, true, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`parent\`);
+    const b3 = comp1({}, key + \`__1\`, node, this, null);
+    const b4 = comp2({}, key + \`__2\`, node, this, null);
+    return multi([b2, b3, b4]);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 4`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`abc\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 5`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`boom\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 6`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`def\`);
+  }
+}"
+`;
+
 exports[`can catch errors onError in class inheritance is called if rethrown 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

tests/components/error_handling.test.ts

@@ -1678,4 +1678,198 @@ describe("can catch errors", () => {
     `);
     expect(fixture.innerHTML).toBe("2");
   });
+
+  test("error in onMounted, graceful recovery", async () => {
+    class Child extends Component {
+      static template = xml`abc`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class OtherChild extends Component {
+      static template = xml`def`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Boom extends Component {
+      static template = xml`boom`;
+      setup() {
+        useLogLifecycle();
+        onMounted(() => {
+          throw new Error("boom");
+        });
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`parent<Child/><Boom/>`;
+      static components = { Child, Boom };
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Root extends Component {
+      static template = xml`<t t-component="component"/>`;
+
+      component: any = Parent;
+      setup() {
+        useLogLifecycle();
+        onError(() => {
+          logStep("error");
+          this.component = OtherChild;
+          this.render();
+        });
+      }
+    }
+
+    await mount(Root, fixture);
+    expect(fixture.innerHTML).toBe("def");
+
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "Root:setup",
+        "Root:willStart",
+        "Root:willRender",
+        "Parent:setup",
+        "Parent:willStart",
+        "Root:rendered",
+        "Parent:willRender",
+        "Child:setup",
+        "Child:willStart",
+        "Boom:setup",
+        "Boom:willStart",
+        "Parent:rendered",
+        "Child:willRender",
+        "Child:rendered",
+        "Boom:willRender",
+        "Boom:rendered",
+        "Boom:mounted",
+        "error",
+        "Root:willRender",
+        "OtherChild:setup",
+        "OtherChild:willStart",
+        "Root:rendered",
+        "OtherChild:willRender",
+        "OtherChild:rendered",
+        "OtherChild:mounted",
+        "Root:mounted",
+      ]
+    `);
+  });
+
+  test("error in onMounted, graceful recovery, variation", async () => {
+    class Child extends Component {
+      static template = xml`abc`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class OtherChild extends Component {
+      static template = xml`def`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Boom extends Component {
+      static template = xml`boom`;
+      setup() {
+        useLogLifecycle();
+        onMounted(() => {
+          throw new Error("boom");
+        });
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`parent<Child/><Boom/>`;
+      static components = { Child, Boom };
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Root extends Component {
+      static template = xml`R<t t-if="state.gogogo" t-component="component"/>`;
+
+      component: any = Parent;
+      state = useState({ gogogo: false });
+
+      setup() {
+        useLogLifecycle();
+        onError(() => {
+          logStep("error");
+          this.component = OtherChild;
+          this.render();
+        });
+      }
+    }
+
+    const root = await mount(Root, fixture);
+    expect(fixture.innerHTML).toBe("R");
+
+    // standard mounting process
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "Root:setup",
+        "Root:willStart",
+        "Root:willRender",
+        "Root:rendered",
+        "Root:mounted",
+      ]
+    `);
+
+    root.state.gogogo = true;
+    await nextTick();
+
+    expect(fixture.innerHTML).toBe("Rparentabcboom");
+    // rerender, root creates sub components, it crashes, tries to recover
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "Root:willRender",
+        "Parent:setup",
+        "Parent:willStart",
+        "Root:rendered",
+        "Parent:willRender",
+        "Child:setup",
+        "Child:willStart",
+        "Boom:setup",
+        "Boom:willStart",
+        "Parent:rendered",
+        "Child:willRender",
+        "Child:rendered",
+        "Boom:willRender",
+        "Boom:rendered",
+        "Root:willPatch",
+        "Boom:mounted",
+        "error",
+        "Root:willRender",
+        "OtherChild:setup",
+        "OtherChild:willStart",
+        "Root:rendered",
+      ]
+    `);
+
+    await nextTick();
+    expect(fixture.innerHTML).toBe("Rdef");
+
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "OtherChild:willRender",
+        "OtherChild:rendered",
+        "Root:willPatch",
+        "Child:willDestroy",
+        "Boom:willUnmount",
+        "Boom:willDestroy",
+        "Parent:willDestroy",
+        "OtherChild:mounted",
+        "Root:patched",
+      ]
+    `);
+  });
 });