From 8bc466d1ea95ec1bd45788846ca1b6a629f567e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Tue, 25 Aug 2026 12:38:08 +0000 Subject: [PATCH 1/2] [FIX] owl-core: let markup and defaults take a String Before this commit, `markup(value)` and the default value of a string type are typed as a `string`, so `markup(html)` and `t.string().optional(term)` do not compile with a `String` object, which both accept at runtime. This happens because only the vdom types both: `text()` takes a `string | String`, while these two take the primitive only. `stringType` tests `instanceof String` to validate a value, and `markup` passes its value to `new Markup()`, which converts it. This commit types both for a `String` object, so a `Markup` or a lazily translated term can be passed. `StringLike` only applies to a plain string type, so the default of a selection or of a number stays the same. --- packages/owl-core/src/types.ts | 8 +++++++- packages/owl-core/src/utils.ts | 4 ++-- packages/owl-core/tests/validation.test.ts | 5 +++++ packages/owl-runtime/tests/utils.test.ts | 5 +++++ packages/owl/tests/types_defaults.ts | 14 ++++++++++++++ packages/owl/tests/types_markup.ts | 20 ++++++++++++++++++++ 6 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 packages/owl/tests/types_markup.ts diff --git a/packages/owl-core/src/types.ts b/packages/owl-core/src/types.ts index 94853226b..20bb15d70 100644 --- a/packages/owl-core/src/types.ts +++ b/packages/owl-core/src/types.ts @@ -31,6 +31,10 @@ export type WithDefault = T & { [hasDefault]: T }; export declare const isOptional: unique symbol; export type Optional = T & { [isOptional]: T }; +// A string type also accepts a `String` object: `stringType` validates one, and +// the vdom renders it as text. +export type StringLike = [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 & { * 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; + optional( + value: T extends Function ? () => T : StringLike | (() => StringLike) + ): WithDefault; type: T; }; diff --git a/packages/owl-core/src/utils.ts b/packages/owl-core/src/utils.ts index 479dffbcc..314676094 100644 --- a/packages/owl-core/src/utils.ts +++ b/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)) { diff --git a/packages/owl-core/tests/validation.test.ts b/packages/owl-core/tests/validation.test.ts index e4492f447..9d26a61ce 100644 --- a/packages/owl-core/tests/validation.test.ts +++ b/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({ diff --git a/packages/owl-runtime/tests/utils.test.ts b/packages/owl-runtime/tests/utils.test.ts index c392c753b..c6d9ae04e 100644 --- a/packages/owl-runtime/tests/utils.test.ts +++ b/packages/owl-runtime/tests/utils.test.ts @@ -78,6 +78,11 @@ describe("markup", () => { const html = markup("Hello"); expect(html).toBeInstanceOf(Markup); }); + test("String object is flagged as safe, on its own value", () => { + const html = markup(new String("Hello")); + expect(html).toBeInstanceOf(Markup); + expect(html.toString()).toBe("Hello"); + }); describe("htmlEscape", () => { test("htmlEscape escapes text", () => { const res = htmlEscape("

test

"); diff --git a/packages/owl/tests/types_defaults.ts b/packages/owl/tests/types_defaults.ts index b9b139360..b48ef96f2 100644 --- a/packages/owl/tests/types_defaults.ts +++ b/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(); // 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) diff --git a/packages/owl/tests/types_markup.ts b/packages/owl/tests/types_markup.ts new file mode 100644 index 000000000..004a1fe47 --- /dev/null +++ b/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("a")); + +// @ts-expect-error a number is not a string +markup(1); +// @ts-expect-error an element is not a string +markup(document.createElement("div")); From 8340a11a47dfbf0f4ad13d3365e47384ca30131c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Tue, 25 Aug 2026 13:11:32 +0000 Subject: [PATCH 2/2] [FIX] owl-runtime: compare text and html by value Before this commit, `VText.patch`, `VHtml.patch` and the text slot of a compiled block compare the new value with `!==`, so two `String` objects holding the same text count as a change: a `Markup` built again on every render makes the html block re-parse its content and replace its nodes, losing the selection inside, and a text node has its data written again. This happens because an object only matches itself, and all three take a `String` object: `text()` is typed `string | String`, and `safeOutput` sends every `Markup` to `html()`. This commit compares the two values as text, which is what the DOM receives in the end. The same value keeps its fast path, and the conversion only runs for two values that are not the same object. `toText` also returns a string for a `String` object, as its signature says. --- .../owl-runtime/src/blockdom/block_compiler.ts | 9 ++++++++- packages/owl-runtime/src/blockdom/html.ts | 2 +- packages/owl-runtime/src/blockdom/text.ts | 12 ++++++++---- .../owl-runtime/tests/blockdom/block.test.ts | 17 +++++++++++++++++ .../owl-runtime/tests/blockdom/html.test.ts | 14 ++++++++++++++ .../owl-runtime/tests/blockdom/text.test.ts | 16 ++++++++++++++++ 6 files changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/owl-runtime/src/blockdom/block_compiler.ts b/packages/owl-runtime/src/blockdom/block_compiler.ts index 4c5c2ec9e..33b587de7 100644 --- a/packages/owl-runtime/src/blockdom/block_compiler.ts +++ b/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); + } +} diff --git a/packages/owl-runtime/src/blockdom/html.ts b/packages/owl-runtime/src/blockdom/html.ts index 429b2986d..f17cb1994 100644 --- a/packages/owl-runtime/src/blockdom/html.ts +++ b/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]; diff --git a/packages/owl-runtime/src/blockdom/text.ts b/packages/owl-runtime/src/blockdom/text.ts index 8e56348fc..41272d08a 100644 --- a/packages/owl-runtime/src/blockdom/text.ts +++ b/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) : ""; } } diff --git a/packages/owl-runtime/tests/blockdom/block.test.ts b/packages/owl-runtime/tests/blockdom/block.test.ts index 89bdd76f8..3d40ac7f8 100644 --- a/packages/owl-runtime/tests/blockdom/block.test.ts +++ b/packages/owl-runtime/tests/blockdom/block.test.ts @@ -39,6 +39,23 @@ describe("adding/patching blocks", () => { expect(fixture.innerHTML).toBe("

foo

"); }); + test("block text slot patched with an equal String object leaves the node alone", () => { + const block = createBlock("

"); + 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("

foo

"); + + patch(tree, block([new String("bar")])); + expect(observer.takeRecords()).toHaveLength(1); + expect(fixture.innerHTML).toBe("

bar

"); + observer.disconnect(); + }); + test("block with 2 dynamic text nodes", async () => { const block = createBlock("

"); const tree = block(["foo", "bar"]); diff --git a/packages/owl-runtime/tests/blockdom/html.test.ts b/packages/owl-runtime/tests/blockdom/html.test.ts index 5e398a06a..49a205e29 100644 --- a/packages/owl-runtime/tests/blockdom/html.test.ts +++ b/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("
coucou
"); }); + test("patching with an equal Markup keeps the content nodes", () => { + const tree = html(markup("foo") as any); + mount(tree, fixture); + const b = fixture.querySelector("b"); + + patch(tree, html(markup("foo") as any)); + expect(fixture.querySelector("b")).toBe(b); + + patch(tree, html(markup("bar") as any)); + expect(fixture.querySelector("b")).not.toBe(b); + expect(fixture.innerHTML).toBe("bar"); + }); + test("html vnode can be used as text", () => { mount(text(html("

a

") as any), fixture); expect(fixture.textContent).toBe("

a

"); diff --git a/packages/owl-runtime/tests/blockdom/text.test.ts b/packages/owl-runtime/tests/blockdom/text.test.ts index 55c7ce609..88c99fb0b 100644 --- a/packages/owl-runtime/tests/blockdom/text.test.ts +++ b/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"],