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 8ab59318cc..51ae8ee04a 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts @@ -4,13 +4,14 @@ import { getTreeFramePtr, renderTreeFrame, FrameType, RenderTreeFramePointer } f import { platform } from '../Environment'; import { EventDelegator } from './EventDelegator'; import { EventForDotNet, UIEventArgs } from './EventForDotNet'; +import { LogicalElement, toLogicalElement, insertLogicalChild, removeLogicalChild, getLogicalParent, getLogicalChild, createAndInsertLogicalContainer, isSvgElement } from './LogicalElements'; const selectValuePropname = '_blazorSelectValue'; let raiseEventMethod: MethodHandle; let renderComponentMethod: MethodHandle; export class BrowserRenderer { private eventDelegator: EventDelegator; - private childComponentLocations: { [componentId: number]: Element } = {}; + private childComponentLocations: { [componentId: number]: LogicalElement } = {}; constructor(private browserRendererId: number) { this.eventDelegator = new EventDelegator((event, componentId, eventHandlerId, eventArgs) => { @@ -19,7 +20,7 @@ export class BrowserRenderer { } public attachRootComponentToElement(componentId: number, element: Element) { - this.attachComponentToElement(componentId, element); + this.attachComponentToElement(componentId, toLogicalElement(element)); } public updateComponent(componentId: number, edits: System_Array, editsOffset: number, editsLength: number, referenceFrames: System_Array) { @@ -39,11 +40,11 @@ export class BrowserRenderer { this.eventDelegator.removeListener(eventHandlerId); } - private attachComponentToElement(componentId: number, element: Element) { + private attachComponentToElement(componentId: number, element: LogicalElement) { this.childComponentLocations[componentId] = element; } - private applyEdits(componentId: number, parent: Element, childIndex: number, edits: System_Array, editsOffset: number, editsLength: number, referenceFrames: System_Array) { + private applyEdits(componentId: number, parent: LogicalElement, childIndex: number, edits: System_Array, editsOffset: number, editsLength: number, referenceFrames: System_Array) { let currentDepth = 0; let childIndexAtCurrentDepth = childIndex; const maxEditIndexExcl = editsOffset + editsLength; @@ -60,41 +61,55 @@ export class BrowserRenderer { } case EditType.removeFrame: { const siblingIndex = renderTreeEdit.siblingIndex(edit); - removeNodeFromDOM(parent, childIndexAtCurrentDepth + siblingIndex); + removeLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex); break; } case EditType.setAttribute: { const frameIndex = renderTreeEdit.newTreeIndex(edit); const frame = getTreeFramePtr(referenceFrames, frameIndex); const siblingIndex = renderTreeEdit.siblingIndex(edit); - const element = parent.childNodes[childIndexAtCurrentDepth + siblingIndex] as HTMLElement; - this.applyAttribute(componentId, element, frame); + const element = getLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex); + if (element instanceof HTMLElement) { + this.applyAttribute(componentId, element, frame); + } else { + throw new Error(`Cannot set attribute on non-element child`); + } 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)!); + const element = getLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex); + if (element instanceof HTMLElement) { + const attributeName = renderTreeEdit.removedAttributeName(edit)!; + element.removeAttribute(attributeName); + } else { + throw new Error(`Cannot remove attribute from non-element child`); + } break; } case EditType.updateText: { const frameIndex = renderTreeEdit.newTreeIndex(edit); const frame = getTreeFramePtr(referenceFrames, frameIndex); const siblingIndex = renderTreeEdit.siblingIndex(edit); - const domTextNode = parent.childNodes[childIndexAtCurrentDepth + siblingIndex] as Text; - domTextNode.textContent = renderTreeFrame.textContent(frame); + const textNode = getLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex); + if (textNode instanceof Text) { + textNode.textContent = renderTreeFrame.textContent(frame); + } else { + throw new Error(`Cannot set text content on non-text child`); + } break; } case EditType.stepIn: { const siblingIndex = renderTreeEdit.siblingIndex(edit); - parent = parent.childNodes[childIndexAtCurrentDepth + siblingIndex] as HTMLElement; + parent = getLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex); currentDepth++; childIndexAtCurrentDepth = 0; break; } case EditType.stepOut: { - parent = parent.parentElement!; + parent = getLogicalParent(parent)!; currentDepth--; childIndexAtCurrentDepth = currentDepth === 0 ? childIndex : 0; // The childIndex is only ever nonzero at zero depth break; @@ -107,7 +122,7 @@ export class BrowserRenderer { } } - private insertFrame(componentId: number, parent: Element, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number): number { + private insertFrame(componentId: number, parent: LogicalElement, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number): number { const frameType = renderTreeFrame.frameType(frame); switch (frameType) { case FrameType.element: @@ -129,48 +144,31 @@ export class BrowserRenderer { } } - private insertElement(componentId: number, parent: Element, childIndex: number, frames: System_Array, frame: RenderTreeFramePointer, frameIndex: number) { + private insertElement(componentId: number, parent: LogicalElement, 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' ? + const newDomElementRaw = tagName === 'svg' || isSvgElement(parent) ? document.createElementNS('http://www.w3.org/2000/svg', tagName) : document.createElement(tagName); - insertNodeIntoDOM(newDomElement, parent, childIndex); + const newElement = toLogicalElement(newDomElementRaw); + insertLogicalChild(newDomElementRaw, parent, childIndex); // Apply attributes const descendantsEndIndexExcl = frameIndex + renderTreeFrame.subtreeLength(frame); for (let descendantIndex = frameIndex + 1; descendantIndex < descendantsEndIndexExcl; descendantIndex++) { const descendantFrame = getTreeFramePtr(frames, descendantIndex); if (renderTreeFrame.frameType(descendantFrame) === FrameType.attribute) { - this.applyAttribute(componentId, newDomElement, descendantFrame); + this.applyAttribute(componentId, newDomElementRaw, descendantFrame); } else { // As soon as we see a non-attribute child, all the subsequent child frames are // not attributes, so bail out and insert the remnants recursively - this.insertFrameRange(componentId, newDomElement, 0, frames, descendantIndex, descendantsEndIndexExcl); + this.insertFrameRange(componentId, newElement, 0, frames, descendantIndex, descendantsEndIndexExcl); break; } } } - 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. - // To improve on this in the future: - // - If we can statically detect that a given component always produces a single top-level - // element anyway, then don't wrap it in a further nonstandard element - // - If we really want to support child components producing multiple top-level frames and - // not being wrapped in a container at all, then every time a component is refreshed in - // the DOM, we could update an array on the parent element that specifies how many DOM - // nodes correspond to each of its render tree frames. Then when that parent wants to - // locate the first DOM node for a render tree frame, it can sum all the frame counts for - // all the preceding render trees frames. It's O(N), but where N is the number of siblings - // (counting child components as a single item), so N will rarely if ever be large. - // We could even keep track of whether all the child components happen to have exactly 1 - // top level frames, and in that case, there's no need to sum as we can do direct lookups. - const containerElement = parent.namespaceURI === 'http://www.w3.org/2000/svg' ? - document.createElementNS('http://www.w3.org/2000/svg', 'g') : - document.createElement('blazor-component'); - insertNodeIntoDOM(containerElement, parent, childIndex); + private insertComponent(parent: LogicalElement, childIndex: number, frame: RenderTreeFramePointer) { + const containerElement = createAndInsertLogicalContainer(parent, childIndex); // All we have to do is associate the child component ID with its location. We don't actually // do any rendering here, because the diff for the child will appear later in the render batch. @@ -178,10 +176,10 @@ export class BrowserRenderer { this.attachComponentToElement(childComponentId, containerElement); } - private insertText(parent: Element, childIndex: number, textFrame: RenderTreeFramePointer) { + private insertText(parent: LogicalElement, childIndex: number, textFrame: RenderTreeFramePointer) { const textContent = renderTreeFrame.textContent(textFrame)!; - const newDomTextNode = document.createTextNode(textContent); - insertNodeIntoDOM(newDomTextNode, parent, childIndex); + const newTextNode = document.createTextNode(textContent); + insertLogicalChild(newTextNode, parent, childIndex); } private applyAttribute(componentId: number, toDomElement: Element, attributeFrame: RenderTreeFramePointer) { @@ -246,7 +244,7 @@ export class BrowserRenderer { } } - private insertFrameRange(componentId: number, parent: Element, childIndex: number, frames: System_Array, startIndex: number, endIndexExcl: number): number { + private insertFrameRange(componentId: number, parent: LogicalElement, 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); @@ -268,23 +266,6 @@ function isCheckbox(element: Element) { return element.tagName === 'INPUT' && element.getAttribute('type') === 'checkbox'; } -function insertNodeIntoDOM(node: Node, parent: Element, childIndex: number) { - if (childIndex >= parent.childNodes.length) { - parent.appendChild(node); - } else { - parent.insertBefore(node, parent.childNodes[childIndex]); - } -} - -function removeNodeFromDOM(parent: Element, childIndex: number) { - parent.removeChild(parent.childNodes[childIndex]); -} - -function removeAttributeFromDOM(parent: Element, childIndex: number, attributeName: string) { - const element = parent.childNodes[childIndex] as Element; - element.removeAttribute(attributeName); -} - function raiseEvent(event: Event, browserRendererId: number, componentId: number, eventHandlerId: number, eventArgs: EventForDotNet) { event.preventDefault(); diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/LogicalElements.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/LogicalElements.ts new file mode 100644 index 0000000000..8f9bb06673 --- /dev/null +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/LogicalElements.ts @@ -0,0 +1,148 @@ +/* + A LogicalElement plays the same role as an Element instance from the point of view of the + API consumer. Inserting and removing logical elements updates the browser DOM just the same. + + The difference is that, unlike regular DOM mutation APIs, the LogicalElement APIs don't use + the underlying DOM structure as the data storage for the element hierarchy. Instead, the + LogicalElement APIs take care of tracking hierarchical relationships separately. The point + of this is to permit a logical tree structure in which parent/child relationships don't + have to be materialized in terms of DOM element parent/child relationships. And the reason + why we want that is so that hierarchies of Blazor components can be tracked even when those + components' render output need not be a single literal DOM element. + + Consumers of the API don't need to know about the implementation, but how it's done is: + - Each LogicalElement is materialized in the DOM as either: + - A Node instance, for actual Node instances inserted using 'insertLogicalChild' or + for Element instances promoted to LogicalElement via 'toLogicalElement' + - A Comment instance, for 'logical container' instances inserted using 'createAndInsertLogicalContainer' + - Then, on that instance (i.e., the Node or Comment), we store an array of 'logical children' + instances, e.g., + [firstChild, secondChild, thirdChild, ...] + ... plus we store a reference to the 'logical parent' (if any) + - The 'logical children' array means we can look up in O(1): + - The number of logical children (not currently implemented because not required, but trivial) + - The logical child at any given index + - Whenever a logical child is added or removed, we update the parent's array of logical children +*/ + +const logicalChildrenPropname = createSymbolOrFallback('_blazorLogicalChildren'); +const logicalParentPropname = createSymbolOrFallback('_blazorLogicalParent'); + +export function toLogicalElement(element: Element) { + if (element.childNodes.length > 0) { + throw new Error('New logical elements must start empty'); + } + + element[logicalChildrenPropname] = []; + return element as any as LogicalElement; +} + +export function createAndInsertLogicalContainer(parent: LogicalElement, childIndex: number): LogicalElement { + const containerElement = document.createComment('!'); + insertLogicalChild(containerElement, parent, childIndex); + return containerElement as any as LogicalElement; +} + +export function insertLogicalChild(child: Node, parent: LogicalElement, childIndex: number) { + const childAsLogicalElement = child as any as LogicalElement; + if (child instanceof Comment) { + const existingGrandchildren = getLogicalChildrenArray(childAsLogicalElement); + if (existingGrandchildren && getLogicalChildrenArray(childAsLogicalElement).length > 0) { + // There's nothing to stop us implementing support for this scenario, and it's not difficult + // (after inserting 'child' itself, also iterate through its logical children and physically + // put them as following-siblings in the DOM). However there's no scenario that requires it + // presently, so if we did implement it there'd be no good way to have tests for it. + throw new Error('Not implemented: inserting non-empty logical container'); + } + } + + if (getLogicalParent(childAsLogicalElement)) { + // Likewise, we could easily support this scenario too (in this 'if' block, just splice + // out 'child' from the logical children array of its previous logical parent by using + // Array.prototype.indexOf to determine its previous sibling index). + // But again, since there's not currently any scenario that would use it, we would not + // have any test coverage for such an implementation. + throw new Error('Not implemented: moving existing logical children'); + } + + const newSiblings = getLogicalChildrenArray(parent); + const newPhysicalParent = getClosestDomElement(parent); + if (childIndex < newSiblings.length) { + newPhysicalParent.insertBefore(child, newSiblings[childIndex] as any as Node); + newSiblings.splice(childIndex, 0, childAsLogicalElement); + } else { + if (parent instanceof Comment) { + const parentLogicalNextSibling = getLogicalNextSibling(parent); + if (parentLogicalNextSibling) { + newPhysicalParent.insertBefore(child, parentLogicalNextSibling as any as Node); + } else { + newPhysicalParent.appendChild(child); + } + } else { + newPhysicalParent.appendChild(child); + } + + newSiblings.push(childAsLogicalElement); + } + + childAsLogicalElement[logicalParentPropname] = parent; + if (!(logicalChildrenPropname in childAsLogicalElement)) { + childAsLogicalElement[logicalChildrenPropname] = []; + } +} + +export function removeLogicalChild(parent: LogicalElement, childIndex: number) { + const childrenArray = getLogicalChildrenArray(parent); + const childToRemove = childrenArray.splice(childIndex, 1)[0]; + + // If it's a logical container, also remove its descendants + if (childToRemove instanceof Comment) { + const grandchildrenArray = getLogicalChildrenArray(childToRemove); + while (grandchildrenArray.length > 0) { + removeLogicalChild(childToRemove, 0); + } + } + + // Finally, remove the node itself + const domNodeToRemove = childToRemove as any as Node; + domNodeToRemove.parentNode!.removeChild(domNodeToRemove); +} + +export function getLogicalParent(element: LogicalElement): LogicalElement | null { + return (element[logicalParentPropname] as LogicalElement) || null; +} + +export function getLogicalChild(parent: LogicalElement, childIndex: number): LogicalElement { + return getLogicalChildrenArray(parent)[childIndex]; +} + +export function isSvgElement(element: LogicalElement) { + return getClosestDomElement(element).namespaceURI === 'http://www.w3.org/2000/svg'; +} + +function getLogicalChildrenArray(element: LogicalElement) { + return element[logicalChildrenPropname] as LogicalElement[]; +} + +function getLogicalNextSibling(element: LogicalElement): LogicalElement | null { + const siblings = getLogicalChildrenArray(getLogicalParent(element)!); + const siblingIndex = Array.prototype.indexOf.call(siblings, element); + return siblings[siblingIndex + 1] || null; +} + +function getClosestDomElement(logicalElement: LogicalElement) { + if (logicalElement instanceof Element) { + return logicalElement; + } else if (logicalElement instanceof Comment) { + return logicalElement.parentNode! as Element; + } else { + throw new Error('Not a valid logical element'); + } +} + +function createSymbolOrFallback(fallback: string): symbol | string { + return typeof Symbol === 'function' ? Symbol() : fallback; +} + +// Nominal type to represent a logical element without needing to allocate any object for instances +export interface LogicalElement { LogicalElement__DO_NOT_IMPLEMENT: any }; 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 7e2b434583..04bacdc44c 100644 --- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts +++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts @@ -17,8 +17,8 @@ export function attachRootComponentToElement(browserRendererId: number, elementS if (!browserRenderer) { browserRenderer = browserRenderers[browserRendererId] = new BrowserRenderer(browserRendererId); } - browserRenderer.attachRootComponentToElement(componentId, element); clearElement(element); + browserRenderer.attachRootComponentToElement(componentId, element); } export function renderBatch(browserRendererId: number, batch: RenderBatchPointer) { diff --git a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs index 7d7d7ea648..0cdf238cee 100644 --- a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs @@ -116,12 +116,7 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests Assert.Equal("Parent component", appElement.FindElement(By.CssSelector("fieldset > legend")).Text); - // TODO: Once we remove the wrapper elements from around child components, - // assert that the child component text node is directly inside the
- var childComponentWrapper = appElement.FindElement(By.CssSelector("fieldset > blazor-component")); - Assert.Single(childComponentWrapper.FindElements(By.CssSelector("*"))); - - var styledElement = childComponentWrapper.FindElement(By.TagName("h1")); + var styledElement = appElement.FindElement(By.CssSelector("fieldset > h1")); Assert.Equal("Hello, world!", styledElement.Text); Assert.Equal("color: red;", styledElement.GetAttribute("style")); Assert.Equal("somevalue", styledElement.GetAttribute("customattribute")); @@ -131,13 +126,13 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests public void CanTriggerEventsOnChildComponents() { // Counter is displayed as child component. Initial count is zero. - var childComponentWrapper = MountTestComponent() - .FindElements(By.CssSelector("blazor-component")).Single(); - var counterDisplay = childComponentWrapper.FindElement(By.TagName("p")); - Assert.Equal("Current count: 0", counterDisplay.Text); + var appElement = MountTestComponent(); + var counterDisplay = appElement + .FindElements(By.TagName("p")) + .Single(element => element.Text == "Current count: 0"); // Clicking increments count in child component - childComponentWrapper.FindElement(By.TagName("button")).Click(); + appElement.FindElement(By.TagName("button")).Click(); Assert.Equal("Current count: 1", counterDisplay.Text); }