From d648e6c16f4815bb8715acf69de6b540890ad09b Mon Sep 17 00:00:00 2001 From: Jane Chu <7559015+janechu@users.noreply.github.com> Date: Fri, 29 May 2026 14:39:33 -0700 Subject: [PATCH] fix(fast-element): avoid mutating render template bindings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...-2613f3cc-434d-4262-96ad-f1ab70e28a86.json | 7 ++ packages/fast-element/SIZES.md | 2 +- .../src/templating/render-binding.pw.spec.ts | 71 +++++++++++++++++ .../fast-element/src/templating/render.ts | 78 ++++++++++++++----- 4 files changed, 139 insertions(+), 19 deletions(-) create mode 100644 change/@microsoft-fast-element-2613f3cc-434d-4262-96ad-f1ab70e28a86.json create mode 100644 packages/fast-element/src/templating/render-binding.pw.spec.ts diff --git a/change/@microsoft-fast-element-2613f3cc-434d-4262-96ad-f1ab70e28a86.json b/change/@microsoft-fast-element-2613f3cc-434d-4262-96ad-f1ab70e28a86.json new file mode 100644 index 00000000000..c765a6a582a --- /dev/null +++ b/change/@microsoft-fast-element-2613f3cc-434d-4262-96ad-f1ab70e28a86.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Avoid mutating render template bindings", + "packageName": "@microsoft/fast-element", + "email": "7559015+janechu@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/packages/fast-element/SIZES.md b/packages/fast-element/SIZES.md index e6ac7937aa9..e611f377c2c 100644 --- a/packages/fast-element/SIZES.md +++ b/packages/fast-element/SIZES.md @@ -4,7 +4,7 @@ Bundle sizes for `@microsoft/fast-element` exports. | Export | Minified | Gzip | Brotli | |--------|----------|------|--------| -| CDN Rollup Bundle | 76.36 KB | 22.91 KB | 20.35 KB | +| CDN Rollup Bundle | 76.61 KB | 23.01 KB | 20.39 KB | | FASTElement (@microsoft/fast-element/fast-element.js) | 23.73 KB | 7.36 KB | 6.63 KB | | Updates (@microsoft/fast-element/updates.js) | 473 B | 335 B | 288 B | | Observable (@microsoft/fast-element/observable.js) | 6.70 KB | 2.50 KB | 2.22 KB | diff --git a/packages/fast-element/src/templating/render-binding.pw.spec.ts b/packages/fast-element/src/templating/render-binding.pw.spec.ts new file mode 100644 index 00000000000..1bee56c98b4 --- /dev/null +++ b/packages/fast-element/src/templating/render-binding.pw.spec.ts @@ -0,0 +1,71 @@ +import { expect, test } from "@playwright/test"; + +test.describe("The render template binding", () => { + test("does not mutate a provided template binding so it can be reused", async ({ + page, + }) => { + await page.goto("/"); + + const result = await page.evaluate(async () => { + // @ts-expect-error: Client module. + const { render, RenderInstruction, html, oneWay, Fake } = await import( + "/main.js" + ); + + const childEditTemplate = html` +

Child Edit Template

+ `; + const parentEditTemplate = html` +

Parent Edit Template

+ `; + + class TestChild { + name = "FAST"; + } + + class TestParent { + name = "FAST"; + } + + RenderInstruction.register({ + type: TestChild, + template: childEditTemplate, + name: "edit", + }); + + RenderInstruction.register({ + type: TestParent, + template: parentEditTemplate, + name: "edit", + }); + + const source = { + child: new TestChild(), + parent: new TestParent(), + viewName: "edit", + }; + const context = Fake.executionContext(); + const templateBinding = oneWay((x: any) => x.viewName); + const originalEvaluate = templateBinding.evaluate; + const childDirective = render((x: any) => x.child, templateBinding); + const parentDirective = render((x: any) => x.parent, templateBinding); + + return { + bindingEvaluateUnchanged: templateBinding.evaluate === originalEvaluate, + bindingValueUnchanged: + templateBinding.evaluate(source, context) === "edit", + childTemplate: + childDirective.templateBinding.evaluate(source, context) === + childEditTemplate, + parentTemplate: + parentDirective.templateBinding.evaluate(source, context) === + parentEditTemplate, + }; + }); + + expect(result.bindingEvaluateUnchanged).toBe(true); + expect(result.bindingValueUnchanged).toBe(true); + expect(result.childTemplate).toBe(true); + expect(result.parentTemplate).toBe(true); + }); +}); diff --git a/packages/fast-element/src/templating/render.ts b/packages/fast-element/src/templating/render.ts index d8a52720086..06a7d4805af 100644 --- a/packages/fast-element/src/templating/render.ts +++ b/packages/fast-element/src/templating/render.ts @@ -21,6 +21,7 @@ import { type ViewBehaviorFactory, type ViewController, } from "./html-directive.js"; +import { HydrationStage } from "./hydration-view.js"; import { Markup } from "./markup.js"; import { type CaptureType, @@ -29,7 +30,6 @@ import { type TemplateValue, ViewTemplate, } from "./template.js"; -import { HydrationStage } from "./hydration-view.js"; type ComposableView = ContentView & { isComposed?: boolean; @@ -368,6 +368,8 @@ const nullTemplate = html`   `; +type TemplateBindingValue = ContentTemplate | string | Node; + function instructionToTemplate(def: RenderInstruction | undefined) { if (def === void 0) { return nullTemplate; @@ -376,6 +378,62 @@ function instructionToTemplate(def: RenderInstruction | undefined) { return def.template; } +function resolveTemplateBindingValue( + result: TemplateBindingValue, + dataBinding: Binding, + source: TSource, + context: ExecutionContext, +): ContentTemplate { + if (isString(result)) { + return instructionToTemplate( + getForInstance(dataBinding.evaluate(source, context), result), + ); + } + + if (result instanceof Node) { + return (result as any).$fastTemplate ?? new NodeTemplate(result); + } + + return result; +} + +function adaptTemplateBinding( + binding: Binding, + dataBinding: Binding, +): Binding { + const evaluateTemplate = binding.evaluate; + const adapter = Object.create(Object.getPrototypeOf(binding)) as Binding< + TSource, + ContentTemplate, + TParent + >; + + for (const propertyName of Object.getOwnPropertyNames(binding)) { + if (propertyName !== "evaluate") { + Object.defineProperty( + adapter, + propertyName, + Object.getOwnPropertyDescriptor(binding, propertyName)!, + ); + } + } + + Object.defineProperty(adapter, "evaluate", { + configurable: true, + enumerable: true, + value: (source: TSource, context: ExecutionContext) => + resolveTemplateBindingValue( + evaluateTemplate(source, context), + dataBinding, + source, + context, + ), + writable: true, + }); + + return adapter; +} + function createElementTemplate( tagName: string, options?: ElementCreateOptions, @@ -731,23 +789,7 @@ export function render( return instructionToTemplate(getForInstance(data, template)); }); } else if (template instanceof Binding) { - const evaluateTemplate = template.evaluate; - - template.evaluate = (s: any, c: ExecutionContext) => { - let result = evaluateTemplate(s, c); - - if (isString(result)) { - result = instructionToTemplate( - getForInstance(dataBinding.evaluate(s, c), result), - ); - } else if (result instanceof Node) { - result = (result as any).$fastTemplate ?? new NodeTemplate(result); - } - - return result; - }; - - templateBinding = template as any; + templateBinding = adaptTemplateBinding(template, dataBinding); } else { templateBinding = oneTime((s: any, c: ExecutionContext) => template); }