[BUGFIX] DocumentFragment support — {{#in-element}} with DocumentFragment targets

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';

packages/@glimmer-workspace/integration-tests/lib/suites/in-element-document-fragment.ts

@@ -0,0 +1,220 @@
+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}}<p id="frag-p">{{this.message}}</p>{{/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}}' +
+        '<p id="msg">{{this.step this.message}}</p>' +
+        '{{#if this.show}}' +
+        '<span id="extra">extra {{this.step "extra rendered"}}</span>' +
+        '{{/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
+  'Multiple in-element calls to the same DocumentFragment with insertBefore=null'() {
+    const fragment = document.createDocumentFragment();
+
+    this.render(
+      '{{#in-element this.fragment insertBefore=null}}<p id="a">{{this.foo}}</p>{{/in-element}}' +
+        '{{#in-element this.fragment insertBefore=null}}<p id="b">{{this.bar}}</p>{{/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('<!----><!---->');
+  }
+}

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);

packages/@glimmer/runtime/lib/bounds.ts

@@ -53,10 +53,15 @@ export function move(bounds: Bounds, reference: Nullable<SimpleNode>): Nullable<
 }
 
 export function clear(bounds: Bounds): Nullable<SimpleNode> {
-  let parent = bounds.parentElement();
   let first = bounds.firstNode();
   let last = bounds.lastNode();
 
+  // Use the node's actual current parent rather than the stored parentElement.
+  // When bounds were rendered into a DocumentFragment that was subsequently
+  // appended to a real DOM container, the nodes' parentNode is the container
+  // while parentElement() still returns the (now-empty) fragment.
+  let parent = (first.parentNode as Nullable<SimpleElement>) ?? bounds.parentElement();
+
   let current: SimpleNode = first;
 
   while (true) {

packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts

@@ -7,6 +7,7 @@ import type {
   ModifierInstance,
   Nullable,
   Owner,
+  SimpleElement,
   UpdatingOpcode,
   UpdatingVM,
 } from '@glimmer/interfaces';
@@ -29,10 +30,12 @@ import {
 } from '@glimmer/constants';
 import {
   check,
+  CheckDocumentFragment,
   CheckElement,
   CheckMaybe,
   CheckNode,
   CheckNullable,
+  CheckOr,
   CheckString,
 } from '@glimmer/debug';
 import { debugToString, expect } from '@glimmer/debug-util';
@@ -74,7 +77,10 @@ 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)
+  ) as SimpleElement;
   let insertBefore = check(valueForRef(insertBeforeRef), CheckMaybe(CheckNullable(CheckNode)));
   let guid = valueForRef(guidRef) as string;
 

packages/@glimmer/runtime/lib/vm/element-builder.ts

@@ -96,7 +96,12 @@ 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
+    // parentElement() still returns the original (now-empty) fragment.
+    let parentNode =
+      (block.firstNode().parentNode as SimpleElement | null) ?? block.parentElement();
     let nextSibling = block.reset(env);
 
     let stack = new this(env, parentNode, nextSibling).initialize();
@@ -500,7 +505,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 === parentElement() (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);
       }
     });