Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When no classes are selected, classNames should return null #313

Open
Lovor01 opened this issue Jul 15, 2023 · 5 comments
Open

When no classes are selected, classNames should return null #313

Lovor01 opened this issue Jul 15, 2023 · 5 comments

Comments

@Lovor01
Copy link

Lovor01 commented Jul 15, 2023

ClassNames with no classes selected return empty string (''). For using it in React with jsx, it would be much better to return null.
Example:

<p className={classNames({ first: false, second: false)>
Hello World!
</p>

This results in <p class>Hello World!</p>, while <p>Hello World!</p> would be correct. The second would happen if classNames returned null in such case.

BoberGame added a commit to BoberGame/classnames that referenced this issue Sep 2, 2023
@BoberGame BoberGame mentioned this issue Sep 2, 2023
@jonkoops jonkoops removed the question label Dec 14, 2023
@jonkoops
Copy link
Collaborator

This is a great idea, however considering this changes the public API, this must be considered a breaking change.

@jonkoops
Copy link
Collaborator

I am adding this to the milestone for a potential version 3.0, which will allow us to introduce some breaking changes.

@jonkoops jonkoops added this to the 3.0.0 milestone Dec 27, 2023
@jonkoops
Copy link
Collaborator

@dcousens @JedWatson could I get your take on this enhancement? I think it might be a valuable addition, especially for React users, but the pain of having a return value that is possibly undefined might be problematic for others. I am leaning towards introducing this in v3, as we can always revert it as a non-breaking change in case it causes too much frustration for users.

@dcousens
Copy link
Collaborator

dcousens commented Dec 28, 2023

I'm not against this, it is a reasonable breaking change - but many usage patterns have assumed that concatenation is safe and this change might break that workflow quite substantially.

The fix for users should be as simple as ?? '', but, that's the argument against.

@jonkoops
Copy link
Collaborator

The fix for users should be as simple as ?? '', but, that's the argument against.

Yeah, exactly, this could be an annoying fix to apply. I think we should still go for it, and base our decision on restoring it on user feedback. Perhaps we should roll a couple of beta versions so folks can try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants