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

replace keyof ReactSVG with SVGElementType #2668

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aslilac
Copy link

@aslilac aslilac commented Dec 6, 2024

Closes #2667

What is the purpose of this pull request?

  • New Icon
  • Bug fix
  • New Feature
  • Documentation update
  • Other:

Description

The ReactSVG type has been removed from @types/react 19. The suggested replacement (at least, for lucide's use case) is to use SVGElementType instead.

The two imports removed from createLucideIcon.ts were actually unused and do not need to be replaced.

The use of import type in lucide-react/src/types.ts brings it in sync with lucide-react-native/src/types.ts.

Before Submitting

@ericfennis
Copy link
Member

@aslilac Thanks for the heads-up.
Hmm, SVGElementType is specific to @types/react 19, which means that a project with an older version will not work.
So I think a better idea is to define the element in ReactSVG ourselves so we are not dependent on @types/react

Copy link
Member

@ericfennis ericfennis left a comment

Choose a reason for hiding this comment

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

Replace types with custom ones.

packages/lucide-react-native/src/types.ts Show resolved Hide resolved
packages/lucide-react-native/src/types.ts Outdated Show resolved Hide resolved

export type IconNode = [elementName: keyof ReactSVG, attrs: Record<string, string>][];
export type IconNode = [elementName: SVGElementType, attrs: Record<string, string>][];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export type IconNode = [elementName: SVGElementType, attrs: Record<string, string>][];
type ElementName =
'circle'
| 'ellipse'
| 'g'
| 'line'
| 'path'
| 'polygon'
| 'polyline'
| 'rect';
export type IconNode = [elementName: ElementName, attrs: Record<string, string>][];

Copy link
Member

Choose a reason for hiding this comment

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

Can you provide this list exactly?
The other elements are never used by the icon nodes we provide.

packages/lucide-react/src/types.ts Outdated Show resolved Hide resolved
@aslilac
Copy link
Author

aslilac commented Dec 13, 2024

I think a better idea is to define the element in ReactSVG ourselves so we are not dependent on @types/react

That sounds like a good call. I vendored in its definition, with links to the original source.

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.

react 19: ReactSVG has been removed from @types/react
2 participants