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

Add support for objects with toString methods #52

Open
sandgraham opened this issue Sep 21, 2022 · 1 comment
Open

Add support for objects with toString methods #52

sandgraham opened this issue Sep 21, 2022 · 1 comment

Comments

@sandgraham
Copy link

sandgraham commented Sep 21, 2022

Hi there, I'm using a css-in-js library (stitches) which returns class names as objects with custom toString methods. This requires calling toString before passing the "class names" to clsx- otherwise clsx assumes the object is a config/mapping object.

// Current usage
const pink = css({ color: "pink" });
clsx(pink().toString())

I currently have a patch package which adds an additional check to the toVal function, checking to see if a mix object has it's own toString method and calls/returns that if so.

// General idea
if (mix.hasOwnProperty("toString")) {
  str += mix.toString();
}

I noticed that classnames has a condition for this (original PR). React also calls toString if you pass an object to className. I'm wondering if it might be useful to include this behavior in clsx? I can contribute a PR if so.

@lukeed
Copy link
Owner

lukeed commented Sep 21, 2022

it might be worth looking into a codemod to automatically call the stitches' toString() method. For example, it could definitely be worked into the clsx babel plugin.

Strings are always the best (as in most performant) path for clsx to operate, so our current usage is definitely the preferred approach. Adding a toString check to clsx itself would be a significant performance hit (and byte size, percentage-wise). And your hasOwnProperty example snippet wouldn't suffice as it wouldnt cover class instances that have a toString method on its prototype.

It'd have to be something like:

// ...
if (typeof mix.toString === 'function' && mix.toString !== Object.prototype.toString) {
  str && (str += ' ');
  str += mix.toString();
} else for (k in mix) {
  if (mix[k]) {
    str && (str += ' ');
    str += k;
  }
}

TBD - I'll run some benches to actually measure the impact

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