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

fix(cli): deduplicate classNames in applyColorMapping #1089

Merged
merged 6 commits into from
Sep 19, 2023

Conversation

santidalmasso
Copy link
Contributor

Deduplicate class names within the applyColorMapping function. To resolve this, the method of storing class names has been updated from using arrays to sets, which inherently ensures that all class names are unique.

@vercel
Copy link

vercel bot commented Aug 1, 2023

@santidalmasso is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Aug 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2023 2:35am

}
}

return lightMode.join(" ") + " " + darkMode.join(" ").trim()
return (
Array.from(lightMode).join(" ") +

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Can be simplified to return [...lightMode, ...darkMode].join(' ').trim().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion to simplify the code. I did try implementing it that way at first, but I encountered a TypeScript error. Since TypeScript was unable to infer that lightMode and darkMode were iterable, it prevented me from using the spread syntax directly. So I opted for the Array.from method instead.

This was the error: Type 'Set<string>' can only be iterated through when using the '--downlevelIteration' flag or with a '--target' of 'es2015' or higher.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the solution you shipped with the latest commit though. Much cleaner now!

@shadcn
Copy link
Collaborator

shadcn commented Aug 1, 2023

@santidalmasso Thanks for the PR. Reviewing...

@shadcn shadcn self-assigned this Aug 1, 2023
@shadcn shadcn added bug Something isn't working area: cli labels Aug 1, 2023
Copy link
Collaborator

@shadcn shadcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THANK YOU.

@shadcn shadcn merged commit ae84578 into shadcn-ui:main Sep 19, 2023
4 of 5 checks passed
lloydrichards pushed a commit to lloydrichards/shadcn-ui that referenced this pull request Oct 3, 2023
* fix(cli): deduplicate classNames in applyColorMapping

* refactor: simplify applyColorMapping return

* chore: add changeset

---------

Co-authored-by: shadcn <[email protected]>
kjxbyz pushed a commit to muse-ui/muse-ui that referenced this pull request Jun 7, 2024
* fix(cli): deduplicate classNames in applyColorMapping

* refactor: simplify applyColorMapping return

* chore: add changeset

---------

Co-authored-by: shadcn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cli bug Something isn't working
Projects
None yet
3 participants