fix corner cases & public api update (#192)

* fix corner cases

fix corner cases

* update renderComponent to match ember rfc

Author: lifeart

README.md

@@ -208,9 +208,12 @@ import App from "./App.gts";
 const Instance = renderComponent(
   App, {
     // application arguments
-    name: 'My App'
-  },
-  document.getElementById("app"),
+    args: {
+      name: 'My App'
+    },
+    // render target (append to)
+    element: document.getElementById("app"),
+  }
 );
 ```
 

src/tests/integration/multiroot-test.gts

@@ -29,10 +29,16 @@ module('Integration | multiroot', function () {
     assert.dom(r2).exists('app two node exists');
     assert.ok(r3, 'app three node exists');
 
-    const appOneInstance = renderComponent(AppOne, {}, r1);
-    const appTwoInstance = renderComponent(AppOne, {}, r2);
+    const appOneInstance = renderComponent(AppOne, {
+      element: r1,
+    });
+    const appTwoInstance = renderComponent(AppOne, {
+      element: r2,
+    });
 
-    const appThreeInstance = renderComponent(AppOne, {}, r3);
+    const appThreeInstance = renderComponent(AppOne, {
+      element: r3,
+    });
 
     function qButton(r: Element) {
       return r.querySelector('[data-test-button]') as HTMLButtonElement;

src/tests/utils.ts

@@ -116,11 +116,12 @@ export async function render(component: ComponentReturnType) {
   let renderResult = renderComponent(
     createTestComponent(component, owner),
     {
-      [$context]: owner,
-    },
-    targetElement,
-    owner,
-    false,
+      args:  {
+        [$context]: owner,
+      },
+      element: targetElement,
+      owner,
+    }
   );
   await rerender();
   return renderResult;

src/utils/benchmark/benchmark.ts

@@ -19,12 +19,17 @@ export function createBenchmark(doc: Document) {
             console.error('Rehydration failed, fallback to normal render', e);
             const fragment = doc.createDocumentFragment();
             cleanupFastContext();
-            renderComponent(Application, {}, fragment, new Root(doc));
+            renderComponent(Application, {
+              element: fragment,
+              owner: new Root(doc),
+            });
             root.innerHTML = '';
             root.appendChild(fragment);
           }
         } else {
-          renderComponent(Application, {}, root);
+          renderComponent(Application, {
+            element: root,
+          });
         }
       });
 

src/utils/component.ts

@@ -1,7 +1,6 @@
 import {
   destroy,
   registerDestructor,
-  Destructors,
   destroySync,
 } from '@/utils/glimmer/destroyable';
 import type {
@@ -36,7 +35,8 @@ import {
   RENDERING_CONTEXT,
   cleanupFastContext,
 } from './context';
-import { cellToText, createRoot, MergedCell } from '.';
+import { createRoot, MergedCell } from '.';
+import { opcodeFor } from './vm';
 
 export type ComponentRenderTarget =
   | Element
@@ -91,13 +91,14 @@ export function renderElement(
       // el.ctx![RENDERED_NODES_PROPERTY].reverse();
     } else if (isFn(el)) {
       // @ts-expect-error
-      renderElement(api, ctx, target, resolveRenderable(el), placeholder);
+      renderElement(api, ctx, target, resolveRenderable(el), placeholder, skipRegistration);
     } else if (isTagLike(el)) {
-      const destructors: Destructors = [];
-      const node = cellToText(api, el, destructors);
+      const node = api.text('');
       ctx[RENDERED_NODES_PROPERTY].push(node);
       api.insert(target, node, placeholder);
-      registerDestructor(ctx, ...destructors);
+      registerDestructor(ctx, opcodeFor(el, (value) => {
+        api.textContent(node, String(value ?? ''));
+      }));
     } else {
       throw new Error(`Unknown element type ${el}`);
     }
@@ -110,11 +111,16 @@ export function renderElement(
 
 export function renderComponent(
   component: typeof Component<any>,
-  componentArgs: Record<string, unknown>,
-  target: ComponentRenderTarget,
-  appRoot: Root | Component<any> = createRoot(),
-  skipRoot?: boolean,
+  params: {
+    owner?: Root;
+    args?: Record<string, unknown>;
+    element?: ComponentRenderTarget;
+  } = {},
 ): ComponentReturnType {
+  const appRoot = params.owner ?? createRoot();
+  const target = params.element ?? document.body;
+  const componentArgs = params.args ?? {};
+
   if (import.meta.env.DEV) {
     if (target === undefined) {
       throw new Error(`Trying to render undefined`);
@@ -123,15 +129,13 @@ export function renderComponent(
   cleanupFastContext();
   const targetElement = targetFor(target);
 
-  if (!skipRoot) {
-    if (!initDOM(appRoot)) {
-      // setting default dom api
-      provideContext(
-        appRoot,
-        RENDERING_CONTEXT,
-        new HTMLBrowserDOMApi((appRoot as Root).document),
-      );
-    }
+  if (!initDOM(appRoot)) {
+    // setting default dom api
+    provideContext(
+      appRoot,
+      RENDERING_CONTEXT,
+      new HTMLBrowserDOMApi((appRoot as Root).document),
+    );
   }
 
   const args = {

src/utils/control-flow/list.ts

@@ -407,8 +407,8 @@ export class SyncListComponent<
         destroyElementSync(value, true);
       }
       parent.innerHTML = '';
-      parent.append(topMarker);
-      parent.append(bottomMarker);
+      this.api.insert(parent, topMarker);
+      this.api.insert(parent, bottomMarker);
       keyMap.clear();
       indexMap.clear();
       return true;
@@ -504,8 +504,8 @@ export class AsyncListComponent<
       await Promise.all(promises);
       promises.length = 0;
       parent.innerHTML = '';
-      parent.append(topMarker);
-      parent.append(bottomMarker);
+      this.api.insert(parent, topMarker);
+      this.api.insert(parent, bottomMarker);
       keyMap.clear();
       indexMap.clear();
       return true;

src/utils/dom-api.ts

@@ -17,12 +17,6 @@ export abstract class DOMApi {
   abstract textContent(node: Node, text: string): void;
   abstract fragment(): DocumentFragment;
   abstract element(tagName: string): Node;
-  abstract append(
-    parent: HTMLElement | Node,
-    child: HTMLElement | Node,
-    // @ts-ignore
-    targetIndex: number = 0,
-  ): void;
   abstract insert(
     parent: HTMLElement | Node,
     child: HTMLElement | Node,
@@ -84,14 +78,6 @@ export class HTMLBrowserDOMApi implements DOMApi {
   element(tagName = ''): HTMLElement {
     return this.doc.createElement(tagName);
   }
-  append(
-    parent: HTMLElement | Node,
-    child: HTMLElement | Node,
-    // @ts-ignore
-    targetIndex: number = 0,
-  ) {
-    this.insert(parent, child, null);
-  }
   insert(
     parent: HTMLElement | Node,
     child: HTMLElement | Node,

src/utils/dom.ts

@@ -613,35 +613,20 @@ function _DOM(
       if (hasShadowMode) {
         const tpl = api.element('template');
         api.attr(tpl, 'shadowrootmode', 'open');
-        element.appendChild(tpl);
+        api.insert(element, tpl);
         // @ts-expect-error children type mismatch
         renderElement(api, ctx, tpl, children);
-        // children.forEach((child, index) => {
-        //   addChild(api, tpl, child, destructors, index);
-        // });
       } else {
         // @ts-expect-error children type mismatch
         renderElement(api, ctx, appendRef!, children);
-
-        // children.forEach((child, index) => {
-        //   addChild(api, appendRef!, child, destructors, index);
-        // });
       }
     } else {
       // @ts-expect-error children type mismatch
       renderElement(api, ctx, appendRef!, children);
-
-      // children.forEach((child, index) => {
-      //   addChild(api, appendRef!, child, destructors, index);
-      // });
     }
   } else {
     // @ts-expect-error children type mismatch
     renderElement(api, ctx, element, children);
-
-    // for (let i = 0; i < children.length; i++) {
-    //   addChild(api, element, children[i], destructors, i);
-    // }
   }
 
   if (destructors.length) {
@@ -1067,19 +1052,6 @@ function slot(name: string, params: () => unknown[], $slot: Slots, ctx: any) {
   }
   return createSlot($slot[name], params, name, ctx);
 }
-export function cellToText(
-  api: DOMApi,
-  cell: Cell | MergedCell,
-  destructors: Destructors,
-) {
-  const textNode = api.text('');
-  destructors.push(
-    opcodeFor(cell, (value) => {
-      api.textContent(textNode, String(value ?? ''));
-    }),
-  );
-  return textNode;
-}
 
 function getRenderTargets(api: DOMApi, debugName: string) {
   const ifPlaceholder = IS_DEV_MODE ? api.comment(debugName) : api.comment('');

src/utils/math-api.ts

@@ -30,9 +30,6 @@ export class MathMLBrowserDOMApi implements DOMApi {
   prop(element: SVGElement, name: string, value: string) {
     element.setAttribute(name, value);
   }
-  append(parent: SVGElement, child: SVGElement) {
-    parent.appendChild(child);
-  }
   insert(parent: SVGElement, child: SVGElement) {
     parent.insertBefore(child, null);
   }

src/utils/ssr/rehydration-dom-api.ts

@@ -156,42 +156,6 @@ export class HTMLRehydrationBrowserDOMApi implements DOMApi {
     }
     return this.doc.createElement(tagName);
   }
-  append(
-    parent: HTMLElement | Node,
-    child: HTMLElement | Node,
-    targetIndex: number = 0,
-  ) {
-    if (isRehydrationScheduled()) {
-      if (import.meta.env.DEV) {
-        if (!parent) {
-          debugger;
-        }
-      }
-      // in this case likely child is a text node, and we don't need to append it, we need to prepend it
-      const childNodes = Array.from(parent.childNodes);
-      const maybeIndex = childNodes.indexOf(child as any);
-      if (maybeIndex !== -1 && maybeIndex === targetIndex) {
-        return;
-      }
-      if (childNodes.length === 0) {
-        this.insert(parent, child, null);
-        return;
-      } else if (targetIndex === 0) {
-        this.insert(parent, child, parent.firstChild);
-        return;
-      } else if (targetIndex >= childNodes.length) {
-        this.insert(parent, child, null);
-        return;
-      }
-      if (!childNodes[targetIndex]) {
-        throw new Error(`Rehydration filed. Unable to find target node.`);
-      }
-      this.insert(parent, child, childNodes[targetIndex]!);
-      return;
-    } else {
-      this.insert(parent, child, null);
-    }
-  }
   insert(
     parent: HTMLElement | Node,
     child: HTMLElement | Node,

src/utils/ssr/rehydration.ts

@@ -141,7 +141,9 @@ export function withRehydration(
       }
     });
 
-    renderComponent(componentCreationCallback, args, targetNode, root, true);
+    renderComponent(componentCreationCallback, {
+      args, element: targetNode, owner: root,
+    });
     if (withRehydrationStack.length > 0) {
       const lastNodes = Array.from(withRehydrationStack);
       console.warn('withRehydrationStack is not empty', lastNodes);

src/utils/ssr/ssr.ts

@@ -20,9 +20,11 @@ export async function renderInBrowser(
   // @todo - add destructor
   renderComponent(
     componentRenderFn,
-    args,
-    rootNode,
-    root,
+    {
+      args,
+      element: rootNode,
+      owner: root,
+    },
   );
   const html = rootNode.innerHTML;
   rootNode.remove();
@@ -44,7 +46,11 @@ export async function render(
   doc.body.appendChild(rootNode);
 
   resetNodeCounter();
-  renderComponent(component, args, rootNode as unknown as HTMLElement, root);
+  renderComponent(component, {
+    args, 
+    element: rootNode as unknown as HTMLElement, 
+    owner: root,
+  });
   resetNodeCounter();
 
   const s = new XMLSerializer();

src/utils/svg-api.ts

@@ -42,9 +42,6 @@ export class SVGBrowserDOMApi implements DOMApi {
       element.setAttribute(name, value);
     }
   }
-  append(parent: SVGElement, child: SVGElement) {
-    parent.appendChild(child);
-  }
   insert(parent: SVGElement, child: SVGElement) {
     parent.insertBefore(child, null);
   }