diff --git a/change/@microsoft-fast-element-4dbc9ebd-2055-4ad1-af2a-5f3dacb6decc.json b/change/@microsoft-fast-element-4dbc9ebd-2055-4ad1-af2a-5f3dacb6decc.json new file mode 100644 index 00000000000..42eac0414fe --- /dev/null +++ b/change/@microsoft-fast-element-4dbc9ebd-2055-4ad1-af2a-5f3dacb6decc.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "feat: warn when duplicate render instruction registrations replace existing entries", + "packageName": "@microsoft/fast-element", + "email": "7559015+janechu@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/packages/fast-element/DESIGN.md b/packages/fast-element/DESIGN.md index 752c5f110e5..c97460d7e89 100644 --- a/packages/fast-element/DESIGN.md +++ b/packages/fast-element/DESIGN.md @@ -241,6 +241,12 @@ See [docs/architecture/html-tagged-template-literal.md](./docs/architecture/html `ViewBehaviorFactory` (created at template-authoring time) is the blueprint; `ViewBehavior` (created per `HTMLView` instance) is the live runtime object. +The `render` directive in `src/templating/render.ts` uses `RenderInstruction` +registrations to resolve templates for arbitrary model types. Registrations are +keyed by model constructor and instruction name. Registering another instruction +for the same type/name pair intentionally replaces the existing instruction and +emits a debug warning through `FAST.warn`; production behavior remains unchanged. + See [docs/template-bindings.md](./docs/template-bindings.md) for the full binding pipeline including `DOMAspect` routing and two-way binding. --- diff --git a/packages/fast-element/src/debug.ts b/packages/fast-element/src/debug.ts index c6a16b7b9a0..b6864e9546b 100644 --- a/packages/fast-element/src/debug.ts +++ b/packages/fast-element/src/debug.ts @@ -22,6 +22,8 @@ const baseDebugMessages = { "'${aspectName}' on '${tagName}' is blocked by the current DOMPolicy.", [1210 /* invalidHydrationAttributeMarker */]: "Invalid data-fe attribute value '${value}'. Expected a positive integer.", + [1211 /* duplicateRenderInstruction */]: + "Replacing existing RenderInstruction for '${type}' with name '${name}'.", [1401 /* missingElementDefinition */]: "Missing FASTElement definition.", [1501 /* noRegistrationForContext */]: "No registration for Context/Interface '${name}'.", diff --git a/packages/fast-element/src/interfaces.ts b/packages/fast-element/src/interfaces.ts index add2031d01e..c3ae10b90cb 100644 --- a/packages/fast-element/src/interfaces.ts +++ b/packages/fast-element/src/interfaces.ts @@ -66,8 +66,6 @@ export type ParameterDecorator = ( parameterIndex: number, ) => void; - - /** * Warning and error messages. * @internal @@ -87,6 +85,7 @@ export const enum Message { cannotSetTemplatePolicyAfterCompilation = 1208, blockedByDOMPolicy = 1209, invalidHydrationAttributeMarker = 1210, + duplicateRenderInstruction = 1211, // 1301 - 1400 Styles // 1401 - 1500 Components missingElementDefinition = 1401, diff --git a/packages/fast-element/src/templating/render-registration.pw.spec.ts b/packages/fast-element/src/templating/render-registration.pw.spec.ts new file mode 100644 index 00000000000..a85a00c6f10 --- /dev/null +++ b/packages/fast-element/src/templating/render-registration.pw.spec.ts @@ -0,0 +1,97 @@ +import { expect, test } from "@playwright/test"; + +const testServerUrl = process.env.FAST_TEST_SERVER_URL ?? "/"; + +test.describe("RenderInstruction registration", () => { + test("warns when registering over an existing instruction", async ({ page }) => { + await page.goto(testServerUrl); + + const result = await page.evaluate(async () => { + // @ts-expect-error: Client module. + const { FAST, RenderInstruction, html } = await import("/main.js"); + + const currentWarn = FAST.warn; + const warnings: { code: number; values?: Record }[] = []; + class TestClass {} + + try { + FAST.warn = function (code: number, values?: Record) { + warnings.push({ code, values }); + }; + + const first = RenderInstruction.register({ + type: TestClass, + template: html` +

First Template

+ `, + name: "test", + }); + const second = RenderInstruction.register({ + type: TestClass, + template: html` +

Second Template

+ `, + name: "test", + }); + const found = RenderInstruction.getByType(TestClass, "test"); + + return { + count: warnings.length, + code: warnings[0]?.code, + name: warnings[0]?.values?.name, + type: warnings[0]?.values?.type, + replaced: found === second && found !== first, + }; + } finally { + FAST.warn = currentWarn; + } + }); + + expect(result.count).toBe(1); + expect(result.code).toBe(1211); + expect(result.name).toBe("test"); + expect(result.type).toBe("TestClass"); + expect(result.replaced).toBe(true); + }); + + test("does not warn when registering a different instruction name", async ({ + page, + }) => { + await page.goto(testServerUrl); + + const result = await page.evaluate(async () => { + // @ts-expect-error: Client module. + const { FAST, RenderInstruction, html } = await import("/main.js"); + + const currentWarn = FAST.warn; + const warnings: { code: number; values?: Record }[] = []; + class TestClass {} + + try { + FAST.warn = function (code: number, values?: Record) { + warnings.push({ code, values }); + }; + + RenderInstruction.register({ + type: TestClass, + template: html` +

Default Template

+ `, + }); + RenderInstruction.register({ + type: TestClass, + template: html` +

Named Template

+ `, + name: "test", + }); + + return warnings.length; + } finally { + FAST.warn = currentWarn; + } + }); + + expect(result).toBe(0); + }); +}); diff --git a/packages/fast-element/src/templating/render.ts b/packages/fast-element/src/templating/render.ts index d8a52720086..b70b6755224 100644 --- a/packages/fast-element/src/templating/render.ts +++ b/packages/fast-element/src/templating/render.ts @@ -6,13 +6,14 @@ import { FASTElementDefinition } from "../components/fast-definitions.js"; import type { FASTElement } from "../components/fast-element.js"; import { isHydratable } from "../components/hydration.js"; import type { DOMPolicy } from "../dom.js"; -import { type Constructable, isFunction, isString } from "../interfaces.js"; +import { type Constructable, isFunction, isString, Message } from "../interfaces.js"; import type { Subscriber } from "../observation/notifier.js"; import type { ExecutionContext, Expression, ExpressionObserver, } from "../observation/observable.js"; +import { FAST } from "../platform.js"; import type { ContentTemplate, ContentView } from "./html-binding-directive.js"; import { type AddViewBehaviorFactory, @@ -21,6 +22,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 +31,6 @@ import { type TemplateValue, ViewTemplate, } from "./template.js"; -import { HydrationStage } from "./hydration-view.js"; type ComposableView = ContentView & { isComposed?: boolean; @@ -488,6 +489,15 @@ function register(optionsOrInstruction: any): RenderInstruction { ? optionsOrInstruction : create(optionsOrInstruction); + if (lookup[instruction.name] !== void 0) { + const typeName = (instruction.type as Function).name || "(anonymous)"; + + FAST.warn(Message.duplicateRenderInstruction, { + type: typeName, + name: instruction.name, + }); + } + return (lookup[instruction.name] = instruction); }