fix(fast-html): add errors fixture for f-template error cases#7406
fix(fast-html): add errors fixture for f-template error cases#7406
Conversation
Add a test fixture that covers the two error conditions thrown by the f-template element: no template element present and multiple template elements present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
janechu
changed the title
There was a problem hiding this comment.
Pull request overview
Adds explicit coverage for <f-template> error cases in @microsoft/fast-html by introducing a new Playwright fixture and a new error code/message for the “multiple <template> children” condition.
Changes:
- Added a new
errorsfixture (HTML + setup + Playwright spec) to validate<f-template>error behavior viaunhandledrejection. - Introduced
Message.moreThanOneTemplateProvided(2001) and throws it when an<f-template>contains more than one<template>. - Registered a new debug message for error code 2001.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fast-html/test/fixtures/errors/main.ts | Defines minimal custom elements and registers <f-template> for the new fixture. |
| packages/fast-html/test/fixtures/errors/index.html | Fixture page that triggers “no template” and “multiple templates” cases. |
| packages/fast-html/test/fixtures/errors/errors.spec.ts | Playwright tests asserting both error paths via unhandledrejection. |
| packages/fast-html/src/interfaces.ts | Adds Message.moreThanOneTemplateProvided = 2001. |
| packages/fast-html/src/debug.ts | Adds a debug message for error code 2001. |
| packages/fast-html/src/components/template.ts | Updates <f-template> handling to throw 2001 when multiple templates exist. |
| change/@microsoft-fast-html-a4161f8e-9273-42b3-9c6f-95249dd0ff2e.json | Beachball change file for the @microsoft/fast-html prerelease fix. |
| [2000 /* noTemplateProvided */]: `The first child of the <f-template> must be a <template>, this is missing from ${name}.`, | ||
| [2001 /* moreThanOneTemplateProvided */]: `There can only be one <template> inside the <f-template>, you must add one for ${name}.`, |
There was a problem hiding this comment.
debugMessages entries are defined using template literals, so ${name} is being interpolated by JavaScript at module evaluation time (e.g. to window.name in browsers) instead of being left as a placeholder for FAST.error(code, { name }) to format. These messages should be plain strings containing the literal ${name} placeholder so formatMessage() can substitute runtime values.
| [2000 /* noTemplateProvided */]: `The first child of the <f-template> must be a <template>, this is missing from ${name}.`, | |
| [2001 /* moreThanOneTemplateProvided */]: `There can only be one <template> inside the <f-template>, you must add one for ${name}.`, | |
| [2000 /* noTemplateProvided */]: "The first child of the <f-template> must be a <template>, this is missing from ${name}.", | |
| [2001 /* moreThanOneTemplateProvided */]: "There can only be one <template> inside the <f-template>, you must add one for ${name}.", |
| @@ -1,3 +1,4 @@ | |||
| export const debugMessages = { | |||
| [2000 /* noTemplateProvided */]: `The first child of the <f-template> must be a <template>, this is missing from ${name}.`, | |||
| [2001 /* moreThanOneTemplateProvided */]: `There can only be one <template> inside the <f-template>, you must add one for ${name}.`, | |||
There was a problem hiding this comment.
The new moreThanOneTemplateProvided message text is misleading: it says "you must add one" even though this error is thrown when multiple <template> elements are present. Update the wording to instruct users to remove extra <template> elements and keep exactly one.
| [2001 /* moreThanOneTemplateProvided */]: `There can only be one <template> inside the <f-template>, you must add one for ${name}.`, | |
| [2001 /* moreThanOneTemplateProvided */]: `There can only be one <template> inside the <f-template>; remove any extra <template> elements and keep exactly one for ${name}.`, |
| test("throws an error when no template element is present", async ({ page }) => { | ||
| await page.addInitScript(() => { | ||
| (window as any).__errors = []; | ||
| window.addEventListener("unhandledrejection", event => { | ||
| (window as any).__errors.push( | ||
| event.reason?.message ?? String(event.reason), | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The page.addInitScript setup for capturing unhandledrejection is duplicated in both tests. Consider moving this into a test.beforeEach (or a small helper) so the listener/storage initialization is defined once and stays consistent if it changes.
| window.addEventListener("unhandledrejection", event => { | ||
| (window as any).__errors.push( | ||
| event.reason?.message ?? String(event.reason), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
In the unhandledrejection handler, the event isn't preventDefault()'d. Since these tests intentionally trigger unhandled rejections, calling event.preventDefault() will suppress default reporting/noise and reduce the risk of future harness/config changes treating the rejection as a test failure.
Pull Request
📖 Description
Adds an
errorstest fixture for@microsoft/fast-htmlthat covers the two error conditions thrown by the<f-template>element:<f-template>contains no<template>child, anoTemplateProvidederror (code 2000) is thrown.<f-template>contains more than one<template>child, amoreThanOneTemplateProvidederror (code 2001) is thrown.The fixture includes
index.html,main.ts, anderrors.spec.ts. Tests usepage.addInitScriptto interceptunhandledrejectionevents (the errors originate inside an unhandled promise chain) for reliable cross-browser coverage across Chromium, Firefox, and WebKit.📑 Test Plan
Run the new fixture tests:
All 6 tests pass (2 cases × 3 browsers).
✅ Checklist
General
$ npm run change