solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
31.64k stars 887 forks source link

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

Open nyanrus opened 2 days ago

nyanrus commented 2 days ago

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

image

Platform

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 commented 2 days ago

Universal render method is different client render method.

client render method will clean nodeselement.textContent = "";

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

image

nyanrus commented 2 days ago

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 commented 2 days ago

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