From 4554b092cfa17facb43248797cf32da586ddcd08 Mon Sep 17 00:00:00 2001 From: Jane Chu <7559015+janechu@users.noreply.github.com> Date: Fri, 29 May 2026 14:35:10 -0700 Subject: [PATCH] fix(fast-element): clear event context after thrown handlers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...-809c6e19-e1e8-49f4-ad14-bdd77f4d76dd.json | 7 ++ packages/fast-element/DESIGN.md | 4 + packages/fast-element/SIZES.md | 8 +- .../fast-element/docs/template-bindings.md | 11 ++- .../html-binding-directive.pw.spec.ts | 77 +++++++++++++++++++ .../src/templating/html-binding-directive.ts | 16 ++-- 6 files changed, 109 insertions(+), 14 deletions(-) create mode 100644 change/@microsoft-fast-element-809c6e19-e1e8-49f4-ad14-bdd77f4d76dd.json create mode 100644 packages/fast-element/src/templating/html-binding-directive.pw.spec.ts diff --git a/change/@microsoft-fast-element-809c6e19-e1e8-49f4-ad14-bdd77f4d76dd.json b/change/@microsoft-fast-element-809c6e19-e1e8-49f4-ad14-bdd77f4d76dd.json new file mode 100644 index 00000000000..340fa910ed9 --- /dev/null +++ b/change/@microsoft-fast-element-809c6e19-e1e8-49f4-ad14-bdd77f4d76dd.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "fix: clear event context when event handlers throw", + "packageName": "@microsoft/fast-element", + "email": "7559015+janechu@users.noreply.github.com", + "dependentChangeType": "none" +} \ No newline at end of file diff --git a/packages/fast-element/DESIGN.md b/packages/fast-element/DESIGN.md index 752c5f110e5..dd309d54a53 100644 --- a/packages/fast-element/DESIGN.md +++ b/packages/fast-element/DESIGN.md @@ -166,6 +166,10 @@ This gives FAST automatic, fine-grained dependency tracking without explicit dec | `listener` | Same as `oneWay` but attaches as a DOM event handler | `normalizeBinding(value)` converts raw arrow functions or static values into a `Binding` object. +Event listener bindings set the current DOM event on `ExecutionContext` only while +the handler expression is evaluating. The event is cleared in a `finally` path even +when the handler throws; for completed evaluations, any result other than `true` +continues to call `preventDefault()` on the event. --- diff --git a/packages/fast-element/SIZES.md b/packages/fast-element/SIZES.md index e6ac7937aa9..f2b4b8fde38 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.37 KB | 22.91 KB | 20.32 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 | @@ -15,11 +15,11 @@ Bundle sizes for `@microsoft/fast-element` exports. | slotted (@microsoft/fast-element/slotted.js) | 4.60 KB | 1.79 KB | 1.58 KB | | volatile (@microsoft/fast-element/volatile.js) | 6.79 KB | 2.53 KB | 2.25 KB | | when (@microsoft/fast-element/when.js) | 1.82 KB | 712 B | 565 B | -| html (@microsoft/fast-element/html.js) | 25.92 KB | 8.50 KB | 7.61 KB | +| html (@microsoft/fast-element/html.js) | 25.93 KB | 8.50 KB | 7.62 KB | | repeat (@microsoft/fast-element/repeat.js) | 29.57 KB | 9.41 KB | 8.48 KB | | css (@microsoft/fast-element/css.js) | 2.43 KB | 1.00 KB | 911 B | -| enableHydration (@microsoft/fast-element/hydration.js) | 43.27 KB | 13.19 KB | 11.88 KB | -| declarativeTemplate (@microsoft/fast-element/declarative.js) | 58.77 KB | 18.45 KB | 16.46 KB | +| enableHydration (@microsoft/fast-element/hydration.js) | 43.28 KB | 13.19 KB | 11.88 KB | +| declarativeTemplate (@microsoft/fast-element/declarative.js) | 58.78 KB | 18.46 KB | 16.47 KB | | ArrayObserver (@microsoft/fast-element/array-observer.js) | 12.51 KB | 4.45 KB | 4.01 KB | | observerMap (@microsoft/fast-element/observer-map.js) | 20.41 KB | 7.24 KB | 6.52 KB | | attributeMap (@microsoft/fast-element/attribute-map.js) | 15.78 KB | 5.58 KB | 5.04 KB | diff --git a/packages/fast-element/docs/template-bindings.md b/packages/fast-element/docs/template-bindings.md index 862acb27fc0..4737ae6fb55 100644 --- a/packages/fast-element/docs/template-bindings.md +++ b/packages/fast-element/docs/template-bindings.md @@ -205,14 +205,19 @@ sequenceDiagram Directive->>Context: ExecutionContext.setEvent(event) Directive->>Binding: evaluate(controller.source, controller.context) Binding->>Source: Execute handler: (s, c) => s.handleClick(c.event) - Source-->>Binding: Return result - Binding-->>Directive: Return result + Source-->>Binding: Return result or throw + Binding-->>Directive: Return result or throw + Directive->>Context: ExecutionContext.setEvent(null) in finally alt result !== true Directive->>DOM: event.preventDefault() end - Directive->>Context: ExecutionContext.setEvent(null) ``` +`ExecutionContext.setEvent(null)` runs in a `finally` path, so thrown event +handlers cannot leak the event context into later handlers or evaluations. When +the handler completes normally, the existing convention remains: any return value +other than `true` prevents the event's default action. + ### 7. Content Binding – Template Composition When a binding expression returns a `ContentTemplate` (e.g., another `ViewTemplate`), the content update sink composes a child view into the DOM. diff --git a/packages/fast-element/src/templating/html-binding-directive.pw.spec.ts b/packages/fast-element/src/templating/html-binding-directive.pw.spec.ts new file mode 100644 index 00000000000..5c3d8556249 --- /dev/null +++ b/packages/fast-element/src/templating/html-binding-directive.pw.spec.ts @@ -0,0 +1,77 @@ +import { expect, test } from "@playwright/test"; + +test.describe("HTMLBindingDirective", () => { + test.beforeEach(async ({ page }) => { + await page.goto("/"); + }); + + test("clears the current event when the event handler throws", async ({ page }) => { + const { + defaultPrevented, + eventAvailableDuringHandler, + eventClearedAfterThrow, + threw, + } = await page.evaluate(async () => { + // @ts-expect-error: Client module. + const { + HTMLBindingDirective, + HTMLDirective, + ExecutionContext, + Fake, + DOM, + nextId, + listener, + } = await import("/main.js"); + + class Model { + eventAvailableDuringHandler = false; + + invokeAction(context: any) { + this.eventAvailableDuringHandler = + context.event === ExecutionContext.getEvent(); + throw new Error("Event handler failed."); + } + } + + const directive = new HTMLBindingDirective( + listener((x: Model, c: any) => x.invokeAction(c), {}), + ); + HTMLDirective.assignAspect(directive, "@my-event"); + + const node = document.createElement("div"); + directive.id = nextId(); + directive.targetNodeId = "r"; + directive.targetTagName = node.tagName ?? null; + directive.policy = DOM.policy; + + const targets = { r: node }; + const behavior = directive.createBehavior(); + const controller = Fake.viewController(targets, behavior); + const model = new Model(); + controller.bind(model); + + const event = new CustomEvent("my-event", { cancelable: true }); + Object.defineProperty(event, "currentTarget", { value: node }); + + let threw = false; + + try { + directive.handleEvent(event); + } catch { + threw = true; + } + + return { + defaultPrevented: event.defaultPrevented, + eventAvailableDuringHandler: model.eventAvailableDuringHandler, + eventClearedAfterThrow: ExecutionContext.getEvent() === null, + threw, + }; + }); + + expect(threw).toBe(true); + expect(eventAvailableDuringHandler).toBe(true); + expect(eventClearedAfterThrow).toBe(true); + expect(defaultPrevented).toBe(false); + }); +}); diff --git a/packages/fast-element/src/templating/html-binding-directive.ts b/packages/fast-element/src/templating/html-binding-directive.ts index 5df23824c49..2f1a17310ba 100644 --- a/packages/fast-element/src/templating/html-binding-directive.ts +++ b/packages/fast-element/src/templating/html-binding-directive.ts @@ -425,7 +425,7 @@ export class HTMLBindingDirective * Implements the EventListener interface. When a DOM event fires on the target * element, this method retrieves the ViewController stored on the element, * sets the event on the ExecutionContext so `c.event` is available to the - * binding expression, and evaluates the expression. If the expression returns + * binding expression, and clears it after evaluation. If the expression returns * anything other than `true`, the event's default action is prevented. * @internal */ @@ -433,12 +433,14 @@ export class HTMLBindingDirective const controller = event.currentTarget![this.data] as ViewController; if (controller.isBound) { - ExecutionContext.setEvent(event); - const result = this.dataBinding.evaluate( - controller.source, - controller.context, - ); - ExecutionContext.setEvent(null); + let result: any; + + try { + ExecutionContext.setEvent(event); + result = this.dataBinding.evaluate(controller.source, controller.context); + } finally { + ExecutionContext.setEvent(null); + } if (result !== true) { event.preventDefault();