-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use export * in component indexes and subcomponent reexports #2625
Changes from all commits
bdb8432
f25e7d0
2fdb183
dd74290
41d3267
f2ec124
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {AccountConnection, AccountConnectionProps} from './AccountConnection'; | ||
export * from './AccountConnection'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Item, ItemProps} from './Item'; | ||
export * from './Item'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Section, SectionProps} from './Section'; | ||
export * from './Section'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
export {Item, ItemProps} from './Item'; | ||
export * from './Item'; | ||
|
||
export {Section, SectionProps} from './Section'; | ||
export * from './Section'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {ActionList, ActionListProps} from './ActionList'; | ||
export * from './ActionList'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {MenuAction, MenuActionProps} from './MenuAction'; | ||
export * from './MenuAction'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {MenuGroup, MenuGroupProps} from './MenuGroup'; | ||
export * from './MenuGroup'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {RollupActions, RollupActionsProps} from './RollupActions'; | ||
export * from './RollupActions'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
export {MenuAction, MenuActionProps} from './MenuAction'; | ||
export * from './MenuAction'; | ||
|
||
export {MenuGroup, MenuGroupProps} from './MenuGroup'; | ||
export * from './MenuGroup'; | ||
|
||
export {RollupActions, RollupActionsProps} from './RollupActions'; | ||
export * from './RollupActions'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {ActionMenu, ActionMenuProps, hasGroupsWithActions} from './ActionMenu'; | ||
export * from './ActionMenu'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {AfterInitialMount} from './AfterInitialMount'; | ||
export * from './AfterInitialMount'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {AppProvider, AppProviderProps} from './AppProvider'; | ||
export * from './AppProvider'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {ComboBox} from './ComboBox'; | ||
export * from './ComboBox'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {TextField} from './TextField'; | ||
export * from './TextField'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
export {ComboBox} from './ComboBox'; | ||
export * from './ComboBox'; | ||
|
||
export {TextField} from './TextField'; | ||
export * from './TextField'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Autocomplete, AutocompleteProps} from './Autocomplete'; | ||
export * from './Autocomplete'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Avatar, AvatarProps} from './Avatar'; | ||
export * from './Avatar'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ import React from 'react'; | |
import {mountWithAppProvider} from 'test-utilities/legacy'; | ||
import {mountWithApp} from 'test-utilities'; | ||
import {Avatar, Image} from 'components'; | ||
import {STYLE_CLASSES} from '../Avatar'; | ||
|
||
describe('<Avatar />', () => { | ||
describe('intials', () => { | ||
|
@@ -116,10 +115,6 @@ describe('<Avatar />', () => { | |
}); | ||
|
||
describe('styleClass', () => { | ||
it('defaults to five styles', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is testing an internal implementation detail, I didn't want to expose STYLE_CLASSES so it was easiest to remove this test instead of moving files around to avoid exposing STYLE_CLASSES |
||
expect(STYLE_CLASSES).toHaveLength(5); | ||
}); | ||
|
||
it('renders a sixth style when unstableGlobalTheming is false', () => { | ||
const avatar = mountWithApp(<Avatar name="e" />, { | ||
features: {unstableGlobalTheming: false}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Backdrop, BackdropProps} from './Backdrop'; | ||
export * from './Backdrop'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1 @@ | ||
export { | ||
Badge, | ||
Progress, | ||
BadgeProps, | ||
Status, | ||
PROGRESS_LABELS, | ||
STATUS_LABELS, | ||
} from './Badge'; | ||
export * from './Badge'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Banner, BannerProps, BannerStatus} from './Banner'; | ||
export * from './Banner'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Breadcrumbs, BreadcrumbsProps} from './Breadcrumbs'; | ||
export * from './Breadcrumbs'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ import {PlusMinor} from '@shopify/polaris-icons'; | |
import {mountWithAppProvider, trigger} from 'test-utilities/legacy'; | ||
import {mountWithApp} from 'test-utilities'; | ||
import {UnstyledLink, Icon, Spinner} from 'components'; | ||
import {Button, IconWrapper} from '../Button'; | ||
import {Button} from '../Button'; | ||
|
||
describe('<Button />', () => { | ||
describe('url', () => { | ||
|
@@ -168,7 +168,7 @@ describe('<Button />', () => { | |
|
||
it('does not render the markup for the icon if none is provided', () => { | ||
const button = mountWithAppProvider(<Button />); | ||
expect(button.find(IconWrapper).exists()).toBe(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IconWrapper shouldn't be exposed outside of Button. |
||
expect(button.find('svg').exists()).toBe(false); | ||
}); | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Item, ItemProps} from './Item'; | ||
export * from './Item'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Item, ItemProps} from './Item'; | ||
export * from './Item'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {ButtonGroup, ButtonGroupProps} from './ButtonGroup'; | ||
export * from './ButtonGroup'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {CalloutCard, CalloutCardProps} from './CalloutCard'; | ||
export * from './CalloutCard'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Caption, CaptionProps} from './Caption'; | ||
export * from './Caption'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Header, HeaderProps} from './Header'; | ||
export * from './Header'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Section, SectionProps} from './Section'; | ||
export * from './Section'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Subsection, SubsectionProps} from './Subsection'; | ||
export * from './Subsection'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
export {Header, HeaderProps} from './Header'; | ||
export * from './Header'; | ||
|
||
export {Section, SectionProps} from './Section'; | ||
export * from './Section'; | ||
|
||
export {Subsection, SubsectionProps} from './Subsection'; | ||
export * from './Subsection'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Card, CardProps} from './Card'; | ||
export * from './Card'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ import {Error, Key, CheckboxHandles} from '../../types'; | |
|
||
import styles from './Checkbox.scss'; | ||
|
||
export interface BaseProps { | ||
export interface CheckboxProps { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This crops up in a few files - a BaseProps interface and the component's props that extend from that without adding anything news - so remove BaseProps. |
||
/** Indicates the ID of the element that describes the checkbox*/ | ||
ariaDescribedBy?: string; | ||
/** Label for the checkbox */ | ||
|
@@ -40,8 +40,6 @@ export interface BaseProps { | |
onBlur?(): void; | ||
} | ||
|
||
export interface CheckboxProps extends BaseProps {} | ||
|
||
export const Checkbox = React.forwardRef<CheckboxHandles, CheckboxProps>( | ||
function Checkbox( | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Checkbox, CheckboxProps} from './Checkbox'; | ||
export * from './Checkbox'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export {Choice, ChoiceProps, helpTextID} from './Choice'; | ||
export * from './Choice'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,10 @@ import {mountWithApp} from 'test-utilities'; | |
// eslint-disable-next-line no-restricted-imports | ||
import {mountWithAppProvider, ReactWrapper} from 'test-utilities/legacy'; | ||
import {RadioButton, Checkbox, InlineError, errorTextID} from 'components'; | ||
import {ChoiceList, ChoiceDescriptor} from '../ChoiceList'; | ||
import {ChoiceList, ChoiceListProps} from '../ChoiceList'; | ||
|
||
describe('<ChoiceList />', () => { | ||
let choices: ChoiceDescriptor[]; | ||
let choices: ChoiceListProps['choices']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where possible we should look at the values that live on the Props interface instead of exposing the interfaces the Props use |
||
|
||
beforeEach(() => { | ||
choices = [ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ export interface CollapsibleProps { | |
children?: React.ReactNode; | ||
} | ||
|
||
export type AnimationState = | ||
type AnimationState = | ||
| 'idle' | ||
| 'measuring' | ||
| 'closingStart' | ||
|
@@ -34,7 +34,7 @@ interface State { | |
|
||
const ParentCollapsibleExpandingContext = createContext(false); | ||
|
||
class Collapsible extends React.Component<CollapsibleProps, State> { | ||
class CollapsibleInner extends React.Component<CollapsibleProps, State> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kill off cases where we export a default value - now things that use AppProvider are exported with a named component, which means that the component index file doesn't need to rename the default export to a named export |
||
static contextType = ParentCollapsibleExpandingContext; | ||
|
||
static getDerivedStateFromProps( | ||
|
@@ -165,7 +165,7 @@ function collapsibleHeight( | |
return `${height || 0}px`; | ||
} | ||
|
||
// Use named export once we work out why not casting this breaks web | ||
// eslint-disable-next-line import/no-default-export | ||
export default Collapsible as ComponentClass<CollapsibleProps> & | ||
typeof Collapsible; | ||
export const Collapsible = CollapsibleInner as ComponentClass< | ||
CollapsibleProps | ||
> & | ||
typeof CollapsibleInner; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1 @@ | ||
import Collapsible, {CollapsibleProps} from './Collapsible'; | ||
|
||
export {Collapsible, CollapsibleProps}; | ||
export * from './Collapsible'; |
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.
This sort of thing crops up a lot. Using this particular case as an example,
Size
was exported here but was never reexported bysrc/Avatar/index.ts
and was never used in any other file, so there is no value in exporting it at all.