Universal renderer: `lazy()` automatically inserts an empty text node, which may or may not be supported by the underlying renderer

[Alloyed]

Describe the bug

This is a regression caused by https://github.com/solidjs/solid/commit/c8fe58e9d259e463def62535f6d23454d4f30cee as far as I can tell, specifically packages/solid/src/render/component.ts line 381

return createMemo(() =>
      (Comp = comp())
        ? untrack(() => {
            if ("_SOLID_DEV_") Object.assign(Comp!, { [$DEVCOMP]: true });
            if (!ctx || sharedConfig.done) return Comp!(props);
            const c = sharedConfig.context;
            setHydrateContext(ctx);
            const r = Comp!(props);
            setHydrateContext(c);
            return r;
          })
        : ""
    ) as unknown as JSX.Element;

upon upgrading, our proprietary universal render started to fail in cases where the lazy() element was being used, because we never implemented a version of createTextNode() that would intelligently handle the empty string the second half of that ternary.

Your Example Website or App

n/a

Steps to Reproduce the Bug or Issue

1. create a universal renderer, and implement createTextNode like so:

    createTextNode: function (_text: string): MyNodeType {
         throw new Error("Text nodes not supported!")
    },

3. create an HTML with a lazy() component inserted.
4. Notice your error is triggered

Expected behavior

n/a

Screenshots or Videos

n/a

Platform

n/a

Additional context

So the reason we don't support bare text nodes in our renderer is that all of our text live inside a specific base element, like so:

<label text="my actual text" />

so there was never a need to implement text nodes, and it would muddy the waters of our implementation to try. We can work around this issue just by ignoring bare text entirely. that said, if the implementation of a built in component tried to insert any other kind of text, for any reason, we'd still have this problem. One approach may just be to document this behavior, with wording along the lines of: "your renderer must at least support empty string and whitespace-only text nodes. your renderer may ignore them entirely if desired."

[ryansolid]

I see. Yeah we needed a fixed marker here. Empty text node seemed the easiest because it is mostly invisible in the DOM. We do this in things like Portals as well but I suppose you don't hit that in the universal renderer as well. Documenting it might be the easiest option I think. I'm not sure how else to best handle it.

[chiefcll]

Hey @ryansolid - I'm experiencing this issue as well.

I have a simple router with some pages that are loaded via Lazy. And this:

<View id="pageContainer" ref={pageContainer} forwardFocus={0} children={props.children} /> as the parent container.

With the Lazy there is an empty textNode being added to the pageContainer. However, when the page is created and added I now have the textNode and the childPage in the pageContainer children. It's not removing / replacing the empty text node when the first page is created. This is due to the page being a single top level node: <View /> - if its multi node (more than 1 child), the clean removes all children and everything is fine. When I switch pages, it replaces the empty text node with the new page, and now my pageContainer has newPage, oldPage as its children.

I'm using the Universal Renderer and just looking for advice on how to proceed. I'm going to try not allowing empty text nodes to be added as Children, but that may affect <Show> and other things...

MORE DETAILS: The first element is a textNode (due to Lazy) and rather than replacing the second node its using the getFirstChild which is the textNode and thus leaving the previous page in the children