[FIX] owl-core, owl-runtime: handle String objects

packages/owl-core/src/types.ts

@@ -31,6 +31,10 @@ export type WithDefault<T> = T & { [hasDefault]: T };
 export declare const isOptional: unique symbol;
 export type Optional<T> = T & { [isOptional]: T };
 
+// A string type also accepts a `String` object: `stringType` validates one, and
+// the vdom renders it as text.
+export type StringLike<T> = [string] extends [T] ? T | String : T;
+
 // Type-level brand carried by every type built by the `types` factories. It
 // is phantom: at runtime, only the `optional` method exists on the validator.
 export declare const typeBrand: unique symbol;
@@ -48,7 +52,9 @@ export type Type<T> = T & {
    * per consumer, so mutable defaults ([], {}) are not shared. A default for
    * a function type must use the factory form.
    */
-  optional(value: T extends Function ? () => T : T | (() => T)): WithDefault<T>;
+  optional(
+    value: T extends Function ? () => T : StringLike<T> | (() => StringLike<T>)
+  ): WithDefault<T>;
 
   type: T;
 };

packages/owl-core/src/utils.ts

@@ -92,9 +92,9 @@ export function htmlEscape(str: any): Markup {
  * If called as a tag function, the interpolated strings are escaped.
  */
 export function markup(strings: TemplateStringsArray, ...placeholders: unknown[]): Markup;
-export function markup(value: string): Markup;
+export function markup(value: string | String): Markup;
 export function markup(
-  valueOrStrings: string | TemplateStringsArray,
+  valueOrStrings: string | String | TemplateStringsArray,
   ...placeholders: unknown[]
 ): Markup {
   if (!Array.isArray(valueOrStrings)) {

packages/owl-core/tests/validation.test.ts

@@ -1006,6 +1006,11 @@ describe("applyDefaults", () => {
     expect(applyDefaults(undefined, t.number())).toBe(undefined);
   });
 
+  test("fills in a String object default as it is", () => {
+    const defaultValue = new String("abc");
+    expect(applyDefaults(undefined, t.string().optional(defaultValue))).toBe(defaultValue);
+  });
+
   test("fills in nested defaults without mutating the input", () => {
     const type = t.object({
       config: t.object({

packages/owl-runtime/src/blockdom/block_compiler.ts

@@ -368,7 +368,7 @@ function updateCtx(ctx: BlockCtx, tree: IntermediateTree) {
           idx: info.idx,
           refIdx: info.refIdx!,
           setData: setText,
-          updateData: setText,
+          updateData: updateText,
         });
         break;
       case "child":
@@ -673,3 +673,10 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
 function setText(this: Text, value: any) {
   characterDataSetData.call(this, toText(value));
 }
+
+function updateText(this: Text, value: any, oldValue: any) {
+  const data = toText(value);
+  if (data !== toText(oldValue)) {
+    characterDataSetData.call(this, data);
+  }
+}

packages/owl-runtime/src/blockdom/html.ts

@@ -49,7 +49,7 @@ class VHtml {
       return;
     }
     const html2 = other.html;
-    if (this.html !== html2) {
+    if (this.html !== html2 && String(this.html) !== String(html2)) {
       const parent = this.parentEl;
       // insert new html in front of current
       const afterNode = this.content[0];

packages/owl-runtime/src/blockdom/text.ts

@@ -48,10 +48,14 @@ class VText {
 
   patch(other: VText) {
     const text2 = other.text;
-    if (this.text !== text2) {
-      characterDataSetData.call(this.el!, toText(text2));
-      this.text = text2;
+    if (this.text === text2) {
+      return;
     }
+    const data = toText(text2);
+    if (data !== toText(this.text)) {
+      characterDataSetData.call(this.el!, data);
+    }
+    this.text = text2;
   }
 
   toString() {
@@ -72,6 +76,6 @@ export function toText(value: any): string {
     case "boolean":
       return value ? "true" : "false";
     default:
-      return value || "";
+      return value ? String(value) : "";
   }
 }

packages/owl-runtime/tests/blockdom/block.test.ts

@@ -39,6 +39,23 @@ describe("adding/patching blocks", () => {
     expect(fixture.innerHTML).toBe("<div><p>foo</p></div>");
   });
 
+  test("block text slot patched with an equal String object leaves the node alone", () => {
+    const block = createBlock("<div><p><block-text-0/></p></div>");
+    const tree = block([new String("foo")]);
+    mount(tree, fixture);
+    const observer = new MutationObserver(() => {});
+    observer.observe(fixture, { characterData: true, subtree: true });
+
+    patch(tree, block([new String("foo")]));
+    expect(observer.takeRecords()).toHaveLength(0);
+    expect(fixture.innerHTML).toBe("<div><p>foo</p></div>");
+
+    patch(tree, block([new String("bar")]));
+    expect(observer.takeRecords()).toHaveLength(1);
+    expect(fixture.innerHTML).toBe("<div><p>bar</p></div>");
+    observer.disconnect();
+  });
+
   test("block with 2 dynamic text nodes", async () => {
     const block = createBlock("<div><p><block-text-0/></p><span><block-text-1/></span></div>");
     const tree = block(["foo", "bar"]);

packages/owl-runtime/tests/blockdom/html.test.ts

@@ -1,4 +1,5 @@
 import { html, mount, patch, text } from "../../src/blockdom";
+import { markup } from "../../src/utils";
 import { makeTestFixture } from "./helpers";
 
 //------------------------------------------------------------------------------
@@ -29,6 +30,19 @@ describe("html block", () => {
     expect(fixture.innerHTML).toBe("<div>coucou</div>");
   });
 
+  test("patching with an equal Markup keeps the content nodes", () => {
+    const tree = html(markup("<b>foo</b>") as any);
+    mount(tree, fixture);
+    const b = fixture.querySelector("b");
+
+    patch(tree, html(markup("<b>foo</b>") as any));
+    expect(fixture.querySelector("b")).toBe(b);
+
+    patch(tree, html(markup("<b>bar</b>") as any));
+    expect(fixture.querySelector("b")).not.toBe(b);
+    expect(fixture.innerHTML).toBe("<b>bar</b>");
+  });
+
   test("html vnode can be used as text", () => {
     mount(text(html("<p>a</p>") as any), fixture);
     expect(fixture.textContent).toBe("<p>a</p>");

packages/owl-runtime/tests/blockdom/text.test.ts

@@ -33,6 +33,22 @@ describe("adding/patching text", () => {
     expect(fixture.innerHTML).toBe("bar");
   });
 
+  test("patching with an equal String object leaves the node alone", () => {
+    const tree = text(new String("foo"));
+    mount(tree, fixture);
+    const observer = new MutationObserver(() => {});
+    observer.observe(fixture, { characterData: true, subtree: true });
+
+    patch(tree, text(new String("foo")));
+    expect(observer.takeRecords()).toHaveLength(0);
+    expect(fixture.innerHTML).toBe("foo");
+
+    patch(tree, text(new String("bar")));
+    expect(observer.takeRecords()).toHaveLength(1);
+    expect(fixture.innerHTML).toBe("bar");
+    observer.disconnect();
+  });
+
   test("falsy values in text nodes", () => {
     const cases = [
       [false, "false"],

packages/owl-runtime/tests/utils.test.ts

@@ -78,6 +78,11 @@ describe("markup", () => {
     const html = markup("<blink>Hello</blink>");
     expect(html).toBeInstanceOf(Markup);
   });
+  test("String object is flagged as safe, on its own value", () => {
+    const html = markup(new String("<blink>Hello</blink>"));
+    expect(html).toBeInstanceOf(Markup);
+    expect(html.toString()).toBe("<blink>Hello</blink>");
+  });
   describe("htmlEscape", () => {
     test("htmlEscape escapes text", () => {
       const res = htmlEscape("<p>test</p>");

packages/owl/tests/types_defaults.ts

@@ -57,6 +57,20 @@ props({ p: t.number().optional(4) });
 props({ p: t.number().optional(() => 4) });
 // @ts-expect-error default must be a number
 props({ p: t.number().optional("4") });
+
+// a string default may be a String object, which the string type validates
+declare const stringObject: String;
+props({ p: t.string().optional(stringObject) });
+props({ p: t.string().optional(() => stringObject) });
+// @ts-expect-error a String object is not a number
+props({ p: t.number().optional(stringObject) });
+class StringObjectDefault {
+  props = props({ label: t.string().optional(stringObject) });
+}
+declare const stringObjectDefault: StringObjectDefault;
+void stringObjectDefault;
+// the reader still gets a string, as the default reads as one
+assertEq<typeof stringObjectDefault.props.label, string>();
 // a default for a function type must use the factory form
 props({ cb: t.function().optional(() => () => {}) });
 // @ts-expect-error a plain function default is rejected (factory form only)

packages/owl/tests/types_markup.ts

@@ -0,0 +1,20 @@
+// Compile-time checks for the values markup() accepts. This file is only
+// typechecked (npm run test:types); it is not executed.
+import { markup } from "../src";
+
+declare const str: string;
+declare const stringObject: String;
+
+// as a tag function, and on a plain string
+markup`a ${str} b`;
+markup(str);
+
+// a String object is accepted, as the constructor converts it: a Markup passes
+// through, and so does a lazily translated term
+markup(stringObject);
+markup(markup("<b>a</b>"));
+
+// @ts-expect-error a number is not a string
+markup(1);
+// @ts-expect-error an element is not a string
+markup(document.createElement("div"));