Consider making the Node type used throughout XmlRenderer generic

[DrRataplan]

Hey there,

I've been using xml-renderer for a bit and I noticed the Node type used in a bunch of places in the codebase is of the type globalThis.Node: the browser built-in variant. This usually works out fine.

I've been using xml-renderer in combination with another DOM implementation: slimdom. At that point, if you want to use some more specific slimdom functions, such as calling XMLSerializer#serializeToString with a node that is coming out of xml-renderer, Typescript warns me about the fact that globalThis.Node is not compatible with slimdom.Node. You'll have to typecast the xml-renderer Node (which is the browser Node type) to slimdom.Node (which is what slimdom accepts).

If xml-renderer would be generic on the Node type, I could pass slimdom.Node and have more powerful typing. Other libraries, such as FontoXPath will then also pick those typings up, making it easier for me to work with xml-renderer.

What would be your thoughts?

Regard,

Martin

[wvbe]

@DrRataplan I think you're completely right and I do plan to make this change. Thanks for logging an issue, it helps me remember!

evaluateXPathToNodes, which xml-renderer depends on, is presumably also generic?

In your opinion, which interface is more intuitive to make generic?

const nerf = new ReactRenderer<slimdom.Node>()

or

nerf.render<slimdom.Node>(createElement, myNode)

[DrRataplan]

Hey Wybe, Awesome! I was planning to make a PR when I find a moment, let's seen who can finish it first! I think the first option makes the most sense. The latter could be useful when XML renderer is processing multiple DOM implementations at once, but that seems silly. It also requires more code. For `GenericRenderer`, the amount of type arguments will get a bit out of hand though... As long as we name them properly, we can cope. And yes, `evaluateXPathTo*` is generic. It even tries to use the type of the passed contextnode, but that is a bit fiddly in practice. Surely something to improve even more now that more projects are using fontoxpath in a typescript setting. I'll see what I can do. Regards, Martin

…

On Thu, 4 Mar 2021, 20:55 Wybe, ***@***.***> wrote: @DrRataplan <https://github.com/DrRataplan> I think you're completely right and I do plan to make this change. Thanks for logging an issue, it helps me remember! evaluateXPathToNodes, which xml-renderer depends on, is presumably also generic? In your opinion, which interface is more intuitive to make generic? const nerf = new ReactRenderer<slimdom.Node>() or nerf.render<slimdom.Node>(createElement, myNode) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGKEJE4FLEAYQHAAUFNKHTTB7QTLANCNFSM4YS5ITNQ> .