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 @types/react as peer dep to lucide-react #2549

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

styu
Copy link

@styu styu commented Oct 22, 2024

What is the purpose of this pull request?

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

Description

Without this, in a PNPM + bazel setup, TypeScript cannot find react typings:

node_modules/.aspect_rules_js/[email protected]_814417300/node_modules/lucide-react/dist/lucide-react.d.ts(1,24): error TS7016: Could not find a declaration file for module 'react'. '/path/to/sandbox/darwin-sandbox/39/execroot/_main/bazel-out/darwin_x86_64-fastbuild/bin/node_modules/.aspect_rules_js/[email protected]/node_modules/react/index.js' implicitly has an 'any' type.
  If the 'react' package actually exposes this module, consider sending a pull request to amend 'https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react'

Since the exported types depend on @types/react, I think this package should either declare @types/react as a direct or a peer dep. I chose a peer dep in this PR to mirror what's being done with react, but happy to change it! I also wasn't sure if a fixed version or * would be preferred either.

In my repo, I was able to confirm this fixes my issue by adding a packageExtension in my root package.json:

"pnpm": {
    "packageExtensions": {
      "lucide-react": {
        "peerDependencies": {
          "@types/react": "*"
        }
      }
    }
}

Before Submitting

Without this, in a PNPM + bazel setup, TypeScript cannot find react typings:

```
node_modules/.aspect_rules_js/[email protected]_814417300/node_modules/lucide-react/dist/lucide-react.d.ts(1,24): error TS7016: Could not find a declaration file for module 'react'. '/path/to/sandbox/darwin-sandbox/39/execroot/_main/bazel-out/darwin_x86_64-fastbuild/bin/node_modules/.aspect_rules_js/[email protected]/node_modules/react/index.js' implicitly has an 'any' type.
  If the 'react' package actually exposes this module, consider sending a pull request to amend 'https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react'
```

Since the exported types depend on `@types/react`, I think this package should either declare `@types/react` as a direct or a peer dep. I chose a peer dep in this PR to mirror what's being done with `react`, but happy to change it!
@github-actions github-actions bot added the ⚛️ react package Lucide React Package label Oct 22, 2024
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚛️ react package Lucide React Package Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant