search

ryansolid / dom-expressions

A Fine-Grained Runtime for Performant DOM Rendering
MIT License
858 stars 125 forks source link

Type definitions of `classList` are wrong #279

Closed danieltroger closed 9 months ago

danieltroger commented 10 months ago

It doesn't return void as defined here https://github.com/ryansolid/dom-expressions/blob/31a05fd3f74aed26e54586c60dee4663b18acd65/packages/lit-dom-expressions/src/index.ts#L12

At least not from what the code looks like

Screenshot 2023-10-20 at 20 30 29
lxsmnsyc commented 10 months ago

I don't think it matters much since the API is purely internal and only used by the compiler.

danieltroger commented 10 months ago

Why is it purely internal? It's exported by solid-js/web.

I used it to set classes manually and it was extremely handy. Basically I have a library that returns an element and I have a signal that contains classes that I want to add to that element after its creation. I don't want to modify the library/bloat the interface, so I just used classList. But it got red because it gets typechecked when it's in my actual code.

I made a PR for you so it's easier to fix :)

https://github.com/ryansolid/dom-expressions/pull/280

lxsmnsyc commented 10 months ago

Why is it purely internal? It's exported by solid-js/web.

Re-exported, as the compiler would refer to the module as to where it should import, since dom-expressions has no true runtime (The runtime is cloned on Solid's side). This is true to the rest of dom-expressions API

danieltroger commented 10 months ago

Sorry I don't quite understand, is there another repo with this for specifically solid where I should open a pull request? How is this runtime cloned on Solid's side?

lxsmnsyc commented 10 months ago

@danieltroger it's okay to make a PR here. dom-expressions code doesn't exist in Solid's repo and exists only during bundling, but yeah the process is that the contents of dom-expressions is cloned into Solid's.

edit: I guess this is no longer true, but yeah stuff is still re-exported. https://github.com/solidjs/solid/blob/main/packages/solid/web/src/client.ts

ryansolid commented 10 months ago

I needs to be re-exported the way dom-expressions works so the compiler can find it (alias it whatever). Sort of awkward tension because yeah I don't really want to consider this stuff external API.

MrHBS commented 9 months ago

I think this is fixed.

danieltroger commented 9 months ago

True sorry, I wrote "see" instead of "fixes" in https://github.com/ryansolid/dom-expressions/pull/280