-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
refactor: remove unnecessary props types #4530
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces type safety improvements across multiple components in the CodeRabbit UI library. The changes primarily focus on refining type definitions for various component props, removing Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
It could also be addressed separately, but we might be able to remove null
as well.
https://github.com/search?q=repo%3Anextui-org%2Fnextui+%22ReactNode+%7C+null%22&type=code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/docs/content/docs/components/accordion.mdx
(1 hunks)apps/docs/content/docs/components/navbar.mdx
(1 hunks)packages/components/accordion/src/accordion-item.tsx
(1 hunks)packages/components/accordion/src/base/accordion-item-base.tsx
(1 hunks)packages/components/listbox/src/base/listbox-item-base.tsx
(1 hunks)packages/components/listbox/src/listbox-item.tsx
(1 hunks)packages/components/menu/src/base/menu-item-base.tsx
(1 hunks)packages/components/menu/src/menu-item.tsx
(1 hunks)packages/components/snippet/src/use-snippet.ts
(1 hunks)packages/components/tabs/src/base/tab-item-base.ts
(1 hunks)packages/components/user/src/use-user.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: TypeScript
- GitHub Check: Build
🔇 Additional comments (10)
packages/components/tabs/src/base/tab-item-base.ts (1)
7-7
: LGTM! Simplified type definitionsThe changes correctly remove redundant
null
from union types sinceReactNode
already includesnull
. This aligns with the PR objective and maintains the same runtime behavior.Also applies to: 11-11
packages/components/listbox/src/listbox-item.tsx (1)
32-32
: LGTM! Type matches implementationThe type change to
ReactNode
accurately reflects the implementation, as the logic always returns a validReactNode
and never returnsnull
.packages/components/menu/src/menu-item.tsx (1)
35-35
: LGTM! Type matches implementationThe type change to
ReactNode
accurately reflects the implementation, as the logic always returns a validReactNode
and never returnsnull
.packages/components/user/src/use-user.ts (1)
20-20
: LGTM! Simplified type definitionsThe changes correctly remove redundant
string
from union types sinceReactNode
already includesstring
. This aligns with the PR objective and maintains the same runtime behavior.Also applies to: 24-24
packages/components/menu/src/base/menu-item-base.tsx (1)
28-28
: LGTM! Type definitions simplified correctly.The changes correctly remove redundant union types since
ReactNode
already includes bothstring
andnull
. This maintains type safety while making the types more concise.Also applies to: 32-32, 36-36, 40-40
packages/components/accordion/src/base/accordion-item-base.tsx (1)
36-36
: LGTM! Type definitions simplified correctly.The changes correctly remove redundant union types since
ReactNode
already includes bothstring
andnull
. This maintains type safety while making the types more concise.Also applies to: 40-40, 44-44
packages/components/listbox/src/base/listbox-item-base.tsx (1)
28-28
: LGTM! Type definitions simplified correctly.The changes correctly remove redundant union types since
ReactNode
already includes bothstring
andnull
. This maintains type safety while making the types more concise.Also applies to: 32-32, 36-36
packages/components/accordion/src/accordion-item.tsx (1)
43-43
: LGTM! Type annotation simplified correctly.The type annotation change for
indicatorContent
correctly removes the redundant union withnull
sinceReactNode
already includesnull
. The function can still returnnull
explicitly, maintaining the same behavior.apps/docs/content/docs/components/navbar.mdx (1)
415-415
: LGTM! Improved type safety.The removal of
null
from the return type enforces that the icon function must always return a valid ReactNode, preventing potential null pointer issues.apps/docs/content/docs/components/accordion.mdx (1)
330-342
: LGTM! Type definitions simplified.The removal of explicit
string
type fromchildren
,title
, andsubtitle
props is correct asReactNode
already includesstring
. This change simplifies the type definitions without affecting functionality.
* docs: optimize route higtlight (#4520) * docs: optimize home display (#4519) * docs: optimize home display and route highlight * docs: optimize home display * fix(alert): propagate className (#4535) * fix(alert): propagate className * chore(alert): remove className from alert theme * fix(avatar): title type in Avatar (#4529) * fix(avatar): title type in Avatar * fix(alert): apply isEmpty check on title * fix(alert): alert interface props type * refactor: remove unnecessary props types (#4530) * refactor(docs): remove string type as it is included in ReactNode * refactor: remove unnecessary types * feat(changeset): add changeset * chore: remove changeset * refactor: remove null since ReactNode unions it already * fix(input): use onPress for wrapper click focus (#4483) * fix(input): use onPress for wrapper click focus * test(input): wrapper click focus test * chore(changeset): input onPress for wrapper click focus * chore(changeset): minor wording * Refactor/rebrand (#4532) * chore: rebrand in progress * chore: update docs to use heroui * chore: components renbranded * chore: figma moved to the docs files * fix: posthog config * fix(docs): extra classname in form example (#4465) * chore: clean git * chore: make heroui private * chore: new logo * chore: node env var renamed * chore: public robots txt deleted * chore: wrangler installed * chore: wrangler renamed * chore: cloudlfare workers removed * chore: force vercel deploy * refactor: first migration and provider * refactor: rename nextui plugin * refactor: rename github site * refactor: rename CONTRIBUTING * refactor: rename package name * refactor: nextjs image hostname * refactor: mdx repo nextui-org rename frontio-ai * refactor: nextui.org rename heroui.com * refactor: add heroui to missing places * fix: heroui plugin name * fix: update docs * docs: nextui to heroui add npmrc pnpm migratation * chore: rename all packages with new org name * chore: replace frontio-ai by frontioai * chore: revert previous changes * chore: small adjustment * chore: doc updated * feat: blog * chore: avatar updated * fix: url * chore: add new ogimage * fix: ogimage command * fix: heroui name and storybook welcome page * fix: og image url * feat: favicon and icon changed --------- Co-authored-by: աӄա <[email protected]> Co-authored-by: winches <[email protected]> * fix: postbuild script * chore: core package updates * ci(changesets): version packages (#4569) Co-authored-by: Junior Garcia <[email protected]> * feat: contributors added to the blog --------- Co-authored-by: winches <[email protected]> Co-authored-by: աӄա <[email protected]> Co-authored-by: Peterl561 <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* docs: optimize route higtlight (#4520) * docs: optimize home display (#4519) * docs: optimize home display and route highlight * docs: optimize home display * fix(alert): propagate className (#4535) * fix(alert): propagate className * chore(alert): remove className from alert theme * fix(avatar): title type in Avatar (#4529) * fix(avatar): title type in Avatar * fix(alert): apply isEmpty check on title * fix(alert): alert interface props type * refactor: remove unnecessary props types (#4530) * refactor(docs): remove string type as it is included in ReactNode * refactor: remove unnecessary types * feat(changeset): add changeset * chore: remove changeset * refactor: remove null since ReactNode unions it already * fix(input): use onPress for wrapper click focus (#4483) * fix(input): use onPress for wrapper click focus * test(input): wrapper click focus test * chore(changeset): input onPress for wrapper click focus * chore(changeset): minor wording * Refactor/rebrand (#4532) * chore: rebrand in progress * chore: update docs to use heroui * chore: components renbranded * chore: figma moved to the docs files * fix: posthog config * fix(docs): extra classname in form example (#4465) * chore: clean git * chore: make heroui private * chore: new logo * chore: node env var renamed * chore: public robots txt deleted * chore: wrangler installed * chore: wrangler renamed * chore: cloudlfare workers removed * chore: force vercel deploy * refactor: first migration and provider * refactor: rename nextui plugin * refactor: rename github site * refactor: rename CONTRIBUTING * refactor: rename package name * refactor: nextjs image hostname * refactor: mdx repo nextui-org rename frontio-ai * refactor: nextui.org rename heroui.com * refactor: add heroui to missing places * fix: heroui plugin name * fix: update docs * docs: nextui to heroui add npmrc pnpm migratation * chore: rename all packages with new org name * chore: replace frontio-ai by frontioai * chore: revert previous changes * chore: small adjustment * chore: doc updated * feat: blog * chore: avatar updated * fix: url * chore: add new ogimage * fix: ogimage command * fix: heroui name and storybook welcome page * fix: og image url * feat: favicon and icon changed --------- Co-authored-by: աӄա <[email protected]> Co-authored-by: winches <[email protected]> * fix: postbuild script * chore: core package updates * ci(changesets): version packages (#4569) Co-authored-by: Junior Garcia <[email protected]> * feat: contributors added to the blog * Revert "fix(input): use onPress for wrapper click focus (#4483)" (#4576) This reverts commit 992220a. * fix: input spaces * ci(changesets): version packages (#4577) Co-authored-by: Junior Garcia <[email protected]> --------- Co-authored-by: winches <[email protected]> Co-authored-by: աӄա <[email protected]> Co-authored-by: Peterl561 <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Closes #
📝 Description
Given that
ReactNode
already unionsstring
andnull
, we don't need to union them again.⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit