Fix renderComponent so that it tracks DOM position during re-renders

Author: NullVoxPopuli

packages/@ember/-internals/glimmer/lib/renderer.ts

@@ -553,6 +553,16 @@ export interface RenderResult {
   destroy(): void;
 }
 
+interface RenderCacheEntry {
+  result: RenderResult;
+  /**
+   * The GlimmerRenderResult from the last render. Used to get positional
+   * information (firstNode) when a re-render replaces the content, so
+   * that the new content is placed at the same DOM position.
+   */
+  glimmerResult: GlimmerRenderResult | undefined;
+}
+
 function intoTarget(into: IntoTarget): Cursor {
   if ('element' in into) {
     return into;
@@ -655,7 +665,7 @@ export function renderComponent(
    * NOTE: destruction is async
    */
   let existing = RENDER_CACHE.get(into);
-  existing?.destroy();
+  existing?.result.destroy();
   /**
    * We can only replace the inner HTML the first time.
    * Because destruction is async, it won't be safe to
@@ -665,26 +675,46 @@ export function renderComponent(
     into.innerHTML = '';
   }
 
-  let innerResult = renderer.render(component, { into, args }).result;
+  /**
+   * If there's an existing render result with valid bounds, use its
+   * firstNode as the nextSibling so that new content is inserted at
+   * the same DOM position. This ensures stable ordering when multiple
+   * renderComponent calls target the same element and one is re-invoked
+   * (e.g., due to tracked dependency changes).
+   *
+   * The old content's DOM nodes are still present (destruction is async),
+   * so firstNode() is a valid position reference. The new content is placed
+   * BEFORE the old content. When the old content is eventually destroyed
+   * (async clear of bounds), the new content remains in the correct position.
+   */
+  let renderTarget: IntoTarget = into;
+  if (existing?.glimmerResult) {
+    let parentElement =
+      into instanceof Element ? (into as unknown as SimpleElement) : (into as Cursor).element;
+    let firstNode = existing.glimmerResult.firstNode();
+    renderTarget = { element: parentElement, nextSibling: firstNode };
+  }
+
+  let innerResult = renderer.render(component, { into: renderTarget, args }).result;
 
   if (innerResult) {
     associateDestroyableChild(owner, innerResult);
   }
 
-  let result = {
+  let result: RenderResult = {
     destroy() {
       if (innerResult) {
         destroy(innerResult);
       }
     },
   };
 
-  RENDER_CACHE.set(into, result);
+  RENDER_CACHE.set(into, { result, glimmerResult: innerResult });
 
   return result;
 }
 
-const RENDER_CACHE = new WeakMap<IntoTarget, RenderResult>();
+const RENDER_CACHE = new WeakMap<IntoTarget, RenderCacheEntry>();
 const RENDERER_CACHE = new WeakMap<object, BaseRenderer>();
 
 export class BaseRenderer {

packages/@ember/-internals/glimmer/tests/integration/components/render-component-test.ts

@@ -638,7 +638,7 @@ moduleFor(
       assertHTML('');
     }
 
-    '@test multiple calls to render in to the same element'() {
+    '@test multiple calls to render in to the same element appear as siblings'() {
       let aHelper = (str: string) => str.toUpperCase();
       let Child = defComponent(`Hi: {{aHelper "there"}}`, { scope: { aHelper } });
       let get = (id: string) => this.element.querySelector(id);
@@ -650,7 +650,12 @@ moduleFor(
       }
       let A = defComponent('a:<Child />', { scope: { Child } });
       let Root = defComponent(
-        [`<div id="a"></div><br>`, `{{render A 'a' owner}}`, `{{render A 'a'}}`].join('\n'),
+        [
+          `<div id="a"></div><br>`,
+          // both of these go in the div
+          `{{render A 'a' owner}}`,
+          `{{render A 'a'}}`,
+        ].join('\n'),
         { scope: { render, A, owner: this.owner } }
       );
 
@@ -662,6 +667,83 @@ moduleFor(
       assertHTML('');
     }
 
+    /**
+     * NOTE: subsequent renders to the same element are prepended to the element's children
+     */
+    '@test multiple calls to render in to the same element appear as siblings and can be updated'() {
+      let aHelper = (str: string) => str.toUpperCase();
+      let dataA = trackedObject({ count: 1 });
+      let dataB = trackedObject({ count: -1 });
+      let Child = defComponent(`Hi: {{aHelper "there"}}`, { scope: { aHelper } });
+
+      let get = (id: string) => this.element.querySelector(id);
+      function render(Comp: GlimmerishComponent, id: string, owner: Owner) {
+        renderComponent(Comp, {
+          into: get(`#${id}`)!,
+          owner,
+        });
+      }
+      let A = defComponent('\n<output>a:<Child />:{{data.count}}</output>\n', {
+        scope: { Child, data: dataA },
+      });
+      let B = defComponent('\n<output>b:<Child />:{{data.count}}</output>', {
+        scope: { Child, data: dataB },
+      });
+
+      let Root = defComponent(
+        [
+          `<div id="a"></div><br>`,
+          // both of these go in the div
+          `{{render A 'a' owner}}`,
+          `{{render B 'a'}}`,
+        ].join('\n'),
+        { scope: { render, A, B, owner: this.owner } }
+      );
+
+      this.renderComponent(Root, {
+        expect: [
+          `<div id="a">`,
+          `<output>b:Hi: THERE:-1</output>`,
+          `<output>a:Hi: THERE:1</output>`,
+          `</div><br>`,
+          '',
+          '',
+        ].join('\n'),
+      });
+
+      this.assertChange({
+        change: () => dataA.count++,
+        expect: [
+          `<div id="a">`,
+          `<output>b:Hi: THERE:-1</output>`,
+          `<output>a:Hi: THERE:2</output>`,
+          `</div><br>`,
+          '',
+          '',
+        ].join('\n'),
+      });
+
+      /**
+       * ERROR in conflict with the NOTE on this test.
+       * the elementns flip locations... which is kinda bonkers
+       */
+      this.assertChange({
+        change: () => dataB.count--,
+        expect: [
+          `<div id="a">`,
+          `<output>b:Hi: THERE:-2</output>`,
+          `<output>a:Hi: THERE:2</output>`,
+          `</div><br>`,
+          '',
+          '',
+        ].join('\n'),
+      });
+
+      run(() => destroy(this));
+
+      assertHTML('');
+    }
+
     async '@test async rendering multiple times to adjacent elements'() {
       let Child = defComponent(`Hi`, { scope: {} });
       let get = (id: string) => this.element.querySelector(id);