diff --git a/packages/@ember/-internals/glimmer/lib/renderer.ts b/packages/@ember/-internals/glimmer/lib/renderer.ts index e533c4042ba..1fa623e0118 100644 --- a/packages/@ember/-internals/glimmer/lib/renderer.ts +++ b/packages/@ember/-internals/glimmer/lib/renderer.ts @@ -353,7 +353,7 @@ export class Renderer extends BaseRenderer { } getBounds(component: View): { - parentElement: SimpleElement; + parentElement: SimpleNode; firstNode: SimpleNode; lastNode: SimpleNode; } { @@ -361,7 +361,7 @@ export class Renderer extends BaseRenderer { assert('object passed to getBounds must have the BOUNDS symbol as a property', bounds); - let parentElement = bounds.parentElement(); + let parentElement = bounds.parentNode(); let firstNode = bounds.firstNode(); let lastNode = bounds.lastNode(); diff --git a/packages/@ember/-internals/glimmer/lib/syntax/in-element.ts b/packages/@ember/-internals/glimmer/lib/syntax/in-element.ts index 5fe3f4c3527..8571d36092e 100644 --- a/packages/@ember/-internals/glimmer/lib/syntax/in-element.ts +++ b/packages/@ember/-internals/glimmer/lib/syntax/in-element.ts @@ -21,8 +21,13 @@ `{{in-element}}` requires a single positional argument: - - `destinationElement` -- the DOM element to render into. It must exist at the time - of rendering. + - `destinationElement` -- the DOM element, `DocumentFragment`, or `ShadowRoot` to + render into. It must exist at the time of rendering. + + When the destination is a `DocumentFragment`, appending the fragment to the DOM + moves its children into the new parent. Content rendered by `{{in-element}}` + follows that move: subsequent updates apply to the content in its new location, + and destroying the block removes the content from its new location. It also supports an optional named argument: diff --git a/packages/@glimmer-workspace/integration-tests/lib/dom/simple-utils.ts b/packages/@glimmer-workspace/integration-tests/lib/dom/simple-utils.ts index 0823f5e3302..389565225b6 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/dom/simple-utils.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/dom/simple-utils.ts @@ -21,12 +21,12 @@ import { clearElement } from '@glimmer/util'; import Serializer from '@simple-dom/serializer'; import voidMap from '@simple-dom/void-map'; -export function toInnerHTML(parent: SimpleElement | SimpleDocumentFragment): string { +export function toInnerHTML(parent: SimpleNode): string { const serializer = new Serializer(voidMap); return serializer.serializeChildren(parent); } -export function toOuterHTML(parent: SimpleElement | SimpleDocumentFragment): string { +export function toOuterHTML(parent: SimpleNode): string { const serializer = new Serializer(voidMap); return serializer.serialize(parent); } diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites.ts b/packages/@glimmer-workspace/integration-tests/lib/suites.ts index a3ecce2e83c..1526e13312d 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites.ts @@ -7,6 +7,7 @@ export * from './suites/entry-point'; export * from './suites/has-block'; export * from './suites/has-block-params'; export * from './suites/in-element'; +export * from './suites/in-element-document-fragment'; export * from './suites/initial-render'; export * from './suites/scope'; export * from './suites/shadowing'; diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/in-element-document-fragment.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/in-element-document-fragment.ts new file mode 100644 index 00000000000..ad35f4f0eb0 --- /dev/null +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/in-element-document-fragment.ts @@ -0,0 +1,352 @@ +import { RenderTest } from '../render-test'; +import { test } from '../test-decorator'; + +export class InElementDocumentFragmentSuite extends RenderTest { + static suiteName = '#in-element (DocumentFragment)'; + + @test + 'Renders curlies into a detached DocumentFragment'() { + const fragment = document.createDocumentFragment(); + + this.render('{{#in-element this.fragment}}[{{this.foo}}]{{/in-element}}', { + fragment, + foo: 'Hello Fragment!', + }); + + this.assert.strictEqual( + fragment.textContent, + '[Hello Fragment!]', + 'content rendered in document fragment' + ); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ foo: 'Updated!' }); + this.assert.strictEqual( + fragment.textContent, + '[Updated!]', + 'content updated in document fragment' + ); + this.assertHTML(''); + + this.rerender({ foo: 'Hello Fragment!' }); + this.assert.strictEqual( + fragment.textContent, + '[Hello Fragment!]', + 'content reverted in document fragment' + ); + this.assertHTML(''); + } + + @test + 'Renders curlies into a template.content fragment'() { + const templateEl = document.createElement('template'); + const fragment = templateEl.content; + + this.render('{{#in-element this.fragment}}[{{this.foo}}]{{/in-element}}', { + fragment, + foo: 'Hello Template Content!', + }); + + this.assert.strictEqual( + fragment.textContent, + '[Hello Template Content!]', + 'content rendered in template.content fragment' + ); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ foo: 'Updated!' }); + this.assert.strictEqual( + fragment.textContent, + '[Updated!]', + 'content updated in template.content fragment' + ); + this.assertHTML(''); + + this.rerender({ foo: 'Hello Template Content!' }); + this.assert.strictEqual( + fragment.textContent, + '[Hello Template Content!]', + 'content reverted in template.content fragment' + ); + this.assertHTML(''); + } + + @test + 'Renders elements into a fragment that is later attached to the DOM'() { + const fragment = document.createDocumentFragment(); + const container = document.createElement('div'); + + this.render('{{#in-element this.fragment}}

{{this.message}}

{{/in-element}}', { + fragment, + message: 'in fragment', + }); + + this.assert.strictEqual( + fragment.querySelector('#frag-p')?.textContent, + 'in fragment', + 'content rendered in detached fragment' + ); + this.assertHTML(''); + + // Attach fragment's children to the DOM + container.appendChild(fragment); + this.assert.strictEqual( + container.querySelector('#frag-p')?.textContent, + 'in fragment', + 'content is in the DOM after fragment is appended' + ); + // Fragment itself is now empty (children moved to container) + this.assert.strictEqual(fragment.childNodes.length, 0, 'fragment is empty after append'); + } + + @test + 'Multiple in-element calls to the same DocumentFragment'() { + const fragment = document.createDocumentFragment(); + + this.render( + '{{#in-element this.fragment}}[{{this.foo}}]{{/in-element}}' + + '{{#in-element this.fragment insertBefore=null}}[{{this.bar}}]{{/in-element}}', + { + fragment, + foo: 'first', + bar: 'second', + } + ); + + this.assert.ok(fragment.textContent?.includes('[first]'), 'first block present in fragment'); + this.assert.ok(fragment.textContent?.includes('[second]'), 'second block present in fragment'); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ foo: 'updated-first', bar: 'updated-second' }); + this.assert.ok( + fragment.textContent?.includes('[updated-first]'), + 'first block updated in fragment' + ); + this.assert.ok( + fragment.textContent?.includes('[updated-second]'), + 'second block updated in fragment' + ); + this.assertHTML(''); + } + + @test + 'Rerenders work after DocumentFragment is appended to the DOM'(assert: typeof QUnit.assert) { + const fragment = document.createDocumentFragment(); + const container = document.createElement('div'); + const step = (text: string) => { + assert.step(text); + return text; + }; + + this.render( + '{{#in-element this.fragment}}' + + '

{{this.step this.message}}

' + + '{{#if this.show}}' + + 'extra {{this.step "extra rendered"}}' + + '{{/if}}' + + '{{/in-element}}', + { + fragment, + message: 'initial', + show: false, + step, + } + ); + + assert.verifySteps(['initial'], 'initial render fires step from inside fragment'); + + // Move the fragment's children into the container. After this the fragment is + // empty, but the rendered nodes (including Glimmer's bounds markers) are live + // children of `container`. + container.appendChild(fragment); + assert.strictEqual(fragment.childNodes.length, 0, 'fragment is empty after append'); + assert.ok(container.querySelector('#msg'), 'paragraph is present in container after append'); + + // Rerenders should continue to work after the fragment is attached — Glimmer + // resolves the live parent from the bounds markers' actual parentNode. + this.rerender({ message: 'updated' }); + assert.verifySteps(['updated'], 'text update fires step after fragment was attached to DOM'); + assert.strictEqual( + container.querySelector('#msg')?.textContent, + 'updated', + 'paragraph text is updated in container' + ); + + // New conditional element should appear in the container. + this.rerender({ show: true }); + assert.verifySteps( + ['extra rendered'], + 'conditional element step fires in container after fragment was attached to DOM' + ); + assert.ok( + container.querySelector('#extra'), + 'conditional span appears in container after fragment was attached to DOM' + ); + } + + @test + 'Conditional content follows the fragment after it is attached to the DOM'() { + const fragment = document.createDocumentFragment(); + const container = document.createElement('div'); + + this.render( + '{{#in-element this.fragment}}' + + '

stable

' + + '{{#if this.show}}cond{{/if}}' + + '{{/in-element}}', + { + fragment, + show: false, + } + ); + + container.appendChild(fragment); + + // The conditional block was empty at attach time; toggling it on must + // render the new element into the container (where the surrounding content + // now lives), not into the now-empty fragment. + this.rerender({ show: true }); + this.assert.ok(container.querySelector('#cond'), 'conditional element rendered in container'); + this.assert.strictEqual(fragment.childNodes.length, 0, 'nothing was rendered into fragment'); + this.assert.strictEqual( + container.querySelector('#stable')?.nextSibling?.nodeName, + 'SPAN', + 'conditional element is in position, next to the stable element' + ); + + this.rerender({ show: false }); + this.assert.notOk(container.querySelector('#cond'), 'conditional element removed'); + this.assert.ok(container.querySelector('#stable'), 'stable element remains'); + this.assert.strictEqual(fragment.childNodes.length, 0, 'fragment is still empty'); + } + + @test + 'Destroying {{#in-element}} clears the container after the fragment is attached'() { + const fragment = document.createDocumentFragment(); + const container = document.createElement('div'); + + this.render( + '{{#if this.showing}}' + + '{{#in-element this.fragment}}

hello

{{/in-element}}' + + '{{/if}}', + { + fragment, + showing: true, + } + ); + + container.appendChild(fragment); + this.assert.ok(container.querySelector('#content'), 'content is in container after append'); + + // Tearing down the {{#in-element}} must remove the content from wherever it + // currently lives (the container), not from the stale fragment. + this.rerender({ showing: false }); + this.assert.notOk(container.querySelector('#content'), 'content removed from container'); + this.assert.strictEqual(container.childNodes.length, 0, 'container is empty after destroy'); + + this.rerender({ showing: true }); + this.assert.strictEqual( + fragment.querySelector('#content')?.textContent, + 'hello', + 'content renders into the fragment again when in-element comes back' + ); + } + + @test + '{{#each}} updates follow the content after the fragment is attached'() { + const fragment = document.createDocumentFragment(); + const container = document.createElement('div'); + const text = (parent: ParentNode) => + Array.from(parent.querySelectorAll('span.item')) + .map((node) => node.textContent) + .join(''); + + this.render( + '{{#in-element this.fragment}}' + + '{{#each this.items as |item|}}{{item}}{{/each}}' + + '{{/in-element}}', + { + fragment, + items: ['a', 'b'], + } + ); + + this.assert.strictEqual(text(fragment), 'ab', 'initial items rendered into fragment'); + + container.appendChild(fragment); + + this.rerender({ items: ['a', 'b', 'c'] }); + this.assert.strictEqual(text(container), 'abc', 'new item appended in container'); + this.assert.strictEqual(fragment.childNodes.length, 0, 'nothing was rendered into fragment'); + + this.rerender({ items: ['c', 'b', 'a'] }); + this.assert.strictEqual(text(container), 'cba', 'items reordered in container'); + this.assert.strictEqual(fragment.childNodes.length, 0, 'fragment is still empty'); + + this.rerender({ items: ['b'] }); + this.assert.strictEqual(text(container), 'b', 'items removed in container'); + } + + @test + 'Renders into a ShadowRoot (a DocumentFragment subtype)'() { + const host = document.createElement('div'); + const shadowRoot = host.attachShadow({ mode: 'open' }); + + this.render( + '{{#in-element this.shadowRoot}}

{{this.message}}

{{/in-element}}', + { + shadowRoot, + message: 'hello', + } + ); + + this.assert.strictEqual( + shadowRoot.querySelector('#in-shadow')?.textContent, + 'hello', + 'content rendered into shadow root' + ); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ message: 'updated' }); + this.assert.strictEqual( + shadowRoot.querySelector('#in-shadow')?.textContent, + 'updated', + 'content updated in shadow root' + ); + } + + @test + 'Multiple in-element calls to the same DocumentFragment with insertBefore=null'() { + const fragment = document.createDocumentFragment(); + + this.render( + '{{#in-element this.fragment insertBefore=null}}

{{this.foo}}

{{/in-element}}' + + '{{#in-element this.fragment insertBefore=null}}

{{this.bar}}

{{/in-element}}', + { + fragment, + foo: 'first', + bar: 'second', + } + ); + + // Use childNodes to traverse the fragment's direct children since glimmer also + // inserts comment marker nodes alongside the rendered elements. + const nodes = Array.from(fragment.childNodes); + const pA = nodes.find((n) => (n as Element).id === 'a') as HTMLElement | undefined; + const pB = nodes.find((n) => (n as Element).id === 'b') as HTMLElement | undefined; + + this.assert.strictEqual(pA?.textContent, 'first', 'first block appended to fragment'); + this.assert.strictEqual(pB?.textContent, 'second', 'second block appended to fragment'); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ foo: 'updated-first', bar: 'updated-second' }); + this.assert.strictEqual(pA?.textContent, 'updated-first', 'first block updated in fragment'); + this.assert.strictEqual(pB?.textContent, 'updated-second', 'second block updated in fragment'); + this.assertHTML(''); + } +} diff --git a/packages/@glimmer-workspace/integration-tests/test/ember-component-test.ts b/packages/@glimmer-workspace/integration-tests/test/ember-component-test.ts index 5f1c5f4c509..56728ebf5de 100644 --- a/packages/@glimmer-workspace/integration-tests/test/ember-component-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/ember-component-test.ts @@ -2165,7 +2165,7 @@ class CurlyBoundsTrackingTest extends CurlyTest { const { bounds, element } = instance.captured; assert.strictEqual( - bounds.parentElement(), + bounds.parentNode(), document.querySelector('#qunit-fixture') as unknown as SimpleElement ); assert.strictEqual(bounds.firstNode(), castToSimple(element)); @@ -2201,7 +2201,7 @@ class CurlyBoundsTrackingTest extends CurlyTest { const { bounds } = instance.captured; assert.strictEqual( - check(bounds.parentElement(), HTMLElement), + check(bounds.parentNode(), HTMLElement), check(document.querySelector('#qunit-fixture'), HTMLElement) ); assert.strictEqual( diff --git a/packages/@glimmer-workspace/integration-tests/test/jit-suites-test.ts b/packages/@glimmer-workspace/integration-tests/test/jit-suites-test.ts index 69afb5a202c..31d0c9a14ac 100644 --- a/packages/@glimmer-workspace/integration-tests/test/jit-suites-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/jit-suites-test.ts @@ -5,6 +5,7 @@ import { GlimmerishComponents, HasBlockParamsHelperSuite, HasBlockSuite, + InElementDocumentFragmentSuite, InElementSuite, jitComponentSuite, jitSuite, @@ -18,6 +19,7 @@ import { jitComponentSuite(DebuggerSuite); jitSuite(EachSuite); jitSuite(InElementSuite); +jitSuite(InElementDocumentFragmentSuite); jitComponentSuite(GlimmerishComponents); jitComponentSuite(TemplateOnlyComponents); diff --git a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts index 47919a1e7ac..3a7dd336a26 100644 --- a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts @@ -20,7 +20,7 @@ import type { export interface AppendingBlock extends Bounds { debug?: { first: () => Nullable; last: () => Nullable }; - openElement(element: SimpleElement): void; + openElement(element: SimpleNode): void; closeElement(): void; didAppendNode(node: SimpleNode): void; didAppendBounds(bounds: Bounds): void; @@ -48,11 +48,7 @@ export interface ResettableBlock extends FixedBlock { } export interface DOMStack { - pushRemoteElement( - element: SimpleElement, - guid: string, - insertBefore: Maybe - ): FixedBlock; + pushRemoteElement(element: SimpleNode, guid: string, insertBefore: Maybe): FixedBlock; popRemoteElement(): FixedBlock; popElement(): void; openElement(tag: string, _operations?: ElementOperations): SimpleElement; @@ -101,7 +97,7 @@ export interface TreeBuilder extends Cursor, DOMStack, TreeOperations { dom: GlimmerTreeConstruction; updateOperations: GlimmerTreeChanges; constructing: Nullable; - element: SimpleElement; + element: SimpleNode; hasBlocks: boolean; debugBlocks(): AppendingBlock[]; diff --git a/packages/@glimmer/interfaces/lib/dom/bounds.d.ts b/packages/@glimmer/interfaces/lib/dom/bounds.d.ts index e1c97f3fe5f..62cf147befc 100644 --- a/packages/@glimmer/interfaces/lib/dom/bounds.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/bounds.d.ts @@ -1,14 +1,16 @@ import type { Nullable } from '../core.js'; -import type { SimpleElement, SimpleNode } from './simple.js'; +import type { SimpleNode } from './simple.js'; export interface Bounds { - // a method to future-proof for wormholing; may not be needed ultimately - parentElement(): SimpleElement; + // Like DOM's parentNode, this may be an Element, a DocumentFragment, or a + // Document (DOM's parentElement is always an Element or null, which is why + // this is not named parentElement). + parentNode(): SimpleNode; firstNode(): SimpleNode; lastNode(): SimpleNode; } export interface Cursor { - readonly element: SimpleElement; + readonly element: SimpleNode; readonly nextSibling: Nullable; } diff --git a/packages/@glimmer/interfaces/lib/dom/changes.d.ts b/packages/@glimmer/interfaces/lib/dom/changes.d.ts index a847cbeeb36..43c39a174d1 100644 --- a/packages/@glimmer/interfaces/lib/dom/changes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/changes.d.ts @@ -3,9 +3,9 @@ import type { Bounds } from './bounds.js'; import type { Namespace, SimpleComment, SimpleElement, SimpleNode, SimpleText } from './simple.js'; export interface GlimmerDOMOperations { - createElement(tag: string, context?: SimpleElement): SimpleElement; - insertBefore(parent: SimpleElement, node: SimpleNode, reference: Nullable): void; - insertHTMLBefore(parent: SimpleElement, nextSibling: Nullable, html: string): Bounds; + createElement(tag: string, context?: SimpleNode): SimpleElement; + insertBefore(parent: SimpleNode, node: SimpleNode, reference: Nullable): void; + insertHTMLBefore(parent: SimpleNode, nextSibling: Nullable, html: string): Bounds; createTextNode(text: string): SimpleText; createComment(data: string): SimpleComment; } @@ -13,7 +13,7 @@ export interface GlimmerDOMOperations { export interface GlimmerTreeChanges extends GlimmerDOMOperations { setAttribute(element: SimpleElement, name: string, value: string): void; removeAttribute(element: SimpleElement, name: string): void; - insertAfter(element: SimpleElement, node: SimpleNode, reference: SimpleNode): void; + insertAfter(element: SimpleNode, node: SimpleNode, reference: SimpleNode): void; } export interface GlimmerTreeConstruction extends GlimmerDOMOperations { diff --git a/packages/@glimmer/interfaces/lib/dom/tree-construction.d.ts b/packages/@glimmer/interfaces/lib/dom/tree-construction.d.ts index 75570d944d6..96f9b63c5da 100644 --- a/packages/@glimmer/interfaces/lib/dom/tree-construction.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/tree-construction.d.ts @@ -1,4 +1,4 @@ -import type { Namespace, SimpleDocumentFragment, SimpleElement, SimpleNode } from './simple.js'; +import type { Namespace, SimpleNode } from './simple.js'; export type NodeToken = number; @@ -17,5 +17,5 @@ export interface SpecTreeConstruction { appendComment(text: string): NodeToken; setAttribute(name: string, value: string, namespace?: Namespace): void; - appendTo(parent: SimpleElement | SimpleDocumentFragment): NodeTokens; + appendTo(parent: SimpleNode): NodeTokens; } diff --git a/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts b/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts index 7f5bcc2c9ec..635ec6e2346 100644 --- a/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts @@ -1,4 +1,4 @@ -import type { SimpleElement, SimpleNode } from '@simple-dom/interface'; +import type { SimpleNode } from '@simple-dom/interface'; import type { Bounds } from '../dom/bounds.js'; import type { Arguments, CapturedArguments } from './arguments.js'; @@ -25,7 +25,7 @@ export interface CapturedRenderNode { args: Arguments; instance: unknown; bounds: null | { - parentElement: SimpleElement; + parentElement: SimpleNode; firstNode: SimpleNode; lastNode: SimpleNode; }; diff --git a/packages/@glimmer/interfaces/lib/runtime/render.d.ts b/packages/@glimmer/interfaces/lib/runtime/render.d.ts index bd8ad0a2023..53f3a613313 100644 --- a/packages/@glimmer/interfaces/lib/runtime/render.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/render.d.ts @@ -1,4 +1,4 @@ -import type { SimpleElement, SimpleNode } from '@simple-dom/interface'; +import type { SimpleNode } from '@simple-dom/interface'; import type { RichIteratorResult } from '../core.js'; import type { Bounds } from '../dom/bounds.js'; @@ -14,7 +14,7 @@ export interface RenderResult extends Bounds, ExceptionHandler { rerender(options?: { alwaysRevalidate: false }): void; - parentElement(): SimpleElement; + parentNode(): SimpleNode; firstNode(): SimpleNode; lastNode(): SimpleNode; diff --git a/packages/@glimmer/node/lib/serialize-builder.ts b/packages/@glimmer/node/lib/serialize-builder.ts index b75499cb299..25e1b15403d 100644 --- a/packages/@glimmer/node/lib/serialize-builder.ts +++ b/packages/@glimmer/node/lib/serialize-builder.ts @@ -34,9 +34,12 @@ class SerializeBuilder extends NewTreeBuilder implements TreeBuilder { private serializeBlockDepth = 0; override __openBlock(): void { - let { tagName } = this.element; - - if (tagName !== 'TITLE' && tagName !== 'SCRIPT' && tagName !== 'STYLE') { + if ( + 'tagName' in this.element && + this.element.tagName !== 'TITLE' && + this.element.tagName !== 'SCRIPT' && + this.element.tagName !== 'STYLE' + ) { let depth = this.serializeBlockDepth++; this.__appendComment(`%+b:${depth}%`); } @@ -45,26 +48,32 @@ class SerializeBuilder extends NewTreeBuilder implements TreeBuilder { } override __closeBlock(): void { - let { tagName } = this.element; - super.__closeBlock(); - if (tagName !== 'TITLE' && tagName !== 'SCRIPT' && tagName !== 'STYLE') { + if ( + 'tagName' in this.element && + this.element.tagName !== 'TITLE' && + this.element.tagName !== 'SCRIPT' && + this.element.tagName !== 'STYLE' + ) { let depth = --this.serializeBlockDepth; this.__appendComment(`%-b:${depth}%`); } } override __appendHTML(html: string): Bounds { - let { tagName } = this.element; - - if (tagName === 'TITLE' || tagName === 'SCRIPT' || tagName === 'STYLE') { + if ( + 'tagName' in this.element && + (this.element.tagName === 'TITLE' || + this.element.tagName === 'SCRIPT' || + this.element.tagName === 'STYLE') + ) { return super.__appendHTML(html); } // Do we need to run the html tokenizer here? let first = this.__appendComment('%glmr%'); - if (tagName === 'TABLE') { + if ('tagName' in this.element && this.element.tagName === 'TABLE') { let openIndex = html.indexOf('<'); if (openIndex > -1) { let tr = html.slice(openIndex + 1, openIndex + 3); @@ -84,10 +93,14 @@ class SerializeBuilder extends NewTreeBuilder implements TreeBuilder { } override __appendText(string: string): SimpleText { - let { tagName } = this.element; let current = currentNode(this); - if (tagName === 'TITLE' || tagName === 'SCRIPT' || tagName === 'STYLE') { + if ( + 'tagName' in this.element && + (this.element.tagName === 'TITLE' || + this.element.tagName === 'SCRIPT' || + this.element.tagName === 'STYLE') + ) { return super.__appendText(string); } else if (string === '') { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -111,6 +124,7 @@ class SerializeBuilder extends NewTreeBuilder implements TreeBuilder { override openElement(tag: string) { if (tag === 'tr') { if ( + 'tagName' in this.element && this.element.tagName !== 'TBODY' && this.element.tagName !== 'THEAD' && this.element.tagName !== 'TFOOT' @@ -130,7 +144,7 @@ class SerializeBuilder extends NewTreeBuilder implements TreeBuilder { } override pushRemoteElement( - element: SimpleElement, + element: SimpleNode, cursorId: string, insertBefore: Maybe = null ): RemoteBlock { @@ -144,7 +158,7 @@ class SerializeBuilder extends NewTreeBuilder implements TreeBuilder { export function serializeBuilder( env: Environment, - cursor: { element: SimpleElement; nextSibling: Nullable } + cursor: { element: SimpleNode; nextSibling: Nullable } ): TreeBuilder { return SerializeBuilder.forInitialRender(env, cursor); } diff --git a/packages/@glimmer/runtime/lib/bounds.ts b/packages/@glimmer/runtime/lib/bounds.ts index 224a45cb067..9e9b45a5613 100644 --- a/packages/@glimmer/runtime/lib/bounds.ts +++ b/packages/@glimmer/runtime/lib/bounds.ts @@ -1,10 +1,10 @@ -import type { Bounds, Cursor, Nullable, SimpleElement, SimpleNode } from '@glimmer/interfaces'; +import type { Bounds, Cursor, Nullable, SimpleNode } from '@glimmer/interfaces'; import { expect } from '@glimmer/debug-util/lib/platform-utils'; import { setLocalDebugType } from '@glimmer/debug-util/lib/debug-brand'; export class CursorImpl implements Cursor { constructor( - public element: SimpleElement, + public element: SimpleNode, public nextSibling: Nullable ) { setLocalDebugType('cursor', this); @@ -15,13 +15,13 @@ export type DestroyableBounds = Bounds; export class ConcreteBounds implements Bounds { constructor( - public parentNode: SimpleElement, + private parent: SimpleNode, private first: SimpleNode, private last: SimpleNode ) {} - parentElement(): SimpleElement { - return this.parentNode; + parentNode(): SimpleNode { + return this.parent; } firstNode(): SimpleNode { @@ -33,8 +33,16 @@ export class ConcreteBounds implements Bounds { } } +// The parent to mutate through: normally the stored parentNode(), but when the +// bounds were rendered into a DocumentFragment that was later appended to the +// DOM, the nodes' live parentNode is the container while the stored parent is +// the (now-empty) fragment. +function liveParent(bounds: Bounds): SimpleNode { + return bounds.firstNode().parentNode ?? bounds.parentNode(); +} + export function move(bounds: Bounds, reference: Nullable): Nullable { - let parent = bounds.parentElement(); + let parent = liveParent(bounds); let first = bounds.firstNode(); let last = bounds.lastNode(); @@ -54,7 +62,7 @@ export function move(bounds: Bounds, reference: Nullable): Nullable< } export function clear(bounds: Bounds): Nullable { - let parent = bounds.parentElement(); + let parent = liveParent(bounds); let first = bounds.firstNode(); let last = bounds.lastNode(); diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index e12d7f8fbfb..01e91bbda1b 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts @@ -30,10 +30,12 @@ import { } from '@glimmer/constants/lib/syscall-ops'; import { check, + CheckDocumentFragment, CheckElement, CheckMaybe, CheckNode, CheckNullable, + CheckOr, CheckString, } from '@glimmer/debug/lib/stack-check'; import debugToString from '@glimmer/debug-util/lib/debug-to-string'; @@ -77,7 +79,7 @@ APPEND_OPCODES.add(VM_PUSH_REMOTE_ELEMENT_OP, (vm) => { let insertBeforeRef = check(vm.stack.pop(), CheckReference); let guidRef = check(vm.stack.pop(), CheckReference); - let element = check(valueForRef(elementRef), CheckElement); + let element = check(valueForRef(elementRef), CheckOr(CheckElement, CheckDocumentFragment)); let insertBefore = check(valueForRef(insertBeforeRef), CheckMaybe(CheckNullable(CheckNode))); let guid = valueForRef(guidRef) as string; diff --git a/packages/@glimmer/runtime/lib/debug-render-tree.ts b/packages/@glimmer/runtime/lib/debug-render-tree.ts index 8b09ce748d6..fd41ba5c1ac 100644 --- a/packages/@glimmer/runtime/lib/debug-render-tree.ts +++ b/packages/@glimmer/runtime/lib/debug-render-tree.ts @@ -191,7 +191,7 @@ export default class DebugRenderTreeImpl< private captureBounds(node: InternalRenderNode): CapturedRenderNode['bounds'] { let bounds = expect(node.bounds, 'BUG: missing bounds'); - let parentElement = bounds.parentElement(); + let parentElement = bounds.parentNode(); let firstNode = bounds.firstNode(); let lastNode = bounds.lastNode(); return { parentElement, firstNode, lastNode }; diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index 1e3a21359bc..3a3f7ec27b4 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -63,8 +63,8 @@ export class Fragment implements Bounds { this.bounds = bounds; } - parentElement(): SimpleElement { - return this.bounds.parentElement(); + parentNode(): SimpleNode { + return this.bounds.parentNode(); } firstNode(): SimpleNode { @@ -98,7 +98,11 @@ export class NewTreeBuilder implements TreeBuilder { } static resume(env: Environment, block: ResettableBlock): NewTreeBuilder { - let parentNode = block.parentElement(); + // Capture the live parent before resetting, because the bounds may have been + // rendered into a DocumentFragment that was subsequently appended to a real + // DOM container. In that case firstNode().parentNode is the container while + // parentNode() still returns the original (now-empty) fragment. + let parentNode = block.firstNode().parentNode ?? block.parentNode(); let nextSibling = block.reset(env); let stack = new this(env, parentNode, nextSibling).initialize(); @@ -107,7 +111,7 @@ export class NewTreeBuilder implements TreeBuilder { return stack; } - constructor(env: Environment, parentNode: SimpleElement, nextSibling: Nullable) { + constructor(env: Environment, parentNode: SimpleNode, nextSibling: Nullable) { this.pushElement(parentNode, nextSibling); this.env = env; this.dom = env.getAppendOperations(); @@ -131,7 +135,7 @@ export class NewTreeBuilder implements TreeBuilder { return this.blockStack.toArray(); } - get element(): SimpleElement { + get element(): SimpleNode { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme return this.cursors.current!.element; } @@ -218,7 +222,7 @@ export class NewTreeBuilder implements TreeBuilder { this.didOpenElement(element); } - __flushElement(parent: SimpleElement, constructing: SimpleElement) { + __flushElement(parent: SimpleNode, constructing: SimpleElement) { this.dom.insertBefore(parent, constructing, this.nextSibling); } @@ -229,7 +233,7 @@ export class NewTreeBuilder implements TreeBuilder { } pushRemoteElement( - element: SimpleElement, + element: SimpleNode, guid: string, insertBefore: Maybe ): RemoteBlock { @@ -237,7 +241,7 @@ export class NewTreeBuilder implements TreeBuilder { } __pushRemoteElement( - element: SimpleElement, + element: SimpleNode, _guid: string, insertBefore: Maybe ): RemoteBlock { @@ -261,7 +265,7 @@ export class NewTreeBuilder implements TreeBuilder { return block; } - protected pushElement(element: SimpleElement, nextSibling: Maybe = null): void { + protected pushElement(element: SimpleNode, nextSibling: Maybe = null): void { this.cursors.push(new CursorImpl(element, nextSibling)); } @@ -402,7 +406,7 @@ export class AppendingBlockImpl implements AppendingBlock { protected last: Nullable = null; protected nesting = 0; - constructor(private parent: SimpleElement) { + constructor(private parent: SimpleNode) { setLocalDebugType('block:simple', this); if (LOCAL_DEBUG) { @@ -413,7 +417,7 @@ export class AppendingBlockImpl implements AppendingBlock { } } - parentElement() { + parentNode() { return this.parent; } @@ -472,7 +476,7 @@ export class AppendingBlockImpl implements AppendingBlock { } export class RemoteBlock extends AppendingBlockImpl { - constructor(parent: SimpleElement) { + constructor(parent: SimpleNode) { super(parent); setLocalDebugType('block:remote', this); @@ -502,7 +506,12 @@ export class RemoteBlock extends AppendingBlockImpl { // and avoid clearing the node if it was. In most cases this shouldn't happen, // so this might hide bugs where the code clears nested nodes unnecessarily, // so we should eventually try to do the correct fix. - if (this.parentElement() === this.firstNode().parentNode) { + // + // Note: we check firstNode().parentNode !== null (node still has a parent) + // rather than === parentNode() (node is in the original parent), so that + // {{#in-element}} into a DocumentFragment still clears correctly after the + // fragment's children are moved to a real DOM container via appendChild(). + if (this.firstNode().parentNode !== null) { clear(this); } }); @@ -510,7 +519,7 @@ export class RemoteBlock extends AppendingBlockImpl { } export class ResettableBlockImpl extends AppendingBlockImpl implements ResettableBlock { - constructor(parent: SimpleElement) { + constructor(parent: SimpleNode) { super(parent); setLocalDebugType('block:resettable', this); } @@ -530,14 +539,14 @@ export class ResettableBlockImpl extends AppendingBlockImpl implements Resettabl // FIXME: All the noops in here indicate a modelling problem export class AppendingBlockList implements AppendingBlock { constructor( - private readonly parent: SimpleElement, + private readonly parent: SimpleNode, public boundList: AppendingBlock[] ) { this.parent = parent; this.boundList = boundList; } - parentElement() { + parentNode() { return this.parent; } diff --git a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts index ac6ab04ae8d..beb17df8f1d 100644 --- a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts @@ -198,15 +198,18 @@ export class RehydrateTree extends NewTreeBuilder implements TreeBuilder { const { candidate } = currentCursor; if (candidate === null) return; - const { tagName } = currentCursor.element; - if ( isOpenBlock(candidate) && getBlockDepthWithOffset(candidate, this.startingBlockOffset) === blockDepth ) { this.candidate = this.remove(candidate); currentCursor.openBlockDepth = blockDepth; - } else if (tagName !== 'TITLE' && tagName !== 'SCRIPT' && tagName !== 'STYLE') { + } else if ( + 'tagName' in currentCursor.element && + currentCursor.element.tagName !== 'TITLE' && + currentCursor.element.tagName !== 'SCRIPT' && + currentCursor.element.tagName !== 'STYLE' + ) { this.clearMismatch(candidate); } } diff --git a/packages/@glimmer/runtime/lib/vm/render-result.ts b/packages/@glimmer/runtime/lib/vm/render-result.ts index 32dbca888d0..c428a9d58d5 100644 --- a/packages/@glimmer/runtime/lib/vm/render-result.ts +++ b/packages/@glimmer/runtime/lib/vm/render-result.ts @@ -2,7 +2,6 @@ import type { AppendingBlock, Environment, RenderResult, - SimpleElement, SimpleNode, UpdatingOpcode, } from '@glimmer/interfaces'; @@ -29,8 +28,8 @@ export default class RenderResultImpl implements RenderResult { vm.execute(updating, this); } - parentElement(): SimpleElement { - return this.bounds.parentElement(); + parentNode(): SimpleNode { + return this.bounds.parentNode(); } firstNode(): SimpleNode { diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 92981cc0531..54b638e5f1e 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -125,8 +125,8 @@ export abstract class BlockOpcode implements UpdatingOpcode, Bounds { this.bounds = bounds; } - parentElement() { - return this.bounds.parentElement(); + parentNode() { + return this.bounds.parentNode(); } firstNode() { @@ -232,15 +232,15 @@ export class ListBlockOpcode extends BlockOpcode { let { dom } = vm; let marker = (this.marker = dom.createComment('')); - dom.insertAfter( - bounds.parentElement(), - marker, - expect(bounds.lastNode(), "can't insert after an empty bounds") - ); + // Anchor on the last node's live parent rather than the stored parent: + // they differ when the content was rendered into a DocumentFragment that + // was later appended to the DOM. + let lastNode = expect(bounds.lastNode(), "can't insert after an empty bounds"); + dom.insertAfter(lastNode.parentNode ?? bounds.parentNode(), marker, lastNode); this.sync(iterator); - this.parentElement().removeChild(marker); + marker.parentNode?.removeChild(marker); this.marker = null; this.lastIterator = iterator; } @@ -356,8 +356,13 @@ export class ListBlockOpcode extends BlockOpcode { let { key } = item; let nextSibling = before === undefined ? this.marker : before.firstNode(); + // The insertion reference (the sync marker or an existing item's first + // node) is live in the DOM; insert through its parent, which differs from + // the stored parent when the content was rendered into a DocumentFragment + // that was later appended to the DOM. bounds.firstNode() is not usable + // here: sync() empties the block list, so the bounds are mid-initialization. let elementStack = NewTreeBuilder.forInitialRender(env, { - element: bounds.parentElement(), + element: nextSibling?.parentNode ?? bounds.parentNode(), nextSibling, });