From 90246aa015eabe69603f899843950bc1ede4cb35 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 31 May 2024 10:26:23 -0700 Subject: [PATCH] feat: add typings for + )).toJSON(); expect(tree).toMatchSnapshot(); }); @@ -94,9 +94,21 @@ describe('); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const ref = (_current: HTMLAnchorElement) => {}; // Check typing of a ref - should not show type errors. + render(); expect(screen.getByRole('link').getAttribute('href')).toEqual('https://www.poop.com/💩'); }); }); + + test('with size="inline"', () => { + const tree = renderer.create(( +

+ 2 items selected. + +

+ )).toJSON(); + expect(tree).toMatchSnapshot(); + }); }); }); diff --git a/src/Button/ButtonGroup.test.jsx b/src/Button/ButtonGroup.test.tsx similarity index 100% rename from src/Button/ButtonGroup.test.jsx rename to src/Button/ButtonGroup.test.tsx diff --git a/src/Button/ButtonToolbar.test.jsx b/src/Button/ButtonToolbar.test.tsx similarity index 100% rename from src/Button/ButtonToolbar.test.jsx rename to src/Button/ButtonToolbar.test.tsx diff --git a/src/Button/__snapshots__/Button.test.jsx.snap b/src/Button/__snapshots__/Button.test.tsx.snap similarity index 92% rename from src/Button/__snapshots__/Button.test.jsx.snap rename to src/Button/__snapshots__/Button.test.tsx.snap index 863a1f933e..2cfb8b2f61 100644 --- a/src/Button/__snapshots__/Button.test.jsx.snap +++ b/src/Button/__snapshots__/Button.test.tsx.snap @@ -55,13 +55,13 @@ exports[` `; + +exports[` +

+`; diff --git a/src/Button/__snapshots__/ButtonGroup.test.jsx.snap b/src/Button/__snapshots__/ButtonGroup.test.tsx.snap similarity index 100% rename from src/Button/__snapshots__/ButtonGroup.test.jsx.snap rename to src/Button/__snapshots__/ButtonGroup.test.tsx.snap diff --git a/src/Button/__snapshots__/ButtonToolbar.test.jsx.snap b/src/Button/__snapshots__/ButtonToolbar.test.tsx.snap similarity index 100% rename from src/Button/__snapshots__/ButtonToolbar.test.jsx.snap rename to src/Button/__snapshots__/ButtonToolbar.test.tsx.snap diff --git a/src/Button/index.jsx b/src/Button/index.tsx similarity index 51% rename from src/Button/index.jsx rename to src/Button/index.tsx index 9ac374aa65..80c3bc3350 100644 --- a/src/Button/index.jsx +++ b/src/Button/index.tsx @@ -1,31 +1,55 @@ import React from 'react'; -import PropTypes from 'prop-types'; +import PropTypes, { type Requireable } from 'prop-types'; import classNames from 'classnames'; -import BaseButton from 'react-bootstrap/Button'; -import BaseButtonGroup from 'react-bootstrap/ButtonGroup'; -import BaseButtonToolbar from 'react-bootstrap/ButtonToolbar'; +import BaseButton, { type ButtonProps as BaseButtonProps } from 'react-bootstrap/Button'; +import BaseButtonGroup, { type ButtonGroupProps as BaseButtonGroupProps } from 'react-bootstrap/ButtonGroup'; +import BaseButtonToolbar, { type ButtonToolbarProps } from 'react-bootstrap/ButtonToolbar'; +import type { ComponentWithAsProp } from '../utils/types/bootstrap'; import Icon from '../Icon'; -const Button = React.forwardRef(({ +interface ButtonProps extends Omit { + /** + * An icon component to render. Example: + * ``` + * import { Close } from '@openedx/paragon/icons'; + * + * ``` + */ + iconBefore?: React.ComponentType; + /** + * An icon component to render. Example: + * ``` + * import { Close } from '@openedx/paragon/icons'; + * + * ``` + */ + iconAfter?: React.ComponentType; + size?: 'sm' | 'md' | 'lg' | 'inline'; +} + +type ButtonType = ComponentWithAsProp<'button', ButtonProps> & { Deprecated?: any }; + +const Button: ButtonType = React.forwardRef(({ children, iconAfter, iconBefore, + size, ...props }, ref) => ( types do not allow 'md' or 'inline', but we do. {...props} className={classNames(props.className)} ref={ref} > - {iconBefore && } + {iconBefore && } {children} - {iconAfter && } + {iconAfter && } )); Button.propTypes = { - ...Button.propTypes, /** Specifies class name to apply to the button */ className: PropTypes.string, /** Disables the Button, preventing mouse events, even if the underlying component is an `` element */ @@ -51,10 +75,13 @@ Button.propTypes = { variant: PropTypes.string, /** An icon component to render. * Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';` */ - iconBefore: PropTypes.oneOfType([PropTypes.elementType, PropTypes.node]), + iconBefore: PropTypes.elementType as Requireable, /** An icon component to render. * Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';` */ - iconAfter: PropTypes.oneOfType([PropTypes.elementType, PropTypes.node]), + iconAfter: PropTypes.elementType as Requireable, + // The 'as' type casting above is required for TypeScript checking, because the 'PropTypes.elementType' type normally + // allows strings as a value (for use cases like 'div') but we don't support that for /iconBefore/iconAfter. + // The React TypeScript type definitions are more specific (React.ComponentType vs React.ElementType). }; Button.defaultProps = { @@ -66,20 +93,29 @@ Button.defaultProps = { disabled: false, }; -function ButtonGroup(props) { - return ; -} -function ButtonToolbar(props) { - return ; +// We could just re-export 'ButtonGroup' and 'ButtonToolbar', but we currently +// override them to add propTypes validation at runtime, since most Paragon +// consumers aren't using TypeScript yet. We also force ButtonGroup's 'size' +// prop to accept our custom values of 'md' and 'inline' which are used in +// Paragon but not used in the base Bootstrap classes. + +interface ButtonGroupProps extends Omit { + size?: 'sm' | 'md' | 'lg' | 'inline'; } +const ButtonGroup: ComponentWithAsProp<'div', ButtonGroupProps> = ( + React.forwardRef(({ size, ...props }, ref) => ( + + )) +); + ButtonGroup.propTypes = { /** Specifies element type for this component. */ as: PropTypes.elementType, /** An ARIA role describing the button group. */ role: PropTypes.string, /** Specifies the size for all Buttons in the group. */ - size: PropTypes.oneOf(['sm', 'md', 'lg']), + size: PropTypes.oneOf(['sm', 'md', 'lg', 'inline']), /** Display as a button toggle group. */ toggle: PropTypes.bool, /** Specifies if the set of Buttons should appear vertically stacked. */ @@ -97,6 +133,12 @@ ButtonGroup.defaultProps = { size: 'md', }; +const ButtonToolbar: ComponentWithAsProp<'div', ButtonToolbarProps> = ( + React.forwardRef((props, ref) => ( + + )) +); + ButtonToolbar.propTypes = { /** An ARIA role describing the button group. */ role: PropTypes.string, diff --git a/src/Chip/ChipIcon.tsx b/src/Chip/ChipIcon.tsx index a32692c5ce..87e08aa256 100644 --- a/src/Chip/ChipIcon.tsx +++ b/src/Chip/ChipIcon.tsx @@ -8,7 +8,7 @@ import { STYLE_VARIANTS } from './constants'; export interface ChipIconProps { className: string, - src: React.ReactElement | Function, + src: React.ComponentType, onClick?: KeyboardEventHandler & MouseEventHandler, alt?: string, variant: string, diff --git a/src/Chip/index.tsx b/src/Chip/index.tsx index 23abfde5c7..0f78ab2059 100644 --- a/src/Chip/index.tsx +++ b/src/Chip/index.tsx @@ -1,5 +1,5 @@ import React, { ForwardedRef, KeyboardEventHandler, MouseEventHandler } from 'react'; -import PropTypes from 'prop-types'; +import PropTypes, { type Requireable } from 'prop-types'; import classNames from 'classnames'; // @ts-ignore import { requiredWhen } from '../utils/propTypes'; @@ -15,9 +15,9 @@ export interface IChip { onClick?: KeyboardEventHandler & MouseEventHandler, className?: string, variant?: string, - iconBefore?: React.ReactElement | Function, + iconBefore?: React.ComponentType, iconBeforeAlt?: string, - iconAfter?: React.ReactElement | Function, + iconAfter?: React.ComponentType, iconAfterAlt?: string, onIconBeforeClick?: KeyboardEventHandler & MouseEventHandler, onIconAfterClick?: KeyboardEventHandler & MouseEventHandler, @@ -111,7 +111,7 @@ Chip.propTypes = { * * `import { Check } from '@openedx/paragon/icons';` */ - iconBefore: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + iconBefore: PropTypes.elementType as Requireable, /** Specifies icon alt text. */ iconBeforeAlt: requiredWhen(PropTypes.string, ['iconBefore', 'onIconBeforeClick']), /** A click handler for the `Chip` icon before. */ @@ -122,7 +122,7 @@ Chip.propTypes = { * * `import { Check } from '@openedx/paragon/icons';` */ - iconAfter: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + iconAfter: PropTypes.elementType as Requireable, /** Specifies icon alt text. */ iconAfterAlt: requiredWhen(PropTypes.string, ['iconAfter', 'onIconAfterClick']), /** A click handler for the `Chip` icon after. */ diff --git a/src/Icon/index.d.ts b/src/Icon/index.d.ts index 45505bb49a..b9d6f5d746 100644 --- a/src/Icon/index.d.ts +++ b/src/Icon/index.d.ts @@ -1,13 +1,15 @@ import React from 'react'; export interface IconProps extends React.ComponentPropsWithoutRef<'span'> { - src?: React.ReactElement | Function; + // Note: React.ComponentType is what we want here. React.ElementType would allow some element type strings like "div", + // but we only want to allow components like 'Add' (a specific icon component function/class) + src?: React.ComponentType; svgAttrs?: { 'aria-label'?: string; 'aria-labelledby'?: string; }; id?: string | null; - size?: 'xs' | 'sm' | 'md' | 'lg'; + size?: 'xs' | 'sm' | 'md' | 'lg' | 'inline'; className?: string | string[]; hidden?: boolean; screenReaderText?: React.ReactNode; diff --git a/src/Icon/index.jsx b/src/Icon/index.jsx index 6f0a7a3cf3..89403430f2 100644 --- a/src/Icon/index.jsx +++ b/src/Icon/index.jsx @@ -74,7 +74,7 @@ Icon.propTypes = { * An icon component to render. * Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';` */ - src: PropTypes.oneOfType([PropTypes.element, PropTypes.elementType]), + src: PropTypes.elementType, /** HTML element attributes to pass through to the underlying svg element */ svgAttrs: PropTypes.shape({ 'aria-label': PropTypes.string, diff --git a/src/index.d.ts b/src/index.d.ts index e4050de325..8cd1eaea8b 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -5,6 +5,7 @@ // Things that have types // // // // // // // // // // // // // // // // // // // // // // // // // // // export { default as Bubble } from './Bubble'; +export { default as Button, ButtonGroup, ButtonToolbar } from './Button'; export { default as Chip, CHIP_PGN_CLASS } from './Chip'; export { default as ChipCarousel } from './ChipCarousel'; export { default as Hyperlink, HYPER_LINK_EXTERNAL_LINK_ALT_TEXT, HYPER_LINK_EXTERNAL_LINK_TITLE } from './Hyperlink'; @@ -21,7 +22,6 @@ export const Avatar: any; // from './Avatar'; export const AvatarButton: any; // from './AvatarButton'; export const Badge: any; // from './Badge'; export const Breadcrumb: any; // from './Breadcrumb'; -export const Button: any, ButtonGroup: any, ButtonToolbar: any; // from './Button'; export const Card: any, CardColumns: any, diff --git a/src/index.js b/src/index.js index 1b28bb4c94..0e24d8d85f 100644 --- a/src/index.js +++ b/src/index.js @@ -1,10 +1,11 @@ -// To keep this file in sync with the .d.ts file, it's in the same order -// and each line number is the same +// Keep this file in sync with the .d.ts file (manually). It's in the same order +// and each line number is the same, to make it easier. // // // // // // // // // // // // // // // // // // // // // // // // // // // // Things that have types // // // // // // // // // // // // // // // // // // // // // // // // // // // export { default as Bubble } from './Bubble'; +export { default as Button, ButtonGroup, ButtonToolbar } from './Button'; export { default as Chip, CHIP_PGN_CLASS } from './Chip'; export { default as ChipCarousel } from './ChipCarousel'; export { default as Hyperlink, HYPER_LINK_EXTERNAL_LINK_ALT_TEXT, HYPER_LINK_EXTERNAL_LINK_TITLE } from './Hyperlink'; @@ -21,7 +22,6 @@ export { default as Avatar } from './Avatar'; export { default as AvatarButton } from './AvatarButton'; export { default as Badge } from './Badge'; export { default as Breadcrumb } from './Breadcrumb'; -export { default as Button, ButtonGroup, ButtonToolbar } from './Button'; export { default as Card, CardColumns, diff --git a/src/utils/types/bootstrap.test.tsx b/src/utils/types/bootstrap.test.tsx new file mode 100644 index 0000000000..0346c5d2b4 --- /dev/null +++ b/src/utils/types/bootstrap.test.tsx @@ -0,0 +1,86 @@ +/* eslint-disable @typescript-eslint/no-unused-vars */ +import React from 'react'; +import type { BsPropsWithAs, ComponentWithAsProp } from './bootstrap'; + +// Note: these are type-only tests. They don't actually do much at runtime; the important checks are at transpile time. + +describe('BsPropsWithAs', () => { + interface Props extends BsPropsWithAs { + otherProp?: number; + } + + it('defines optional bsPrefix, className, and as but no other props', () => { + const checkProps = (_props: Props) => {}; + // These are all valid props per the prop definition: + checkProps({ }); + checkProps({ bsPrefix: 'bs' }); + checkProps({ className: 'foo bar' }); + checkProps({ as: 'tr' }); + checkProps({ className: 'foo bar', as: 'button', otherProp: 15 }); + // But these are all invalid: + // @ts-expect-error + checkProps({ newProp: 10 }); + // @ts-expect-error + checkProps({ onClick: () => {} }); + // @ts-expect-error + checkProps({ id: 'id' }); + // @ts-expect-error + checkProps({ children: }); + }); +}); + +describe('ComponentWithAsProp', () => { + interface MyProps extends BsPropsWithAs { + customProp?: string; + } + const MyComponent: ComponentWithAsProp<'div', MyProps> = ( + React.forwardRef( + ({ as: Inner = 'div', ...props }, ref) => , + ) + ); + + // eslint-disable-next-line react/function-component-definition + const CustomComponent: React.FC<{ requiredProp: string }> = () => ; + + it('is defined to wrap a
by default, and accepts related props', () => { + // This is valid - by default it is a DIV so accepts props and ref related to DIV: + const divClick: React.MouseEventHandler = () => {}; + const divRef: React.RefObject = { current: null }; + const valid = ; + }); + + it('is defined to wrap a
by default, and rejects unrelated props', () => { + const btnRef: React.RefObject = { current: null }; + // @ts-expect-error because the ref is to a