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 arrays as argument #124

Closed
dcastil opened this issue Aug 10, 2022 · 3 comments · Fixed by #127
Closed

Add support for arrays as argument #124

dcastil opened this issue Aug 10, 2022 · 3 comments · Fixed by #127
Labels
context-v1 Related to tailwind-merge v1 feature request

Comments

@dcastil
Copy link
Owner

dcastil commented Aug 10, 2022

From discussion:

The more I'm thinking about this, the more I think I should include clsx in tailwind-merge. My reasons:

  1. clsx (or its pendant classnames) is probably already included in most projects using tailwind-merge. I have copied parts of clsx and inlined it in tailwind-merge to join strings, so adding it would have almost no impact on bundle size nor runtime performance.

  2. I find the pattern of nested conditions with arrays in clsx really nice and often miss it in tailwind-merge. Basically this:

    clsx('…', someBool && ['…', anotherBool && '…'])
  3. Prepending twMerge with clsx myself is not so nice DX-wise, even when using tailwind-merge with a custom config. It can even lead to anti-patterns.

    // This feels a little weird
    const twMergeInternal = extendTailwindMerge()
    export const twMerge = (...args) => twMergeInternal(clsx(args))
    
    // I really hope no one is doing this!
    export const twMerge = (...args) => extendTailwindMerge()(clsx(dontDoThis!))

I think I'll give it a shot. And if it turns out not to be a good idea, I think tailwind-merge should go into the opposite direction in a v2 update and only accept a single string as argument. This way the user could choose their own string-joining lib and tailwind-merge would leave the weird middle ground it is on right now.

Originally posted by @dcastil in #121 (reply in thread)

@dcastil
Copy link
Owner Author

dcastil commented Aug 10, 2022

I definitely want to support arrays and nested arrays. I don't like objects but would probably be too opinionated not to support them.

@dcastil
Copy link
Owner Author

dcastil commented Aug 12, 2022

There is additional new context in #121, should check that out before working on this.

@dcastil dcastil changed the title Add support for clsx-style collection argument types Add support for arrays as argument Aug 13, 2022
@github-actions
Copy link

This was addressed in release v1.6.0.

@dcastil dcastil added the context-v1 Related to tailwind-merge v1 label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context-v1 Related to tailwind-merge v1 feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant