Skip to content

Conversation

@nadav-ab
Copy link

@nadav-ab nadav-ab commented Jul 21, 2025

add a list of icons and a pre-rendered version for lucide-react icons

nadav-ab added 3 commits July 21, 2025 11:28
- Bump @types/node to ^24.0.15 and add missing React type dependencies
- Add build tooling dependencies (@file-services/node, tsx)
- Remove unused type dependencies (@types/prop-types, @types/scheduler)
- Update lucide-react package build configuration
@nadav-ab nadav-ab requested a review from AviVahl July 22, 2025 07:10
Copy link
Contributor

@AviVahl AviVahl left a comment

Choose a reason for hiding this comment

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

Use conventional commits for PR title.
Add PR description.

the problem with the .json approach is that it is untyped, so when the package user imports it, everything is loose and could easily break with future changes of this package.


const svgsByContent = new Map<string, IconData>();

for (const [exportName, IconComponent] of Object.entries(lucideReact)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

lucide-react exports the same icon 3 times:

  • Speaker
  • LucideSpeaker
  • SpeakerIcon

So while we could list all 3, we could just list "Speaker", and use the "SpeakerIcon" export (for less code confusion)

Copy link
Contributor

Choose a reason for hiding this comment

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

could iterate on lucideReact.icons instead to avoid the duplicates

if (svgsByContent.has(svgString)) {
const existing = svgsByContent.get(svgString)!;
// Keep the shortest name
if (exportName.length < existing.name.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

while this works for the Abc/LucideAbc/AbcIcon case, this makes no sense for icons that are being renamed (with the old one kept around)

Copy link
Author

Choose a reason for hiding this comment

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

what do you suggest?

Copy link
Contributor

Choose a reason for hiding this comment

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

iterate on lucideReact.icons and avoid checking for same-content and aliases.

},
"node_modules/@eslint-community/eslint-utils/node_modules/eslint-visitor-keys": {
"version": "3.4.3",
"resolved": "https://registry.npmjs.org/eslint-visitor-keys/-/eslint-visitor-keys-3.4.3.tgz",
Copy link
Contributor

Choose a reason for hiding this comment

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

you've messed up the package-lock.json
this happens if you npm i without a package-lock, but with existing node_modules.

@nadav-ab nadav-ab changed the title start work on icon lib build script feat: add icon lib description for lucide-react Jul 24, 2025
- Remove tsx from root package dependencies and related esbuild packages
- Add new build script (do-build.ts) for lucide-react package
- Update lucide-react package configuration and README
- Add LICENSE file to lucide-react package
- Update TypeScript configurations across packages
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

Successfully merging this pull request may close these issues.

3 participants