Skip to content

[enhancement] flexible export from root for props#234

Merged
afc163 merged 1 commit into
react-component:masterfrom
xettri:fix-export-issue
Apr 22, 2024
Merged

[enhancement] flexible export from root for props#234
afc163 merged 1 commit into
react-component:masterfrom
xettri:fix-export-issue

Conversation

@xettri
Copy link
Copy Markdown
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.

@codecov
Copy link
Copy Markdown

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.

Comment thread src/index.tsx Outdated
@@ -1,3 +1,6 @@
export * from './Dropdown';
Copy link
Copy Markdown
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
Copy Markdown
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

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