[IMP] app: allow duplicate templates iff they are the same

Previously, we used an option to allow duplicate templates, if that
option was passed, we would always replace the existing template with
the new template, and if it wasn't, we would always throw an error even
if the template's content was the same.

Allowing to replace a template with a different one is problematic, as
it may or may not have already been compiled, which may cause various
problems. On the other hand, if the template is the same, there is no
point in throwing an error, as we can just silently ignore the second
addition.

src/app/template_set.ts

@@ -79,18 +79,20 @@ export class TemplateSet {
     this.helpers = makeHelpers(this.getTemplate.bind(this));
   }
 
-  addTemplate(
-    name: string,
-    template: string | Element,
-    options: { allowDuplicate?: boolean } = {}
-  ) {
-    if (name in this.rawTemplates && !options.allowDuplicate) {
-      throw new Error(`Template ${name} already defined`);
+  addTemplate(name: string, template: string | Element) {
+    if (name in this.rawTemplates) {
+      const rawTemplate = this.rawTemplates[name];
+      const currentAsString = typeof rawTemplate === "string" ? rawTemplate : rawTemplate.outerHTML;
+      const newAsString = typeof template === "string" ? template : template.outerHTML;
+      if (currentAsString === newAsString) {
+        return;
+      }
+      throw new Error(`Template ${name} already defined with different content`);
     }
     this.rawTemplates[name] = template;
   }
 
-  addTemplates(xml: string | Document, options: { allowDuplicate?: boolean } = {}) {
+  addTemplates(xml: string | Document) {
     if (!xml) {
       // empty string
       return;
@@ -98,7 +100,7 @@ export class TemplateSet {
     xml = xml instanceof Document ? xml : parseXML(xml);
     for (const template of xml.querySelectorAll("[t-name]")) {
       const name = template.getAttribute("t-name")!;
-      this.addTemplate(name, template, options);
+      this.addTemplate(name, template);
     }
   }
 

tests/compiler/error_handling.test.ts

@@ -12,15 +12,6 @@ describe("error handling", () => {
     expect(() => context.renderToString("invalidname")).toThrow("Missing template");
   });
 
-  test("cannot add twice the same template", () => {
-    const context = new TestContext();
-    context.addTemplate("test", `<t></t>`);
-    expect(() => context.addTemplate("test", "<div/>", { allowDuplicate: true })).not.toThrow(
-      "already defined"
-    );
-    expect(() => context.addTemplate("test", "<div/>")).toThrow("already defined");
-  });
-
   test("addTemplates throw if parser error", () => {
     const context = new TestContext();
     expect(() => {

tests/compiler/validation.test.ts

@@ -11,11 +11,12 @@ describe("basic validation", () => {
     expect(() => context.getTemplate("invalidname")).toThrow("Missing template");
   });
 
-  test("cannot add twice the same template", () => {
+  test("cannot add a different template with the same name", () => {
     const context = new TemplateSet();
-    expect(() => context.addTemplate("test", "<div/>", { allowDuplicate: true })).not.toThrow(
-      "already defined"
-    );
+    context.addTemplate("test", `<t/>`);
+    // Same template with the same name is fine
+    expect(() => context.addTemplate("test", "<t/>")).not.toThrow();
+    // Different template with the same name crashes
     expect(() => context.addTemplate("test", "<div/>")).toThrow("already defined");
   });