search

break-stuff / cem-tools

Tooling for generating features based on the Custom Elements Manifest
MIT License
106 stars 13 forks source link

Missing class, and className is not correct for intrinsic elements #108

Closed herrKlein closed 5 months ago

herrKlein commented 7 months ago

In the last version of the custom-element-jsx-integration

class is replaced with className https://github.com/break-stuff/cem-tools/commit/7d3e1ff912b72002164f34f993e6aa47de230d6d#diff-c1f723d4c479bb572a4f7f3fcfdaf4dcb716edec497cf4e6a80bd32666cc8417L29

I don't think this is correct. className should not be used for the intrinsic elements, which React just add the attributes given. If I change to className, it compiles, but now className is literally in the html markup, which does nothing.

So I think className should be class again?

break-stuff commented 7 months ago

What framework are you using?

herrKlein commented 7 months ago

React, I just reverted to version 1.1.0 and now it is seemingly working correctly. class on intrinsic elements renders as class in browser and styling is correct.

break-stuff commented 7 months ago

React will not behave correctly with just these. I recommend using the wrappers until react 19 is released with custom element support.

https://www.npmjs.com/package/custom-element-react-wrappers

herrKlein commented 7 months ago

It works with the older version.. Relentlessly waiting for React19, I'm already using the react wrappers for the elements which throw events. The rest should be intrinsic, react just have to put them in the dom. This works for now.

break-stuff commented 6 months ago

React doesn't throw an error when you use class instead of className? I thought those were reserved words in JavaScript. That's the same reason they use htmlFor instead of for.

break-stuff commented 6 months ago

Here are the react guidelines, but I believe this is part of the JSX templating language. https://react.dev/reference/react-dom/components/common#applying-css-styles

I suppose we could add it and if it screams at devs, they can use className. 🤷‍♂️

herrKlein commented 6 months ago

for intrinsic elements react keept the attribute and expects it to be on the intrinsic element. For react components, the property className is used. I am on the verge of testing React 19. I don't know how it knows about the types of the webcomponents. We will see..

Adding it is a good idea.

break-stuff commented 6 months ago

Deployed in release 1.4.0

break-stuff commented 6 months ago

Is this working for you now?

herrKlein commented 6 months ago

Oh yes, its working.. Thanks a lot

break-stuff commented 6 months ago

Thank you for letting me know!