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

No classname will be generated from objects that have the "push" member set to true #17

Closed
micnic opened this issue Jan 29, 2020 · 3 comments

Comments

@micnic
Copy link

micnic commented Jan 29, 2020

In the following case all the classnames defined in the object are missing from the result:

clsx('stack', { pop: true, push: true }); // => 'stack'

This is caused by the if (!!mix.push) { check for Array, this makes clsx so fast, but I think it should be replaced with Array.isArray() or at least add this case to readme to warn people that get into this issue.

@lukeed
Copy link
Owner

lukeed commented Jan 30, 2020

Hey, thanks!

Yeah, I forgot to mention this. It actually wasn't for speed (Array.isArray is really fast) but it was mostly for 100% browser support, including all old versions of IE. Most people won't care about this, but not 100% of people have that luxury (even today).

A silly side effect was that !!mix.push was 6 fewer bytes 😆

Let me make the rounds and talk to some folks and see if they care about this change in support – the "floor" would still only be IE9.. still pretty far back.

I'd then release a minor with Array.isArray support and a note for those who care about anything older.

@micnic
Copy link
Author

micnic commented Jan 30, 2020

As I know Array.isArray() is supported by IE9

sources:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray#Browser_compatibility
https://kangax.github.io/compat-table/es5/ (check "Show obsolete platforms")

@lukeed
Copy link
Owner

lukeed commented Jan 30, 2020

Yup, that's what I meant by "the floor would becomes IE9"

Still very good 👍 Especially when paired with a note for the ~6 people (maybe) who actually benefitted from the IE5-8 support 😜

I'll get to this this weekend – I started talking to some others & this does not appear to affect them.

@lukeed lukeed closed this as completed in 0d6023a Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants