search

swordev / suid

A port of Material-UI (MUI) built with SolidJS.
https://suid.io
MIT License
660 stars 47 forks source link

Make FormControlLabel.tsx ssr compatible #254

Closed BierDav closed 11 months ago

BierDav commented 11 months ago

This potentially makes old code a bit more inefficent when using typography directly as label, but sadly there is no way of reliably checking what the isTypography test for, because we have just access to an html string, which would be even more inefficent to parse it as just wrapping everything in a div.

And when someone wants to be as efficent they have to just set the disableTypography prop. In my opinion it's a worth tradeoff.

juanrgm commented 11 months ago

Why don't you use isServer?

  const isTypography = (v: unknown) =>
    !isServer &&
    v instanceof HTMLElement &&
    v.classList.contains(Typography.toString());
BierDav commented 11 months ago

That would make the whole pr useless, because it would lead to hydration errors

BierDav commented 11 months ago

The only way we would be able to achieve this is by using a context just for this, but I don't think that is worth the effort. The majority is not using a separate typography because there aren't a lot of use cases where it is the best way to do so. But maybe in future we will get features of solid start that allow us to access the already rendered html more easily.

juanrgm commented 11 months ago

I reverted your commit (https://github.com/swordev/suid/commit/c62830e8512ace916677ca3541f3196ec67e8f2d) and implemented a more suitable solution (here) compatible with SSR.