diff --git a/change/@microsoft-fast-element-92fae02f-bbcf-4b67-87b0-301d74cd58c0.json b/change/@microsoft-fast-element-92fae02f-bbcf-4b67-87b0-301d74cd58c0.json new file mode 100644 index 00000000000..4c20fc92227 --- /dev/null +++ b/change/@microsoft-fast-element-92fae02f-bbcf-4b67-87b0-301d74cd58c0.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: reconcile prerendered content that a content binding does not adopt while hydrating, so content inside a when() whose condition has not resolved is no longer left in the DOM unowned and then duplicated once it does.", + "packageName": "@microsoft/fast-element", + "email": "144495202+AKnassa@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/change/@microsoft-fast-html-fcbdf1a0-f598-4dca-99ac-7936667294ee.json b/change/@microsoft-fast-html-fcbdf1a0-f598-4dca-99ac-7936667294ee.json new file mode 100644 index 00000000000..ffe419a5ab1 --- /dev/null +++ b/change/@microsoft-fast-html-fcbdf1a0-f598-4dca-99ac-7936667294ee.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "test: add a hydration fixture covering a deferred child rendered inside a conditional content binding.", + "packageName": "@microsoft/fast-html", + "email": "144495202+AKnassa@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/packages/fast-element/src/templating/html-binding-directive.ts b/packages/fast-element/src/templating/html-binding-directive.ts index 196a2f34f92..c69cf3ae2c4 100644 --- a/packages/fast-element/src/templating/html-binding-directive.ts +++ b/packages/fast-element/src/templating/html-binding-directive.ts @@ -91,6 +91,59 @@ function isContentTemplate(value: any): value is ContentTemplate { return value.create !== undefined; } +/** + * Discards the prerendered nodes of a content binding that the binding did not adopt. + * + * A content binding hydrates by adopting the nodes the server rendered between its + * boundary markers. When it cannot adopt them - because the value is no longer a + * template, or because the view has already hydrated - those nodes must be reconciled + * away, just as `repeat` discards the prerendered item views it does not adopt. + * + * Left in place they are owned by no view, so nothing ever binds them: their bindings + * are never wired to the source, and once the value does resolve, a second, + * client-rendered copy is composed alongside them. + * + * The boundary is consumed, so a range is discarded at most once. + */ +function discardUnadoptedContent( + directive: HTMLBindingDirective, + controller: ViewController, +): void { + if (!isHydratable(controller)) { + return; + } + + const viewNodes = controller.bindingViewBoundaries[directive.targetNodeId]; + + if (viewNodes === undefined) { + return; + } + + delete controller.bindingViewBoundaries[directive.targetNodeId]; + + const { first, last } = viewNodes; + const parent = first.parentNode; + + if (parent === null) { + return; + } + + let current: Node | null = first; + + while (current !== null) { + const next: Node | null = current.nextSibling; + const isLast = current === last; + + parent.removeChild(current); + + if (isLast) { + return; + } + + current = next; + } +} + /** * Sink function for DOMAspect.content bindings (text content interpolation). * Handles two cases: @@ -127,8 +180,12 @@ function updateContent( controller.hydrationStage !== HydrationStage.hydrated ) { const viewNodes = controller.bindingViewBoundaries[this.targetNodeId]; + delete controller.bindingViewBoundaries[this.targetNodeId]; view = value.hydrate(viewNodes.first, viewNodes.last); } else { + // The prerendered nodes cannot be adopted, so reconcile them away + // before rendering the value on the client. + discardUnadoptedContent(this, controller); view = value.create(); } } else { @@ -163,15 +220,23 @@ function updateContent( // If there is a view and it's currently composed into // the DOM, then we need to remove it. - if (view !== void 0 && view.isComposed) { - view.isComposed = false; - view.remove(); - - if (view.needsBindOnly) { - view.needsBindOnly = false; - } else { - view.unbind(); + if (view !== void 0) { + if (view.isComposed) { + view.isComposed = false; + view.remove(); + + if (view.needsBindOnly) { + view.needsBindOnly = false; + } else { + view.unbind(); + } } + } else { + // No view ever composed this target, so any prerendered nodes between the + // binding's boundary markers are owned by nothing. This happens when the + // value is falsy on hydration but the server rendered content for it - + // a `when` whose condition has not resolved yet, for instance. + discardUnadoptedContent(this, controller); } target.textContent = value; diff --git a/packages/fast-html/test/fixtures/when-nested-elements/entry.html b/packages/fast-html/test/fixtures/when-nested-elements/entry.html new file mode 100644 index 00000000000..c410043fe15 --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/entry.html @@ -0,0 +1,15 @@ + + + + + + When Nested Elements Hydration Tests + + + + + + + + + diff --git a/packages/fast-html/test/fixtures/when-nested-elements/fast-build.config.json b/packages/fast-html/test/fixtures/when-nested-elements/fast-build.config.json new file mode 100644 index 00000000000..c291972cdad --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/fast-build.config.json @@ -0,0 +1,6 @@ +{ + "entry": "entry.html", + "state": "state.json", + "output": "index.html", + "templates": "templates.html" +} diff --git a/packages/fast-html/test/fixtures/when-nested-elements/index.html b/packages/fast-html/test/fixtures/when-nested-elements/index.html new file mode 100644 index 00000000000..7283a374b4a --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/index.html @@ -0,0 +1,73 @@ + + + + + + When Nested Elements Hydration Tests + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/fast-html/test/fixtures/when-nested-elements/main.ts b/packages/fast-html/test/fixtures/when-nested-elements/main.ts new file mode 100644 index 00000000000..14cb8103af0 --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/main.ts @@ -0,0 +1,123 @@ +import { attr, FASTElement, observable } from "@microsoft/fast-element"; +import { RenderableFASTElement, TemplateElement } from "@microsoft/fast-html"; + +const RenderableElement = RenderableFASTElement(FASTElement); + +// Mock data source - simulating fetched data, matching the pre-rendered state. +const mockDataSource = { + getData() { + return { show: true, text: "Hello", items: [{ text: "Hello" }] }; + }, +}; + +/** + * Renders a `defer-and-hydrate` child inside a conditional content binding. The + * condition is true at render time, so the pre-rendered DOM contains the child. + */ +export class WhenParent extends RenderableElement { + @observable + public show: boolean = false; + + @observable + public text: string = ""; + + async prepare() { + const data = mockDataSource.getData(); + + this.show = data.show; + this.text = data.text; + } +} + +/** + * The control: the same child rendered through a repeat binding instead. Swapping + * the surrounding directive is the only difference between this and WhenParent. + */ +export class RepeatParent extends RenderableElement { + @observable + public items: Array<{ text: string }> = []; + + async prepare() { + this.items = mockDataSource.getData().items; + } +} + +/** + * The condition was true when the page was rendered, so the pre-rendered DOM + * contains the child, but the value backing the condition has not resolved by the + * time the element hydrates - as happens when the condition depends on data that + * arrives after hydration. The pre-rendered content must be reconciled away rather + * than left behind, and must not be duplicated once the value does resolve. + */ +export class WhenStaleParent extends RenderableElement { + @observable + public show: boolean = false; + + @observable + public text: string = "Hello"; +} + +/** + * The control for {@link WhenStaleParent}: a repeat binding reconciles the same + * pre-rendered/hydration mismatch correctly. + */ +export class RepeatStaleParent extends RenderableElement { + @observable + public items: Array<{ text: string }> = []; +} + +export class DeferredChild extends RenderableElement { + @attr + public text: string = ""; +} + +RenderableFASTElement(WhenParent).defineAsync({ + name: "when-parent", + templateOptions: "defer-and-hydrate", +}); + +RenderableFASTElement(RepeatParent).defineAsync({ + name: "repeat-parent", + templateOptions: "defer-and-hydrate", +}); + +RenderableFASTElement(WhenStaleParent).defineAsync({ + name: "when-stale-parent", + templateOptions: "defer-and-hydrate", +}); + +RenderableFASTElement(RepeatStaleParent).defineAsync({ + name: "repeat-stale-parent", + templateOptions: "defer-and-hydrate", +}); + +RenderableFASTElement(DeferredChild).defineAsync({ + name: "deferred-child", + templateOptions: "defer-and-hydrate", +}); + +TemplateElement.options({ + "when-parent": { + observerMap: "all", + }, + "repeat-parent": { + observerMap: "all", + }, + "when-stale-parent": { + observerMap: "all", + }, + "repeat-stale-parent": { + observerMap: "all", + }, + "deferred-child": { + observerMap: "all", + }, +}) + .config({ + hydrationComplete() { + (window as any).hydrationCompleted = true; + }, + }) + .define({ + name: "f-template", + }); diff --git a/packages/fast-html/test/fixtures/when-nested-elements/state.json b/packages/fast-html/test/fixtures/when-nested-elements/state.json new file mode 100644 index 00000000000..92f0bfd5011 --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/state.json @@ -0,0 +1,5 @@ +{ + "show": true, + "text": "Hello", + "items": [{ "text": "Hello" }] +} diff --git a/packages/fast-html/test/fixtures/when-nested-elements/templates.html b/packages/fast-html/test/fixtures/when-nested-elements/templates.html new file mode 100644 index 00000000000..6a6536b2c6e --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/templates.html @@ -0,0 +1,41 @@ + + + + + + + + + + + + + + + diff --git a/packages/fast-html/test/fixtures/when-nested-elements/when-nested-elements.spec.ts b/packages/fast-html/test/fixtures/when-nested-elements/when-nested-elements.spec.ts new file mode 100644 index 00000000000..19d70d33f1b --- /dev/null +++ b/packages/fast-html/test/fixtures/when-nested-elements/when-nested-elements.spec.ts @@ -0,0 +1,96 @@ +import { expect, test } from "@playwright/test"; + +/** + * A `defer-and-hydrate` child rendered inside a conditional content binding must + * hydrate exactly as it does inside a repeat binding. Swapping the surrounding + * control-flow directive should be the only difference between the two cases. + * + * See https://github.com/microsoft/fast/issues/7634. + */ +test.describe("When Nested Elements Hydration", () => { + async function hydrate(page: import("@playwright/test").Page) { + const hydrationCompleted = page.waitForFunction( + () => (window as any).hydrationCompleted === true, + ); + await page.goto("/fixtures/when-nested-elements/"); + await hydrationCompleted; + } + + for (const parent of ["when-parent", "repeat-parent"]) { + test.describe(parent, () => { + test("should hydrate the deferred child", async ({ page }) => { + await hydrate(page); + + const child = page.locator(`${parent} deferred-child`); + + await expect(child).toHaveCount(1); + await expect(child).not.toHaveAttribute("defer-hydration"); + await expect(child).not.toHaveAttribute("needs-hydration"); + }); + + test("should render the deferred child's bindings", async ({ page }) => { + await hydrate(page); + + await expect(page.locator(`${parent} deferred-child .text`)).toHaveText( + "Hello", + ); + }); + + test("should keep the deferred child's bindings live after hydration", async ({ + page, + }) => { + await hydrate(page); + + await page.locator(`${parent} deferred-child`).evaluate(node => { + (node as any).text = "Updated"; + }); + + await expect(page.locator(`${parent} deferred-child .text`)).toHaveText( + "Updated", + ); + }); + }); + } + + /** + * The pre-rendered DOM contains the child, but the value backing the binding has + * not resolved by the time the element hydrates. Both directives must reconcile + * the pre-rendered content away, and neither may leave it behind as an inert, + * un-hydrated subtree. + */ + for (const [parent, resolve] of [ + ["when-stale-parent", (node: any) => (node.show = true)], + ["repeat-stale-parent", (node: any) => (node.items = [{ text: "Hello" }])], + ] as const) { + test.describe(parent, () => { + test("should render exactly one hydrated child once the value resolves", async ({ + page, + }) => { + await hydrate(page); + + await page.locator(parent).evaluate(resolve); + + const child = page.locator(`${parent} deferred-child`); + + await expect(child).toHaveCount(1); + await expect(child).not.toHaveAttribute("defer-hydration"); + await expect(child).not.toHaveAttribute("needs-hydration"); + await expect(page.locator(`${parent} deferred-child .text`)).toHaveText( + "Hello", + ); + }); + }); + } + + test.describe("when-stale-parent", () => { + test("should discard the prerendered content it cannot adopt", async ({ + page, + }) => { + await hydrate(page); + + // The conditional content binding cannot adopt the prerendered child, so it + // must reconcile it away rather than leave it in the DOM owned by no view. + await expect(page.locator("when-stale-parent deferred-child")).toHaveCount(0); + }); + }); +});