Bug in generic types for universal render target?

[Alloyed]

https://github.com/ryansolid/dom-expressions/blob/main/packages/dom-expressions/src/universal.d.ts

Hi, I'm trying to implement a custom renderer that does not target the dom at all (!). As far as I can tell, to do this I need to

A. replace JSX.Element with a variant that does not reference dom nodes ✅
B. call createRenderer<MyCustomNode>.

Doing this creates types with (roughly) this signature:

type JSX.Element = MyCustomNode | JSX.Element[] | null | undefined; //... etc
const render: (code: () => MyCustomNode, node: MyCustomNode) => () => void

The problem here is that the component definition () => MyCustomNode appears to be different type to JSX.Element. The (DOM) definition of JSX.Element includes all sorts of things that aren't just HTMLElement, but when implementing my renderer I only wrote custom methods in terms of my custom dom node type, and not the array/null/other things.

What I naively would've expected is something like this signature:

interface Renderer<NodeType, JsxElementType> {
    const render: (code: () => JsxElementType, node: NodeType) => () => void;
}

Is the type declaration wrong here, or should I be implementing my renderer to handle all those extra types, too?

[jpdutoit]

I agree the type definition is wrong here. It should allow any JSX.Element return. Casting it to your replacement works fine in my tests.

[jpdutoit]

I believe it should be the following:

interface Renderer<NodeType> {
    const render: (code: () => UniversalJSXElement<NodeType>, node: NodeType) => () => void;
}

type UniversalJSXElement<NodeType> =
  | NodeType
  | Array<UniversalJSXElement<NodeType>> 
  | (string & {})
  | number
  | boolean
  | null
  | undefined

[Alloyed]

ok, found another issue. It's related enough that I don't want open a second ticket for it. Existing docs for custom renderers suggest exposing control flow components like so:

// Forward Solid control flow
export { ErrorBoundary, For, Index, Match, Show, Suspense, SuspenseList, Switch } from 'solid-js';

Here's what the declaration looks like for <For/>, when you do this

import type { JSX } from "solid-js/types/jsx";
export declare function For<T extends readonly any[], U extends JSX.Element>(props: {
    each: T | undefined | null | false;
    fallback?: JSX.Element;
    children: (item: T[number], index: Accessor<number>) => U;
}): JSX.Element;

This means all our control flow primitives are using the definition of JSX provided by solid (aka the dom version) instead of our non-dom version. So likewise, I will probably work around this by replacing these declarations for the time being

EDIT: actually, it's likely that any third party libraries, even if they don't depend on the dom, will be using import("solid-js").JSX too. so maybe I need to be augmenting that type, rather than creating my own independent type.

[jpdutoit]

Yeah, this is a real problem. I work around it by basically copy-pasting all those type definitions

import { For as UntypedFor} from "solid-js"

export const For = UntypedFor as <T extends readonly any[], U extends JSX.Element>(props: {
  each: T | undefined | null | false
  fallback?: JSX.Element
  children: (item: T[number], index: Accessor<number>) => U
}) => JSX.Element

It works, but I end up constantly fighting the VSCode auto-importer to import the methods from the correct file. So, ideally the definitions should be updated to be NodeType agnostic. I think those definitions live in the main solid repo, so an issue there would make sense.

I think a wider discussion is needed here. Currently the developer experience in Typescript is not great.

[chiefcll]

Thanks for the info - we're working on adding Types to our project which is universal renderer... https://github.com/lightning-js/solid
@frank-weindel