Universal Renderer's cleanup method doesn't remove existing node

[nyanrus]

Describe the bug

Universal Renderer's cleanup method (the return value of render function) doesn't remove the existing node, which is inconvenient for using HMR.

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-ycgqgq?file=src%2Findex.tsx

Steps to Reproduce the Bug or Issue

1. See the rendered result.

Expected behavior

I ran the cleanup method and I expected the node to be removed.

Screenshots or Videos

Platform

• OS: Windows 11
• Browser: Firefox
• Version: 115

Additional context

I am using solid-js for Firefox XUL, non-standard HTML, for developing a custom browser.
While I making HMR using Vite, I encountered the problem.
Thank you for the good project!

[this-spring]

Universal render method is different client render method.

client render method will clean nodes element.textContent = "";

I am not sure whether universal render need add this logic.

[nyanrus]

Thank you for the reply.
I'll implement with overriding the render method.
Thanks!
By the way, I think it will be good if there are some docs or codes that guides/allows to implement custom cleanup method.

[this-spring]

emmm, I think cleanup is inner logic. It should not implement custom by users. Override render method is good way.

[ryansolid]

Yeah I added the cleanup to web after the fact possibly.. There is no universal API currently to accomplish this I think? I'm open to suggestions on what sort of API you'd want to see to support this.

[nyanrus]

Currently, I am overriding the render function for support vite's HMR.
In Firefox codebase, I have to insert element in existing node a lot, and the dispose function seems only on render.
so I made options(third param) in render with marker and hotCtx (solid-refresh doing not expected work so I am not using).

const _render = (
  code: () => JSX.Element,
  node: JSX.Element,
  options?: { marker?: Element; hotCtx?: ViteHotContext },
) => {
  let disposer: () => void = () => {};
  createRoot((dispose) => {
    const elem = insert(node, code(), options ? options.marker : undefined);
    disposer = () => {
      dispose();
      if (elem instanceof Element) {
        elem.remove();
      } else if (
        Array.isArray(elem) &&
        elem.every((e) => e instanceof Element)
      ) {
        elem.forEach((e) => e.remove());
      }
    };
    if (options?.hotCtx) {
      hotCtxMap.set(options.hotCtx, [
        ...(hotCtxMap.get(options.hotCtx) ?? []),
        disposer,
      ]);
      console.log("register disposer to hotCtx");
      options.hotCtx.dispose(() => {
        hotCtxMap.get(options.hotCtx!)?.forEach((v) => v());
        hotCtxMap.delete(options.hotCtx!);
      });
    }
  });
  return disposer;
};

https://github.com/nyanrus/noraneko/blob/b8881a58a3a49f4fbca234a8fd9b36c61729cc80/packages/solid-xul/index.ts#L118

If the marker are as default, and custom disposer as

~~~
  mergeProps,
} = createRenderer<JSX.Element>({
  /**
   * @param elem output of insert func
   * @param options render func's third param
   * may the render func's param can be in this func's param.
   */
  cleanupRender: (elem, options) => {
    if (elem instanceof Element) {
      elem.remove();
    } else if (
      Array.isArray(elem) &&
      elem.every((e) => e instanceof Element)
    ) {
      elem.forEach((e) => e.remove());
    }
  },
  createElement: (tag: string): Element => {
    if (tag.startsWith("xul:")) {
      return document.createXULElement(tag.replace("xul:", ""));
    }
    return document.createElement(tag);
  },
~~~

original from https://github.com/nyanrus/noraneko/blob/b8881a58a3a49f4fbca234a8fd9b36c61729cc80/packages/solid-xul/index.ts#L35

, then I could make less the render func's overriding as

const _render = (
  code: () => JSX.Element,
  node: JSX.Element,
  options?: { marker?: Element; hotCtx?: ViteHotContext },
) => {
  const disposer = render(code,node,options);
  if (options?.hotCtx) {
    hotCtxMap.set(options.hotCtx, [
      ...(hotCtxMap.get(options.hotCtx) ?? []),
      disposer,
    ]);
    console.log("register disposer to hotCtx");
    options.hotCtx.dispose(() => {
      hotCtxMap.get(options.hotCtx!)?.forEach((v) => v());
      hotCtxMap.delete(options.hotCtx!);
    });
  }
  return disposer;
};

and it seems comfortable.