diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Interop/InternalRegisteredFunction.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Interop/InternalRegisteredFunction.ts index fe7a4cd82..e75657b8f 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Interop/InternalRegisteredFunction.ts +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Interop/InternalRegisteredFunction.ts @@ -1,12 +1,12 @@ import { invokeWithJsonMarshalling } from './InvokeWithJsonMarshalling'; -import { attachComponentToElement, renderBatch } from '../Rendering/Renderer'; +import { attachRootComponentToElement, renderBatch } from '../Rendering/Renderer'; /** * The definitive list of internal functions invokable from .NET code. * These function names are treated as 'reserved' and cannot be passed to registerFunction. */ export const internalRegisteredFunctions = { - attachComponentToElement, + attachRootComponentToElement, invokeWithJsonMarshalling, renderBatch, }; diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts index c06195c3c..8ab59318c 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts @@ -2,18 +2,24 @@ import { getRenderTreeEditPtr, renderTreeEdit, RenderTreeEditPointer, EditType } from './RenderTreeEdit'; import { getTreeFramePtr, renderTreeFrame, FrameType, RenderTreeFramePointer } from './RenderTreeFrame'; import { platform } from '../Environment'; +import { EventDelegator } from './EventDelegator'; +import { EventForDotNet, UIEventArgs } from './EventForDotNet'; const selectValuePropname = '_blazorSelectValue'; let raiseEventMethod: MethodHandle; let renderComponentMethod: MethodHandle; export class BrowserRenderer { + private eventDelegator: EventDelegator; private childComponentLocations: { [componentId: number]: Element } = {}; constructor(private browserRendererId: number) { + this.eventDelegator = new EventDelegator((event, componentId, eventHandlerId, eventArgs) => { + raiseEvent(event, this.browserRendererId, componentId, eventHandlerId, eventArgs); + }); } - public attachComponentToElement(componentId: number, element: Element) { - this.childComponentLocations[componentId] = element; + public attachRootComponentToElement(componentId: number, element: Element) { + this.attachComponentToElement(componentId, element); } public updateComponent(componentId: number, edits: System_Array, editsOffset: number, editsLength: number, referenceFrames: System_Array) { @@ -29,7 +35,15 @@ export class BrowserRenderer { delete this.childComponentLocations[componentId]; } - applyEdits(componentId: number, parent: Element, childIndex: number, edits: System_Array, editsOffset: number, editsLength: number, referenceFrames: System_Array) { + public disposeEventHandler(eventHandlerId: number) { + this.eventDelegator.removeListener(eventHandlerId); + } + + private attachComponentToElement(componentId: number, element: Element) { + this.childComponentLocations[componentId] = element; + } + + private applyEdits(componentId: number, parent: Element, childIndex: number, edits: System_Array, editsOffset: number, editsLength: number, referenceFrames: System_Array) { let currentDepth = 0; let childIndexAtCurrentDepth = childIndex; const maxEditIndexExcl = editsOffset + editsLength; @@ -58,6 +72,8 @@ export class BrowserRenderer { break; } case EditType.removeAttribute: { + // Note that we don't have to dispose the info we track about event handlers here, because the + // disposed event handler IDs are delivered separately (in the 'disposedEventHandlerIds' array) const siblingIndex = renderTreeEdit.siblingIndex(edit); removeAttributeFromDOM(parent, childIndexAtCurrentDepth + siblingIndex, renderTreeEdit.removedAttributeName(edit)!); break; @@ -91,7 +107,7 @@ export class BrowserRenderer { } } - insertFrame(componentId: number, parent: Element, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number): number { + private insertFrame(componentId: number, parent: Element, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number): number { const frameType = renderTreeFrame.frameType(frame); switch (frameType) { case FrameType.element: @@ -113,7 +129,7 @@ export class BrowserRenderer { } } - insertElement(componentId: number, parent: Element, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number) { + private insertElement(componentId: number, parent: Element, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number) { const tagName = renderTreeFrame.elementName(frame)!; const newDomElement = tagName === 'svg' || parent.namespaceURI === 'http://www.w3.org/2000/svg' ? document.createElementNS('http://www.w3.org/2000/svg', tagName) : @@ -135,7 +151,7 @@ export class BrowserRenderer { } } - insertComponent(parent: Element, childIndex: number, frame: RenderTreeFramePointer) { + private insertComponent(parent: Element, childIndex: number, frame: RenderTreeFramePointer) { // Currently, to support O(1) lookups from render tree frames to DOM nodes, we rely on // each child component existing as a single top-level element in the DOM. To guarantee // that, we wrap child components in these 'blazor-component' wrappers. @@ -162,68 +178,41 @@ export class BrowserRenderer { this.attachComponentToElement(childComponentId, containerElement); } - insertText(parent: Element, childIndex: number, textFrame: RenderTreeFramePointer) { + private insertText(parent: Element, childIndex: number, textFrame: RenderTreeFramePointer) { const textContent = renderTreeFrame.textContent(textFrame)!; const newDomTextNode = document.createTextNode(textContent); insertNodeIntoDOM(newDomTextNode, parent, childIndex); } - applyAttribute(componentId: number, toDomElement: Element, attributeFrame: RenderTreeFramePointer) { + private applyAttribute(componentId: number, toDomElement: Element, attributeFrame: RenderTreeFramePointer) { const attributeName = renderTreeFrame.attributeName(attributeFrame)!; const browserRendererId = this.browserRendererId; const eventHandlerId = renderTreeFrame.attributeEventHandlerId(attributeFrame); + if (eventHandlerId) { + const firstTwoChars = attributeName.substring(0, 2); + const eventName = attributeName.substring(2); + if (firstTwoChars !== 'on' || !eventName) { + throw new Error(`Attribute has nonzero event handler ID, but attribute name '${attributeName}' does not start with 'on'.`); + } + this.eventDelegator.setListener(toDomElement, eventName, componentId, eventHandlerId); + return; + } + if (attributeName === 'value') { if (this.tryApplyValueProperty(toDomElement, renderTreeFrame.attributeValue(attributeFrame))) { return; // If this DOM element type has special 'value' handling, don't also write it as an attribute } } - // TODO: Instead of applying separate event listeners to each DOM element, use event delegation - // and remove all the _blazor*Listener hacks - switch (attributeName) { - case 'onclick': { - toDomElement.removeEventListener('click', toDomElement['_blazorClickListener']); - const listener = evt => raiseEvent(evt, browserRendererId, componentId, eventHandlerId, 'mouse', { Type: 'click' }); - toDomElement['_blazorClickListener'] = listener; - toDomElement.addEventListener('click', listener); - break; - } - case 'onchange': { - toDomElement.removeEventListener('change', toDomElement['_blazorChangeListener']); - const targetIsCheckbox = isCheckbox(toDomElement); - const listener = evt => { - const newValue = targetIsCheckbox ? evt.target.checked : evt.target.value; - raiseEvent(evt, browserRendererId, componentId, eventHandlerId, 'change', { Type: 'change', Value: newValue }); - }; - toDomElement['_blazorChangeListener'] = listener; - toDomElement.addEventListener('change', listener); - break; - } - case 'onkeypress': { - toDomElement.removeEventListener('keypress', toDomElement['_blazorKeypressListener']); - const listener = evt => { - // This does not account for special keys nor cross-browser differences. So far it's - // just to establish that we can pass parameters when raising events. - // We use C#-style PascalCase on the eventInfo to simplify deserialization, but this could - // change if we introduced a richer JSON library on the .NET side. - raiseEvent(evt, browserRendererId, componentId, eventHandlerId, 'keyboard', { Type: evt.type, Key: (evt as any).key }); - }; - toDomElement['_blazorKeypressListener'] = listener; - toDomElement.addEventListener('keypress', listener); - break; - } - default: - // Treat as a regular string-valued attribute - toDomElement.setAttribute( - attributeName, - renderTreeFrame.attributeValue(attributeFrame)! - ); - break; - } + // Treat as a regular string-valued attribute + toDomElement.setAttribute( + attributeName, + renderTreeFrame.attributeValue(attributeFrame)! + ); } - tryApplyValueProperty(element: Element, value: string | null) { + private tryApplyValueProperty(element: Element, value: string | null) { // Certain elements have built-in behaviour for their 'value' property switch (element.tagName) { case 'INPUT': @@ -257,7 +246,7 @@ export class BrowserRenderer { } } - insertFrameRange(componentId: number, parent: Element, childIndex: number, frames: System_Array, startIndex: number, endIndexExcl: number): number { + private insertFrameRange(componentId: number, parent: Element, childIndex: number, frames: System_Array, startIndex: number, endIndexExcl: number): number { const origChildIndex = childIndex; for (let index = startIndex; index < endIndexExcl; index++) { const frame = getTreeFramePtr(frames, index); @@ -296,7 +285,7 @@ function removeAttributeFromDOM(parent: Element, childIndex: number, attributeNa element.removeAttribute(attributeName); } -function raiseEvent(event: Event, browserRendererId: number, componentId: number, eventHandlerId: number, eventInfoType: EventInfoType, eventInfo: any) { +function raiseEvent(event: Event, browserRendererId: number, componentId: number, eventHandlerId: number, eventArgs: EventForDotNet) { event.preventDefault(); if (!raiseEventMethod) { @@ -309,13 +298,11 @@ function raiseEvent(event: Event, browserRendererId: number, componentId: number BrowserRendererId: browserRendererId, ComponentId: componentId, EventHandlerId: eventHandlerId, - EventArgsType: eventInfoType + EventArgsType: eventArgs.type }; platform.callMethod(raiseEventMethod, null, [ platform.toDotNetString(JSON.stringify(eventDescriptor)), - platform.toDotNetString(JSON.stringify(eventInfo)) + platform.toDotNetString(JSON.stringify(eventArgs.data)) ]); } - -type EventInfoType = 'mouse' | 'keyboard' | 'change'; diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventDelegator.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventDelegator.ts new file mode 100644 index 000000000..fb24c2c5f --- /dev/null +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventDelegator.ts @@ -0,0 +1,156 @@ +import { EventForDotNet, UIEventArgs } from './EventForDotNet'; + +export interface OnEventCallback { + (event: Event, componentId: number, eventHandlerId: number, eventArgs: EventForDotNet): void; +} + +// Responsible for adding/removing the eventInfo on an expando property on DOM elements, and +// calling an EventInfoStore that deals with registering/unregistering the underlying delegated +// event listeners as required (and also maps actual events back to the given callback). +export class EventDelegator { + private static nextEventDelegatorId = 0; + private eventsCollectionKey: string; + private eventInfoStore: EventInfoStore; + + constructor(private onEvent: OnEventCallback) { + const eventDelegatorId = ++EventDelegator.nextEventDelegatorId; + this.eventsCollectionKey = `_blazorEvents_${eventDelegatorId}`; + this.eventInfoStore = new EventInfoStore(this.onGlobalEvent.bind(this)); + } + + public setListener(element: Element, eventName: string, componentId: number, eventHandlerId: number) { + // Ensure we have a place to store event info for this element + let infoForElement: EventHandlerInfosForElement = element[this.eventsCollectionKey]; + if (!infoForElement) { + infoForElement = element[this.eventsCollectionKey] = {}; + } + + if (infoForElement.hasOwnProperty(eventName)) { + // We can cheaply update the info on the existing object and don't need any other housekeeping + const oldInfo = infoForElement[eventName]; + this.eventInfoStore.update(oldInfo.eventHandlerId, eventHandlerId); + } else { + // Go through the whole flow which might involve registering a new global handler + const newInfo = { element, eventName, componentId, eventHandlerId }; + this.eventInfoStore.add(newInfo); + infoForElement[eventName] = newInfo; + } + } + + public removeListener(eventHandlerId: number) { + // This method gets called whenever the .NET-side code reports that a certain event handler + // has been disposed. However we will already have disposed the info about that handler if + // the eventHandlerId for the (element,eventName) pair was replaced during diff application. + const info = this.eventInfoStore.remove(eventHandlerId); + if (info) { + // Looks like this event handler wasn't already disposed + // Remove the associated data from the DOM element + const element = info.element; + if (element.hasOwnProperty(this.eventsCollectionKey)) { + const elementEventInfos: EventHandlerInfosForElement = element[this.eventsCollectionKey]; + delete elementEventInfos[info.eventName]; + if (Object.getOwnPropertyNames(elementEventInfos).length === 0) { + delete element[this.eventsCollectionKey]; + } + } + } + } + + private onGlobalEvent(evt: Event) { + if (!(evt.target instanceof Element)) { + return; + } + + // Scan up the element hierarchy, looking for any matching registered event handlers + let candidateElement = evt.target as Element | null; + let eventArgs: EventForDotNet | null = null; // Populate lazily + while (candidateElement) { + if (candidateElement.hasOwnProperty(this.eventsCollectionKey)) { + const handlerInfos = candidateElement[this.eventsCollectionKey]; + if (handlerInfos.hasOwnProperty(evt.type)) { + // We are going to raise an event for this element, so prepare info needed by the .NET code + if (!eventArgs) { + eventArgs = EventForDotNet.fromDOMEvent(evt); + } + + const handlerInfo = handlerInfos[evt.type]; + this.onEvent(evt, handlerInfo.componentId, handlerInfo.eventHandlerId, eventArgs); + } + } + + candidateElement = candidateElement.parentElement; + } + } +} + +// Responsible for adding and removing the global listener when the number of listeners +// for a given event name changes between zero and nonzero +class EventInfoStore { + private infosByEventHandlerId: { [eventHandlerId: number]: EventHandlerInfo } = {}; + private countByEventName: { [eventName: string]: number } = {}; + + constructor(private globalListener: EventListener) { + } + + public add(info: EventHandlerInfo) { + if (this.infosByEventHandlerId[info.eventHandlerId]) { + // Should never happen, but we want to know if it does + throw new Error(`Event ${info.eventHandlerId} is already tracked`); + } + + this.infosByEventHandlerId[info.eventHandlerId] = info; + + const eventName = info.eventName; + if (this.countByEventName.hasOwnProperty(eventName)) { + this.countByEventName[eventName]++; + } else { + this.countByEventName[eventName] = 1; + document.addEventListener(eventName, this.globalListener); + } + } + + public update(oldEventHandlerId: number, newEventHandlerId: number) { + if (this.infosByEventHandlerId.hasOwnProperty(newEventHandlerId)) { + // Should never happen, but we want to know if it does + throw new Error(`Event ${newEventHandlerId} is already tracked`); + } + + // Since we're just updating the event handler ID, there's no need to update the global counts + const info = this.infosByEventHandlerId[oldEventHandlerId]; + delete this.infosByEventHandlerId[oldEventHandlerId]; + info.eventHandlerId = newEventHandlerId; + this.infosByEventHandlerId[newEventHandlerId] = info; + } + + public remove(eventHandlerId: number): EventHandlerInfo { + const info = this.infosByEventHandlerId[eventHandlerId]; + if (info) { + delete this.infosByEventHandlerId[eventHandlerId]; + + const eventName = info.eventName; + if (--this.countByEventName[eventName] === 0) { + delete this.countByEventName[eventName]; + document.removeEventListener(eventName, this.globalListener); + } + } + + return info; + } +} + +interface EventHandlerInfosForElement { + // Although we *could* track multiple event handlers per (element, eventName) pair + // (since they have distinct eventHandlerId values), there's no point doing so because + // our programming model is that you declare event handlers as attributes. An element + // can only have one attribute with a given name, hence only one event handler with + // that name at any one time. + // So to keep things simple, only track one EventHandlerInfo per (element, eventName) + [eventName: string]: EventHandlerInfo +} + +interface EventHandlerInfo { + element: Element; + eventName: string; + componentId: number; + eventHandlerId: number; +} diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventForDotNet.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventForDotNet.ts new file mode 100644 index 000000000..6f68e19ac --- /dev/null +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventForDotNet.ts @@ -0,0 +1,46 @@ +export class EventForDotNet { + constructor(public readonly type: EventArgsType, public readonly data: TData) { + } + + static fromDOMEvent(event: Event): EventForDotNet { + const element = event.target as Element; + switch (event.type) { + case 'click': + case 'mousedown': + case 'mouseup': + return new EventForDotNet('mouse', { Type: event.type }); + case 'change': { + const targetIsCheckbox = isCheckbox(element); + const newValue = targetIsCheckbox ? !!element['checked'] : element['value']; + return new EventForDotNet('change', { Type: event.type, Value: newValue }); + } + case 'keypress': + return new EventForDotNet('keyboard', { Type: event.type, Key: (event as any).key }); + default: + return new EventForDotNet('unknown', { Type: event.type }); + } + } +} + +function isCheckbox(element: Element | null) { + return element && element.tagName === 'INPUT' && element.getAttribute('type') === 'checkbox'; +} + +// The following interfaces must be kept in sync with the UIEventArgs C# classes + +type EventArgsType = 'mouse' | 'keyboard' | 'change' | 'unknown'; + +export interface UIEventArgs { + Type: string; +} + +interface UIMouseEventArgs extends UIEventArgs { +} + +interface UIKeyboardEventArgs extends UIEventArgs { + Key: string; +} + +interface UIChangeEventArgs extends UIEventArgs { + Value: string | boolean; +} diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/RenderBatch.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/RenderBatch.ts index 6beff2ec8..31d371cb3 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/RenderBatch.ts +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/RenderBatch.ts @@ -9,6 +9,7 @@ export const renderBatch = { updatedComponents: (obj: RenderBatchPointer) => platform.readStructField>(obj, 0), referenceFrames: (obj: RenderBatchPointer) => platform.readStructField>(obj, arrayRangeStructLength), disposedComponentIds: (obj: RenderBatchPointer) => platform.readStructField>(obj, arrayRangeStructLength + arrayRangeStructLength), + disposedEventHandlerIds: (obj: RenderBatchPointer) => platform.readStructField>(obj, arrayRangeStructLength + arrayRangeStructLength + arrayRangeStructLength), }; const arrayRangeStructLength = 8; diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts index bae0f92a0..7e2b43458 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts @@ -6,7 +6,7 @@ import { BrowserRenderer } from './BrowserRenderer'; type BrowserRendererRegistry = { [browserRendererId: number]: BrowserRenderer }; const browserRenderers: BrowserRendererRegistry = {}; -export function attachComponentToElement(browserRendererId: number, elementSelector: System_String, componentId: number) { +export function attachRootComponentToElement(browserRendererId: number, elementSelector: System_String, componentId: number) { const elementSelectorJs = platform.toJavaScriptString(elementSelector); const element = document.querySelector(elementSelectorJs); if (!element) { @@ -17,7 +17,7 @@ export function attachComponentToElement(browserRendererId: number, elementSelec if (!browserRenderer) { browserRenderer = browserRenderers[browserRendererId] = new BrowserRenderer(browserRendererId); } - browserRenderer.attachComponentToElement(componentId, element); + browserRenderer.attachRootComponentToElement(componentId, element); clearElement(element); } @@ -53,6 +53,15 @@ export function renderBatch(browserRendererId: number, batch: RenderBatchPointer const componentId = platform.readInt32Field(componentIdPtr); browserRenderer.disposeComponent(componentId); } + + const disposedEventHandlerIds = renderBatchStruct.disposedEventHandlerIds(batch); + const disposedEventHandlerIdsLength = arrayRange.count(disposedEventHandlerIds); + const disposedEventHandlerIdsArray = arrayRange.array(disposedEventHandlerIds); + for (let i = 0; i < disposedEventHandlerIdsLength; i++) { + const eventHandlerIdPtr = platform.getArrayEntryPtr(disposedEventHandlerIdsArray, i, 4); + const eventHandlerId = platform.readInt32Field(eventHandlerIdPtr); + browserRenderer.disposeEventHandler(eventHandlerId); + } } function clearElement(element: Element) { diff --git a/src/Microsoft.AspNetCore.Blazor.Browser/Rendering/BrowserRenderer.cs b/src/Microsoft.AspNetCore.Blazor.Browser/Rendering/BrowserRenderer.cs index 310e19644..0569fffd6 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser/Rendering/BrowserRenderer.cs +++ b/src/Microsoft.AspNetCore.Blazor.Browser/Rendering/BrowserRenderer.cs @@ -60,7 +60,7 @@ public void AddComponent(Type componentType, string domElementSelector) var component = InstantiateComponent(componentType); var componentId = AssignComponentId(component); RegisteredFunction.InvokeUnmarshalled( - "attachComponentToElement", + "attachRootComponentToElement", _browserRendererId, domElementSelector, componentId); diff --git a/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs b/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs index 686a8323b..1f5ee20fe 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs @@ -213,7 +213,7 @@ protected RenderTreeFrame bind(DateTime value, string format) /// A that represents the event handler. protected RenderTreeFrame onclick(Action handler) // Note that the 'sequence' value is updated later when inserted into the tree - => RenderTreeFrame.Attribute(0, "onclick", _ => handler()); + => RenderTreeFrame.Attribute(0, "onclick", handler != null ? (_ => handler()) : (UIEventHandler)null); /// /// Handles change events by invoking . diff --git a/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatch.cs b/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatch.cs index e3483f581..b962ddd59 100644 --- a/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatch.cs +++ b/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatch.cs @@ -27,14 +27,21 @@ public readonly struct RenderBatch /// public ArrayRange DisposedComponentIDs { get; } + /// + /// Gets the IDs of the event handlers that were disposed. + /// + public ArrayRange DisposedEventHandlerIDs { get; } + internal RenderBatch( ArrayRange updatedComponents, ArrayRange referenceFrames, - ArrayRange disposedComponentIDs) + ArrayRange disposedComponentIDs, + ArrayRange disposedEventHandlerIDs) { UpdatedComponents = updatedComponents; ReferenceFrames = referenceFrames; DisposedComponentIDs = disposedComponentIDs; + DisposedEventHandlerIDs = disposedEventHandlerIDs; } } } diff --git a/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatchBuilder.cs b/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatchBuilder.cs index 9892e6d7d..b3ecada84 100644 --- a/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatchBuilder.cs +++ b/src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatchBuilder.cs @@ -45,6 +45,7 @@ public RenderBatch ToBatch() => new RenderBatch( UpdatedComponentDiffs.ToRange(), ReferenceFramesBuffer.ToRange(), - DisposedComponentIds.ToRange()); + DisposedComponentIds.ToRange(), + DisposedEventHandlerIds.ToRange()); } } diff --git a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs index 82897365d..7d7d7ea64 100644 --- a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs @@ -85,6 +85,30 @@ public void CanTriggerKeyPressEvents() li => Assert.Equal("b", li.Text)); } + [Fact] + public void CanAddAndRemoveEventHandlersDynamically() + { + var appElement = MountTestComponent(); + var countDisplayElement = appElement.FindElement(By.TagName("p")); + var incrementButton = appElement.FindElement(By.TagName("button")); + var toggleClickHandlerCheckbox = appElement.FindElement(By.CssSelector("[type=checkbox]")); + + // Initial count is zero; clicking button increments count + Assert.Equal("Current count: 0", countDisplayElement.Text); + incrementButton.Click(); + Assert.Equal("Current count: 1", countDisplayElement.Text); + + // We can remove an event handler + toggleClickHandlerCheckbox.Click(); + incrementButton.Click(); + Assert.Equal("Current count: 1", countDisplayElement.Text); + + // We can add an event handler + toggleClickHandlerCheckbox.Click(); + incrementButton.Click(); + Assert.Equal("Current count: 2", countDisplayElement.Text); + } + [Fact] public void CanRenderChildComponents() { diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs index f9a708f24..0432f4a86 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs @@ -434,7 +434,8 @@ public void RecognizesAttributeEventHandlerValuesChanged() newTree.CloseElement(); // Act - var (result, referenceFrames) = GetSingleUpdatedComponent(); + var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(initializeFromFrames: true); + var removedEventHandlerFrame = oldTree.GetFrames().Array[2]; // Assert Assert.Collection(result.Edits, @@ -444,6 +445,10 @@ public void RecognizesAttributeEventHandlerValuesChanged() Assert.Equal(0, entry.ReferenceFrameIndex); }); AssertFrame.Attribute(referenceFrames[0], "will change", addedHandler); + Assert.NotEqual(0, removedEventHandlerFrame.AttributeEventHandlerId); + Assert.Equal( + new[] { removedEventHandlerFrame.AttributeEventHandlerId }, + batch.DisposedEventHandlerIDs); } [Fact] @@ -1163,7 +1168,7 @@ public void PreservesEventHandlerIdsForRetainedEventHandlers() newTree.CloseElement(); // Act - var (result, referenceFrames) = GetSingleUpdatedComponent(initializeFromFrames: true); + var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(initializeFromFrames: true); var oldAttributeFrame = oldTree.GetFrames().Array[1]; var newAttributeFrame = newTree.GetFrames().Array[1]; @@ -1173,6 +1178,7 @@ public void PreservesEventHandlerIdsForRetainedEventHandlers() AssertFrame.Attribute(newAttributeFrame, "ontest", retainedHandler); Assert.NotEqual(0, oldAttributeFrame.AttributeEventHandlerId); Assert.Equal(oldAttributeFrame.AttributeEventHandlerId, newAttributeFrame.AttributeEventHandlerId); + Assert.Empty(batch.DisposedEventHandlerIDs); } [Fact] @@ -1189,7 +1195,7 @@ public void PreservesEventHandlerIdsForRetainedEventHandlers_SlowPath() newTree.CloseElement(); // Act - var (result, referenceFrames) = GetSingleUpdatedComponent(initializeFromFrames: true); + var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(initializeFromFrames: true); var oldAttributeFrame = oldTree.GetFrames().Array[1]; var newAttributeFrame = newTree.GetFrames().Array[2]; @@ -1199,6 +1205,7 @@ public void PreservesEventHandlerIdsForRetainedEventHandlers_SlowPath() AssertFrame.Attribute(newAttributeFrame, "ontest", retainedHandler); Assert.NotEqual(0, oldAttributeFrame.AttributeEventHandlerId); Assert.Equal(oldAttributeFrame.AttributeEventHandlerId, newAttributeFrame.AttributeEventHandlerId); + Assert.Empty(batch.DisposedEventHandlerIDs); } [Fact] @@ -1317,11 +1324,17 @@ public void QueuesRemovedChildComponentsForDisposal() } private (RenderTreeDiff, RenderTreeFrame[]) GetSingleUpdatedComponent(bool initializeFromFrames = false) + { + var result = GetSingleUpdatedComponentWithBatch(initializeFromFrames); + return (result.Item1, result.Item2); + } + + private (RenderTreeDiff, RenderTreeFrame[], RenderBatch) GetSingleUpdatedComponentWithBatch(bool initializeFromFrames = false) { var batch = GetRenderedBatch(initializeFromFrames); var diffsInBatch = batch.UpdatedComponents; Assert.Equal(1, diffsInBatch.Count); - return (diffsInBatch.Array[0], batch.ReferenceFrames.ToArray()); + return (diffsInBatch.Array[0], batch.ReferenceFrames.ToArray(), batch); } private RenderBatch GetRenderedBatch(bool initializeFromFrames = false) diff --git a/test/testapps/BasicTestApp/CounterComponent.cshtml b/test/testapps/BasicTestApp/CounterComponent.cshtml index 6c3d9c5d5..d54b508aa 100644 --- a/test/testapps/BasicTestApp/CounterComponent.cshtml +++ b/test/testapps/BasicTestApp/CounterComponent.cshtml @@ -1,9 +1,15 @@ 

Counter

Current count: @currentCount

- +

+ + @functions { int currentCount = 0; + bool handleClicks = true; void IncrementCount() {