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

[enhancement] flexible export from root for props #234

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

xettri
Copy link
Contributor

@xettri xettri commented Nov 17, 2023

DropdownProps

Before

import { DropdownProps } from 'rc-dropdown/lib/Dropdown';

Now

import { DropdownProps } from 'rc-dropdown/lib/Dropdown';
import { DropdownProps } from 'rc-dropdown';

OverlayProps

Before

import { OverlayProps } from 'rc-dropdown/lib/Overlay';

Now

import { OverlayProps } from 'rc-dropdown/lib/Overlay';
import { OverlayProps } from 'rc-dropdown';

TriggerProps

Before

Not supported

Hack
install: @rc-component/trigger
import { TriggerProps } from '@rc-component/trigger';

Now

import { TriggerProps } from 'rc-dropdown';
import { TriggerProps } from 'rc-dropdown/lib/Dropdown';

Why need TriggerProps ?

We pass all rest props to Trigger (@rc-component/trigger), for advance customization we required that support in ts so we can skip extra install @rc-component/trigger, with install externally it can lead issue because of version miss match.

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6138657) 100.00% compared to head (6927c17) 100.00%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #234   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          107       107           
  Branches        31        31           
=========================================
  Hits           107       107           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/index.tsx Outdated
@@ -1,3 +1,6 @@
export * from './Dropdown';
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 * from './Dropdown';
export type { TriggerProps, DropdownProps } from './Dropdown';

Do not use export *.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@afc163 sounds better, updated PR with this suggestion

src/index.tsx Show resolved Hide resolved
src/index.tsx Outdated
@@ -1,3 +1,6 @@
import Dropdown from './Dropdown'
export type { DropdownProps, TriggerProps } from './Dropdown';
Copy link
Member

@afc163 afc163 Nov 20, 2023

Choose a reason for hiding this comment

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

Suggested change
export type { DropdownProps, TriggerProps } from './Dropdown';
export type { TriggerProps } from '@rc-component/trigger';
export type { DropdownProps } from './Dropdown';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@afc163 got your point, updated the PR

Copy link
Contributor Author

@xettri xettri left a comment

Choose a reason for hiding this comment

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

Resolved feedback's

@afc163 afc163 merged commit c392d2d into react-component:master Apr 22, 2024
9 checks passed
@xettri xettri deleted the fix-export-issue branch April 22, 2024 12:57
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.

2 participants