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

Change icon import names to have a prefix e.g. <LucideCamera /> #806

Closed
jacobawebb opened this issue Oct 7, 2022 · 24 comments · Fixed by #899
Closed

Change icon import names to have a prefix e.g. <LucideCamera /> #806

jacobawebb opened this issue Oct 7, 2022 · 24 comments · Fixed by #899
Labels
🐛 bug Something isn't working

Comments

@jacobawebb
Copy link

It's not all the time I run into this issue but I often find myself having to use different icons or renaming components within React projects so that they don't clash with the imports.

E.g. if I want to use <Login /> Lucide icon but I have a <Login /> functional component.

A suggestion would be to prefix the imports so <Camera /> would become <LcCamera /> or similar.

@jacobawebb jacobawebb added the 🐛 bug Something isn't working label Oct 7, 2022
@ericfennis
Copy link
Member

Hmm, I understand the issue. I think it is a bit hard to change. Because it is a breaking change for everyone. But we are thinking of adding aliases in the future, maybe we can think of a solution as well.

@jacobawebb
Copy link
Author

It would be a nice solution to have multiple names for the same export but I can't think of a nice way to do this without module / function duplication.

@jakobsen
Copy link

I disagree with this suggestion. Renaming imports is trivial and more flexible than introducing some arbitrary prefix that would be a breaking change.

Assuming you're using Typescript/Javascript, a suggestion for how to handle it could for instance be to create something like components/icons/index.[ts|js] and do

export {
  Camera as CameraIcon,
  LogIn as LoginIcon,
  // and whatever other icons you need with whatever pre-/postfix you prefer
} from "lucide-react";

They may then be imported as import { CameraIcon } from "/components/icons".

@jacobawebb
Copy link
Author

jacobawebb commented Oct 24, 2022 via email

@jakobsen
Copy link

Another way you could do it is to import as

import * as Lucide from "lucide-react";

and then render components as

<Lucide.Camera />

I think namespacing imports properly should be the responsibility of the app developer, not the library developer, and therefore disagree that this is something that needs a solution from Lucide's or @ericfennis' end.

@ericfennis
Copy link
Member

We have a plan to support aliases in all our packages that uses lucide icons as components.
I was thinking to add an aliases.js file with all the icons aliases. and export them in the main index.ts.
import { Camera } from "lucide-react"; or `import { CameraIcon } from "lucide-react"; will lead to the same icon file. This only will working nicely with ESModules.

If you want to have all the icons you can then still do import * as icons from "lucide-react/icons";
But importing icons this way is not recommended and bad practice if you are bundeling your application for browsers. This way of importing breaks the tree-shaking ability in bundlers. So your bundled app will include all 800+ icons.

@Snailedlt
Copy link

import {CameraIcon} from "lucide-react";

would be ideal imo.

Will aliases like you mention above also be added to other language packages like Svelte and Vue? And would it still cause issues if I had a local component named Camera?
For example:

import {Camera} from "./src/CustomButtons"

@ericfennis
Copy link
Member

@Snailedlt yess, we want to implement it in all packages.

@Snailedlt
Copy link

@ericfennis Neat!
What about my second question?

And would it still cause issues if I had a local component named Camera? For example:

import {Camera} from "./src/CustomButtons"

@jacobawebb
Copy link
Author

We have a plan to support aliases in all our packages that uses lucide icons as components. I was thinking to add an aliases.js file with all the icons aliases. and export them in the main index.ts. import { Camera } from "lucide-react"; or `import { CameraIcon } from "lucide-react"; will lead to the same icon file. This only will working nicely with ESModules.

If you want to have all the icons you can then still do import * as icons from "lucide-react/icons"; But importing icons this way is not recommended and bad practice if you are bundeling your application for browsers. This way of importing breaks the tree-shaking ability in bundlers. So your bundled app will include all 800+ icons.

I can understand where some may think this is the responsibility of the developer, your application, your job sort of thing.

On the other side, packages are meant to make life simpler, without having to make unecessary accommodations, whilst also maintaining a small bundle size.

I raised this issue initially, and although biased, I do think it's the best approach to just simply rename them completely and announce a breaking change.

Maintaining an aliases.js would work, but adds more room for potentially forgetting an icon and it not being supported with that naming convention.

I'd propose to simply change the naming to LucideCamera or LICamera.

https://react-icons.github.io/react-icons/icons?name=fi React icons has this exact setup, although it's due to it having multiple icon packages and it needs to distinguish between them, it does work really well.

@ericfennis ericfennis mentioned this issue Jan 13, 2023
8 tasks
@ericfennis
Copy link
Member

@jakobsen I completely agree.
Maintaining aliases.js is not a problem. I already started on this feature in #899. The aliases file is completely rendered on build time so no SVG icon will be missed that is in the icons directory.

I did already some tests and it works great.
You can import each icon in three ways:

import { Camera } from 'lucide-{packageName}' import { LucideCamera } from 'lucide-{packageName}'
`import { CameraIcon } from 'lucide-{packageName}'

It will point all to the same module. So tree-shaking is still working.

See the linked draft PR #899

@piotrkulpinski
Copy link

This is still not ideal IMO as the icons pollute the component name scope quite a bit.
For example if I want to import the NextJS Link component, the IDE automatically suggests Link icon from Lucide which is not ideal.

@karsa-mistmere
Copy link
Member

@piotrkulpinski: I'm afraid for the time being the old, unprefixed names will have be kept for backwards compatibility, although I do agree that using a prefixed version from the start would have been ideal. Maybe we can drop the unprefixed names in v1.0.

@jguddas
Copy link
Member

jguddas commented Sep 15, 2023

This is IMO a ts-server issue now.

microsoft/TypeScript#51155

@capaj
Copy link

capaj commented Oct 25, 2023

@jguddas solving it in ts-server is super super complex though.

I would love to see prefixes. Ideally we can have prefixed react components in a separate package like
import { LucideCamera } from 'lucide-react/prefixed'
? Introducing it like this would not break anyone's existing code.

@karsa-mistmere
Copy link
Member

@capaj We do already have prefixed components, the issue is no longer with that, but dropping support for the unprefixed ones.

@capaj
Copy link

capaj commented Oct 25, 2023

@karsa-mistmere what do you mean?
Where? I may be stupid, but I don't see how to import them with a prefix. Are you suggesting I use
https://lucide.dev/guide/packages/lucide-react#one-generic-icon-component

? I could, but I don't like that solution, because I bundle with Vite and it would force all the icons into my bundle.

@karsa-mistmere
Copy link
Member

karsa-mistmere commented Oct 25, 2023

I'm not quite sure what generic components have to do with prefixed component names.

This issue has to do with adding prefixes to icon component names, which has already been implemented via alias exports, e.g. in lucide-react ArrowUpSquare is exported as both ArrowUpSquare and ArrowUpSquareIcon as well as LucideArrowUpSquare.

E.g. you can easily use

import { LucideCamera } from 'lucide-react';

instead of

import { Camera } from 'lucide-react';

@capaj
Copy link

capaj commented Oct 25, 2023

Thanks!
I was just trying it wrong

@ericfennis
Copy link
Member

I think we should write something about these prefixes and suffixes in our documentation.

@rwieruch
Copy link

rwieruch commented Mar 27, 2024

Adding my two cents here: For the following:

<Link>

I always end up with the following import:

import { Link } from 'lucide-react';

Instead I am always in need for this one:

import Link from 'next/link';

And I guess there a re more conflicts like this. Because Lucid has the alias with *Icon I always use icons like this:

<LinkIcon />

Then it gets the import right for obvious reasons. However, I would also like to avoid it the other way around as mentioned above. I know it's a breaking change, but what if Lucide would only enable *Icon, e.g. LinkIcon imports (so not exporting the Link equivalent from the library)? It would not only solve the auto import issue, but also make things more explicit in code, because there would be no mental overhead on first sight anymore.

@capaj
Copy link

capaj commented Mar 27, 2024

I think prefixes are better than suffixes. Also prefix 'Lucide' is better than 'Icon'.
If you have multiple icon packs in your codebase lucide prefix makes it perfectly clear what you're working with.

@rwieruch
Copy link

Agreed. I think my suggestion was more towards removing the exports in Lucid that do not have a prefix/suffix, such as Link.

@ericfennis
Copy link
Member

Thanks for your view on this!
We have a related discussion in #1830.
I've also written a proposal in #1830, so let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants