cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.07k stars 399 forks source link

[react-jss] Empty second rule is generated when using function value in useStyles hook #1264

Open haddigan opened 4 years ago

haddigan commented 4 years ago

Expected behavior: useStyles hook shouldn't create a class name with no rules.

Describe the bug: I'm creating a useStyles hook where the rules are all derived from prop values:

const useStyles = createUseStyles({
  styles: {
    margin: props => props.margin,
    padding: props => props.padding,
    border: props => props.border
  }
});

When the hook is called, it returns two class names, one with the rules derived from the prop values and another class name with no rules at all.

668 seems to describe something similar but is marked as resolved... Could this have been fixed in the HOC but not the hook?

Codesandbox link: https://codesandbox.io/s/upbeat-gould-n2ejd

Versions (please complete the following information):

HenriBeck commented 4 years ago

Why exactly is this a problem? This is known and kind of expected as we split the styles into static ones and dynamic ones. If you only have an empty object, it will create an empty class with no rules.

I don’t really this is worth fixing as it should have no to little impact on the performance

kof commented 4 years ago

We could see this as a potential improvement, but I would give this a low prio.

HardeepAsrani commented 4 years ago

+1 for this. Would love to se this being implemented.

danhickman commented 2 years ago

+1 for this. Sifting through empty classes in the browser dev tools makes debugging more difficult because of the useless clutter.

And on a related note, @HenriBeck, why split the dynamic and static styles in the first place? I'd much rather just have one class with all the styles. Is there some option to force this?

kof commented 2 years ago

Dynamic styles are called dynamic because they are either different for different instances of components or they change over time, either way dynamic styles can't be reused across instances, so if you make one rule, you would have much more duplication

danhickman commented 2 years ago

Dynamic styles are called dynamic because they are either different for different instances of components or they change over time, either way dynamic styles can't be reused across instances, so if you make one rule, you would have much more duplication

OK, that makes sense. I would still really like a way to suppress the empty classes. I have split out most of the static and dynamic styles into separate sheets to make things more modular and reusable so in most cases I only have static or dynamic styles in a single sheet so almost always creating empty rules.

kof commented 2 years ago

We should not have the empty rule at all, there was some difficulty implementing it afaik ... you are welcome to try

mahhmoud-enzyme commented 2 years ago

Any fix coming soon??

danhickman commented 1 year ago

@kof If we can't get rid of the empty rules, is there any way to detect that they are empty so we can at least not apply the CSS class to the elements? I've got many cases where there are no static styles and the dynamic styles end up empty as well because there are no dynamic styles applied. It would be super helpful if we could detect this case and not apply the classes to elements.