From c4f750b89cfee7457daa4e526c3a9beb655f8be1 Mon Sep 17 00:00:00 2001 From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> Date: Fri, 21 May 2021 08:01:13 -0700 Subject: [PATCH 01/12] Initial commit to require explicit children prop for all components --- packages/components/src/elevation/types.ts | 1 - packages/components/src/flex/types.ts | 15 ++++++++++++++- packages/components/src/grid/types.ts | 4 ++++ packages/components/src/spacer/hook.ts | 4 ++++ packages/components/src/text/types.ts | 4 ++++ packages/components/src/truncate/types.ts | 6 +++++- packages/components/src/ui/card/types.ts | 8 ++++++++ .../src/ui/context/polymorphic-component.ts | 10 +++++++--- .../src/ui/context/use-context-system.js | 4 +--- packages/components/src/ui/control-group/types.ts | 4 ++++ packages/components/src/ui/form-group/types.ts | 1 + packages/components/src/ui/popover/types.ts | 5 +++++ packages/components/src/ui/scrollable/types.ts | 4 ++++ packages/components/src/ui/surface/types.ts | 4 ++++ .../components/src/ui/visually-hidden/hook.js | 2 +- 15 files changed, 66 insertions(+), 10 deletions(-) diff --git a/packages/components/src/elevation/types.ts b/packages/components/src/elevation/types.ts index 02537255988ffa..dee387b07aa368 100644 --- a/packages/components/src/elevation/types.ts +++ b/packages/components/src/elevation/types.ts @@ -34,5 +34,4 @@ export type Props = { * In the example below, `isInteractive` is activated to give a better sense of depth. */ value?: number; - children?: never; }; diff --git a/packages/components/src/flex/types.ts b/packages/components/src/flex/types.ts index f4360e145c201b..ea545fef77c6bf 100644 --- a/packages/components/src/flex/types.ts +++ b/packages/components/src/flex/types.ts @@ -53,6 +53,10 @@ export type FlexProps = { * @default false */ wrap?: boolean; + /** + * The children elements. + */ + children?: React.ReactNode; /** * @deprecated */ @@ -70,6 +74,15 @@ export type FlexItemProps = { * @default true */ isBlock?: boolean; + /** + * The children elements. + */ + children?: React.ReactNode; }; -export type FlexBlockProps = Omit< FlexItemProps, 'isBlock' >; +export type FlexBlockProps = Omit< FlexItemProps, 'isBlock' > & { + /** + * The children elements. + */ + children?: React.ReactNode; +}; diff --git a/packages/components/src/grid/types.ts b/packages/components/src/grid/types.ts index b66529f5253c0a..266bd8b7a7e51b 100644 --- a/packages/components/src/grid/types.ts +++ b/packages/components/src/grid/types.ts @@ -72,4 +72,8 @@ export type Props = { * Adjusts the CSS grid `template-rows`. */ templateRows?: CSSProperties[ 'gridTemplateRows' ]; + /** + * The children elements. + */ + children?: React.ReactNode; }; diff --git a/packages/components/src/spacer/hook.ts b/packages/components/src/spacer/hook.ts index a40bbcc1de44a5..946f4a01398dda 100644 --- a/packages/components/src/spacer/hook.ts +++ b/packages/components/src/spacer/hook.ts @@ -73,6 +73,10 @@ export interface SpacerProps { * Adjusts right padding. */ paddingRight?: number; + /** + * The children elements. + */ + children?: React.ReactNode; } export function useSpacer( diff --git a/packages/components/src/text/types.ts b/packages/components/src/text/types.ts index 63a94694ee9980..b6dbdb9d31ffd6 100644 --- a/packages/components/src/text/types.ts +++ b/packages/components/src/text/types.ts @@ -103,4 +103,8 @@ export interface Props extends TruncateProps { * Letters or words within `Text` can be highlighted using `highlightWords`. */ highlightWords?: string[]; + /** + * The children elements. + */ + children?: React.ReactNode; } diff --git a/packages/components/src/truncate/types.ts b/packages/components/src/truncate/types.ts index 61ff73ad64c0de..e4d38ab0abfeec 100644 --- a/packages/components/src/truncate/types.ts +++ b/packages/components/src/truncate/types.ts @@ -25,7 +25,11 @@ export interface Props { */ limit?: number; /** - * Clamps the text content to the specifiec `numberOfLines`, adding the `ellipsis` at the end. + * Clamps the text content to the specified `numberOfLines`, adding the `ellipsis` at the end. */ numberOfLines?: number; + /** + * The children elements. + */ + children?: React.ReactNode; } diff --git a/packages/components/src/ui/card/types.ts b/packages/components/src/ui/card/types.ts index c934fed5277d77..bc602e5040e56d 100644 --- a/packages/components/src/ui/card/types.ts +++ b/packages/components/src/ui/card/types.ts @@ -51,6 +51,10 @@ export type CardBodyProps = { * @default true */ scrollable?: boolean; + /** + * The children elements. + */ + children?: React.ReactNode; }; export type CardHeaderSize = 'medium' | 'small' | 'xSmall'; @@ -62,6 +66,10 @@ export type CardHeaderProps = { * @default 'medium' */ size?: CardHeaderSize; + /** + * The children elements. + */ + children?: React.ReactNode; }; export type CardFooterProps = CardHeaderProps & { diff --git a/packages/components/src/ui/context/polymorphic-component.ts b/packages/components/src/ui/context/polymorphic-component.ts index 82c992acc1ef5f..96571287cbd409 100644 --- a/packages/components/src/ui/context/polymorphic-component.ts +++ b/packages/components/src/ui/context/polymorphic-component.ts @@ -3,16 +3,20 @@ */ // eslint-disable-next-line no-restricted-imports import type * as React from 'react'; -import type { As, RenderProp, ExtractHTMLAttributes } from 'reakit-utils/types'; +import type { + As, + // RenderProp, + // ExtractHTMLAttributes +} from 'reakit-utils/types'; import type { Interpolation } from 'create-emotion'; /** * Based on https://github.com/reakit/reakit/blob/master/packages/reakit-utils/src/types.ts */ export type PolymorphicComponentProps< P, T extends As > = P & - Omit< React.ComponentPropsWithRef< T >, 'as' | keyof P > & { + Omit< React.ComponentPropsWithRef< T >, 'as' | keyof P | 'children' > & { as?: T | keyof JSX.IntrinsicElements; - children?: React.ReactNode | RenderProp< ExtractHTMLAttributes< any > >; + // children?: React.ReactNode | RenderProp< ExtractHTMLAttributes< any > >; }; export type ElementTypeFromPolymorphicComponentProps< diff --git a/packages/components/src/ui/context/use-context-system.js b/packages/components/src/ui/context/use-context-system.js index 8501aa491e1070..8b2250fe485fc1 100644 --- a/packages/components/src/ui/context/use-context-system.js +++ b/packages/components/src/ui/context/use-context-system.js @@ -15,12 +15,10 @@ import { useComponentsContext } from './context-system-provider'; import { getNamespace, getConnectedNamespace } from './utils'; import { getStyledClassNameFromKey } from './get-styled-class-name-from-key'; -/* eslint-disable jsdoc/valid-types */ /** * @template TProps - * @typedef {TProps & { className: string; children?: import('react').ReactNode }} ConnectedProps + * @typedef {TProps & { className: string; }} ConnectedProps */ -/* eslint-enable jsdoc/valid-types */ /** * Custom hook that derives registered props from the Context system. diff --git a/packages/components/src/ui/control-group/types.ts b/packages/components/src/ui/control-group/types.ts index 60e02f0186ed2f..042f3432c19bce 100644 --- a/packages/components/src/ui/control-group/types.ts +++ b/packages/components/src/ui/control-group/types.ts @@ -23,4 +23,8 @@ export type Props = Pick< FlexProps, 'direction' > & { * Adjust the layout (width) of content using CSS grid (`grid-template-columns`). */ templateColumns?: CSSProperties[ 'gridTemplateColumns' ]; + /** + * The children elements. + */ + children?: React.ReactNode; }; diff --git a/packages/components/src/ui/form-group/types.ts b/packages/components/src/ui/form-group/types.ts index a02fa343b459f0..9ff3a2aa249307 100644 --- a/packages/components/src/ui/form-group/types.ts +++ b/packages/components/src/ui/form-group/types.ts @@ -13,6 +13,7 @@ import type { Props as GridProps } from '../../grid/types'; export type FormGroupLabelProps = ControlLabelProps & { labelHidden?: boolean; id?: ReactText; + children?: React.ReactNode; }; export type FormGroupContentProps = FormGroupLabelProps & { diff --git a/packages/components/src/ui/popover/types.ts b/packages/components/src/ui/popover/types.ts index 3f0e0402a4df00..1f8374eeacdaee 100644 --- a/packages/components/src/ui/popover/types.ts +++ b/packages/components/src/ui/popover/types.ts @@ -72,9 +72,14 @@ export type Props = PopperProps & { * @see https://reakit.io/docs/popover/#usepopoverstate */ visible?: boolean; + /** + * The children elements. + */ + children?: React.ReactNode; }; export type ContentProps = { elevation?: number; maxWidth?: CSSProperties[ 'maxWidth' ]; + children?: React.ReactNode; }; diff --git a/packages/components/src/ui/scrollable/types.ts b/packages/components/src/ui/scrollable/types.ts index 9b5d86db916bff..1d8195467b0ec0 100644 --- a/packages/components/src/ui/scrollable/types.ts +++ b/packages/components/src/ui/scrollable/types.ts @@ -13,4 +13,8 @@ export type Props = { * @default false */ smoothScroll?: boolean; + /** + * The children elements. + */ + children?: React.ReactNode; }; diff --git a/packages/components/src/ui/surface/types.ts b/packages/components/src/ui/surface/types.ts index 128fb1a2858dbe..92b4d5d16cbe1e 100644 --- a/packages/components/src/ui/surface/types.ts +++ b/packages/components/src/ui/surface/types.ts @@ -38,4 +38,8 @@ export type Props = { * * `tertiary`: Used as the app/site wide background. Visible in **dark mode** only. Use case is rare. */ variant?: SurfaceVariant; + /** + * The children elements. + */ + children?: React.ReactNode; }; diff --git a/packages/components/src/ui/visually-hidden/hook.js b/packages/components/src/ui/visually-hidden/hook.js index e8e7980c72dbec..ee0732bff1d86b 100644 --- a/packages/components/src/ui/visually-hidden/hook.js +++ b/packages/components/src/ui/visually-hidden/hook.js @@ -12,7 +12,7 @@ import * as styles from './styles'; /** @typedef {import('../context').PolymorphicComponentProps<{}, 'div'>} Props */ /** - * @param {import('../context').PolymorphicComponentProps<{}, 'div'>} props + * @param {import('../context').PolymorphicComponentProps<{ children?: import('react').ReactNode }, 'div'>} props */ export function useVisuallyHidden( { className, ...props } ) { // circumvent the context system and write the classnames ourselves From 10d49d3a8f8320536313722e141cee4f68ffdbf2 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 14 May 2021 09:26:05 +0200 Subject: [PATCH 02/12] Fix mispelled prop declaration --- packages/components/src/ui/control-group/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/ui/control-group/types.ts b/packages/components/src/ui/control-group/types.ts index 042f3432c19bce..cf4058c14e3be4 100644 --- a/packages/components/src/ui/control-group/types.ts +++ b/packages/components/src/ui/control-group/types.ts @@ -12,7 +12,7 @@ import type { FlexProps } from '../../flex/types'; export type ControlGroupContext = { isFirst?: boolean; isLast?: boolean; - isMidde?: boolean; + isMiddle?: boolean; isOnly?: boolean; isVertical?: boolean; styles?: string; From 525394ec054cc1dcb23db3d15c2bc8a371ca1e39 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 14 May 2021 09:26:21 +0200 Subject: [PATCH 03/12] Add pending TODO items --- packages/components/src/draggable/index.js | 2 +- packages/components/src/flex/types.ts | 11 +++-------- packages/components/src/grid/types.ts | 2 +- packages/components/src/text/types.ts | 4 ---- packages/components/src/truncate/types.ts | 2 +- packages/components/src/ui/card/types.ts | 4 ++-- packages/components/src/ui/control-group/types.ts | 2 +- packages/components/src/ui/form-group/types.ts | 1 - packages/components/src/ui/popover/types.ts | 5 +++-- packages/components/src/ui/scrollable/types.ts | 2 +- packages/components/src/ui/surface/types.ts | 2 +- packages/components/src/ui/tooltip/content.js | 3 ++- packages/components/src/ui/tooltip/types.ts | 10 +++++++--- 13 files changed, 23 insertions(+), 27 deletions(-) diff --git a/packages/components/src/draggable/index.js b/packages/components/src/draggable/index.js index e4a9b43059da83..b45624e040250c 100644 --- a/packages/components/src/draggable/index.js +++ b/packages/components/src/draggable/index.js @@ -17,7 +17,7 @@ const bodyClass = 'is-dragging-components-draggable'; /** * @typedef Props - * @property {(props: RenderProp) => JSX.Element | null} children Children. + * @property {(props: RenderProp) => JSX.Element | null} children Children. TODO: any action needed here? * @property {(event: import('react').DragEvent) => void} [onDragStart] Callback when dragging starts. * @property {(event: import('react').DragEvent) => void} [onDragOver] Callback when dragging happens over the document. * @property {(event: import('react').DragEvent) => void} [onDragEnd] Callback when dragging ends. diff --git a/packages/components/src/flex/types.ts b/packages/components/src/flex/types.ts index ea545fef77c6bf..b52b1e4d282bcf 100644 --- a/packages/components/src/flex/types.ts +++ b/packages/components/src/flex/types.ts @@ -56,7 +56,7 @@ export type FlexProps = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; /** * @deprecated */ @@ -77,12 +77,7 @@ export type FlexItemProps = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; -export type FlexBlockProps = Omit< FlexItemProps, 'isBlock' > & { - /** - * The children elements. - */ - children?: React.ReactNode; -}; +export type FlexBlockProps = Omit< FlexItemProps, 'isBlock' >; diff --git a/packages/components/src/grid/types.ts b/packages/components/src/grid/types.ts index 266bd8b7a7e51b..1830f85c6942cb 100644 --- a/packages/components/src/grid/types.ts +++ b/packages/components/src/grid/types.ts @@ -75,5 +75,5 @@ export type Props = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; diff --git a/packages/components/src/text/types.ts b/packages/components/src/text/types.ts index b6dbdb9d31ffd6..63a94694ee9980 100644 --- a/packages/components/src/text/types.ts +++ b/packages/components/src/text/types.ts @@ -103,8 +103,4 @@ export interface Props extends TruncateProps { * Letters or words within `Text` can be highlighted using `highlightWords`. */ highlightWords?: string[]; - /** - * The children elements. - */ - children?: React.ReactNode; } diff --git a/packages/components/src/truncate/types.ts b/packages/components/src/truncate/types.ts index e4d38ab0abfeec..74d07cd89a5608 100644 --- a/packages/components/src/truncate/types.ts +++ b/packages/components/src/truncate/types.ts @@ -31,5 +31,5 @@ export interface Props { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; } diff --git a/packages/components/src/ui/card/types.ts b/packages/components/src/ui/card/types.ts index bc602e5040e56d..b8794489bc266f 100644 --- a/packages/components/src/ui/card/types.ts +++ b/packages/components/src/ui/card/types.ts @@ -54,7 +54,7 @@ export type CardBodyProps = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; export type CardHeaderSize = 'medium' | 'small' | 'xSmall'; @@ -69,7 +69,7 @@ export type CardHeaderProps = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; export type CardFooterProps = CardHeaderProps & { diff --git a/packages/components/src/ui/control-group/types.ts b/packages/components/src/ui/control-group/types.ts index cf4058c14e3be4..d36592224c066a 100644 --- a/packages/components/src/ui/control-group/types.ts +++ b/packages/components/src/ui/control-group/types.ts @@ -26,5 +26,5 @@ export type Props = Pick< FlexProps, 'direction' > & { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; diff --git a/packages/components/src/ui/form-group/types.ts b/packages/components/src/ui/form-group/types.ts index 9ff3a2aa249307..a02fa343b459f0 100644 --- a/packages/components/src/ui/form-group/types.ts +++ b/packages/components/src/ui/form-group/types.ts @@ -13,7 +13,6 @@ import type { Props as GridProps } from '../../grid/types'; export type FormGroupLabelProps = ControlLabelProps & { labelHidden?: boolean; id?: ReactText; - children?: React.ReactNode; }; export type FormGroupContentProps = FormGroupLabelProps & { diff --git a/packages/components/src/ui/popover/types.ts b/packages/components/src/ui/popover/types.ts index 1f8374eeacdaee..153280934c0bc7 100644 --- a/packages/components/src/ui/popover/types.ts +++ b/packages/components/src/ui/popover/types.ts @@ -37,6 +37,7 @@ export type Props = PopperProps & { baseId?: string; /** * Content to render within the `Popover` floating label. + * [TODO: is this used at all?] */ content?: ReactNode; /** @@ -75,11 +76,11 @@ export type Props = PopperProps & { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; export type ContentProps = { elevation?: number; maxWidth?: CSSProperties[ 'maxWidth' ]; - children?: React.ReactNode; + children: React.ReactNode; }; diff --git a/packages/components/src/ui/scrollable/types.ts b/packages/components/src/ui/scrollable/types.ts index 1d8195467b0ec0..6e93a7b6d6880e 100644 --- a/packages/components/src/ui/scrollable/types.ts +++ b/packages/components/src/ui/scrollable/types.ts @@ -16,5 +16,5 @@ export type Props = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; diff --git a/packages/components/src/ui/surface/types.ts b/packages/components/src/ui/surface/types.ts index 92b4d5d16cbe1e..e00255581d69ab 100644 --- a/packages/components/src/ui/surface/types.ts +++ b/packages/components/src/ui/surface/types.ts @@ -41,5 +41,5 @@ export type Props = { /** * The children elements. */ - children?: React.ReactNode; + children: React.ReactNode; }; diff --git a/packages/components/src/ui/tooltip/content.js b/packages/components/src/ui/tooltip/content.js index 1fc0780eae468b..210648a6113282 100644 --- a/packages/components/src/ui/tooltip/content.js +++ b/packages/components/src/ui/tooltip/content.js @@ -17,10 +17,11 @@ const { TooltipPopoverView } = styles; /** * - * @param {import('../context').PolymorphicComponentProps} props + * @param {import('../context').PolymorphicComponentProps} props * @param {import('react').Ref} forwardedRef */ function TooltipContent( props, forwardedRef ) { + // TODO: what about here const { children, className, ...otherProps } = useContextSystem( props, 'TooltipContent' diff --git a/packages/components/src/ui/tooltip/types.ts b/packages/components/src/ui/tooltip/types.ts index 34c8add7b520aa..446afcdb298b8d 100644 --- a/packages/components/src/ui/tooltip/types.ts +++ b/packages/components/src/ui/tooltip/types.ts @@ -2,9 +2,9 @@ * External dependencies */ // eslint-disable-next-line no-restricted-imports -import type { TooltipInitialState } from 'reakit'; +import type { TooltipInitialState, TooltipProps } from 'reakit'; // eslint-disable-next-line no-restricted-imports -import type { ReactElement } from 'react'; +import type { ReactElement, ReactNode } from 'react'; /** * Internal dependencies @@ -59,6 +59,10 @@ export type Props = TooltipInitialState & * @see https://reakit.io/docs/tooltip/#usetooltipstate */ visible?: boolean; - children: JSX.Element; + children: ReactElement; focusable?: boolean; }; + +export type ContentProps = TooltipProps & { + children: ReactNode; +}; From ad92791e26d45c30045425bd1246d818f7fb1341 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 19 May 2021 18:09:12 +0200 Subject: [PATCH 04/12] Remove comments --- packages/components/src/ui/popover/types.ts | 1 - packages/components/src/ui/tooltip/content.js | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/components/src/ui/popover/types.ts b/packages/components/src/ui/popover/types.ts index 153280934c0bc7..e5ea81a5e5d3ae 100644 --- a/packages/components/src/ui/popover/types.ts +++ b/packages/components/src/ui/popover/types.ts @@ -37,7 +37,6 @@ export type Props = PopperProps & { baseId?: string; /** * Content to render within the `Popover` floating label. - * [TODO: is this used at all?] */ content?: ReactNode; /** diff --git a/packages/components/src/ui/tooltip/content.js b/packages/components/src/ui/tooltip/content.js index 210648a6113282..c022c7d52184b5 100644 --- a/packages/components/src/ui/tooltip/content.js +++ b/packages/components/src/ui/tooltip/content.js @@ -21,7 +21,6 @@ const { TooltipPopoverView } = styles; * @param {import('react').Ref} forwardedRef */ function TooltipContent( props, forwardedRef ) { - // TODO: what about here const { children, className, ...otherProps } = useContextSystem( props, 'TooltipContent' From d846dff344ef1ba03e91047087f67b3234cea25c Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 19 May 2021 18:35:13 +0200 Subject: [PATCH 05/12] Clean up code around removing children props from root component types --- .../components/src/ui/context/polymorphic-component.ts | 7 +------ packages/components/src/ui/context/use-context-system.js | 1 + 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/components/src/ui/context/polymorphic-component.ts b/packages/components/src/ui/context/polymorphic-component.ts index 96571287cbd409..efebcba74cae1b 100644 --- a/packages/components/src/ui/context/polymorphic-component.ts +++ b/packages/components/src/ui/context/polymorphic-component.ts @@ -3,11 +3,7 @@ */ // eslint-disable-next-line no-restricted-imports import type * as React from 'react'; -import type { - As, - // RenderProp, - // ExtractHTMLAttributes -} from 'reakit-utils/types'; +import type { As } from 'reakit-utils/types'; import type { Interpolation } from 'create-emotion'; /** @@ -16,7 +12,6 @@ import type { Interpolation } from 'create-emotion'; export type PolymorphicComponentProps< P, T extends As > = P & Omit< React.ComponentPropsWithRef< T >, 'as' | keyof P | 'children' > & { as?: T | keyof JSX.IntrinsicElements; - // children?: React.ReactNode | RenderProp< ExtractHTMLAttributes< any > >; }; export type ElementTypeFromPolymorphicComponentProps< diff --git a/packages/components/src/ui/context/use-context-system.js b/packages/components/src/ui/context/use-context-system.js index 8b2250fe485fc1..06acef66d1ee59 100644 --- a/packages/components/src/ui/context/use-context-system.js +++ b/packages/components/src/ui/context/use-context-system.js @@ -73,6 +73,7 @@ export function useContextSystem( props, namespace ) { finalComponentProps[ key ] = overrideProps[ key ]; } + // @ts-ignore finalComponentProps.children = rendered; finalComponentProps.className = classes; From 4c5b1e6350ad86963aa9069ca05e66075c85d2ae Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 19 May 2021 18:35:38 +0200 Subject: [PATCH 06/12] Disallow Divider component from having children --- packages/components/src/divider/component.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/divider/component.tsx b/packages/components/src/divider/component.tsx index 2ea05e62d63f4d..614e2a4b251b23 100644 --- a/packages/components/src/divider/component.tsx +++ b/packages/components/src/divider/component.tsx @@ -23,7 +23,7 @@ import type { PolymorphicComponentProps } from '../ui/context'; import * as styles from './styles'; import { space } from '../ui/utils/space'; -export interface DividerProps extends SeparatorProps { +export interface DividerProps extends Omit< SeparatorProps, 'children' > { /** * Adjusts all margins. */ From bff9b9de74a002aa2ec0cc27c0e43283aecae84a Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 20 May 2021 12:57:49 +0200 Subject: [PATCH 07/12] Remove TODO comment --- packages/components/src/draggable/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/draggable/index.js b/packages/components/src/draggable/index.js index b45624e040250c..e4a9b43059da83 100644 --- a/packages/components/src/draggable/index.js +++ b/packages/components/src/draggable/index.js @@ -17,7 +17,7 @@ const bodyClass = 'is-dragging-components-draggable'; /** * @typedef Props - * @property {(props: RenderProp) => JSX.Element | null} children Children. TODO: any action needed here? + * @property {(props: RenderProp) => JSX.Element | null} children Children. * @property {(event: import('react').DragEvent) => void} [onDragStart] Callback when dragging starts. * @property {(event: import('react').DragEvent) => void} [onDragOver] Callback when dragging happens over the document. * @property {(event: import('react').DragEvent) => void} [onDragEnd] Callback when dragging ends. From 1c9f4d3f9db98bf49fb1c7b1bb4abe62b65bc2e1 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 20 May 2021 13:02:40 +0200 Subject: [PATCH 08/12] Add explanation about `children` prop omission in ViewOwnProps --- packages/components/src/ui/context/polymorphic-component.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/src/ui/context/polymorphic-component.ts b/packages/components/src/ui/context/polymorphic-component.ts index efebcba74cae1b..62b01376e6a9a3 100644 --- a/packages/components/src/ui/context/polymorphic-component.ts +++ b/packages/components/src/ui/context/polymorphic-component.ts @@ -8,6 +8,10 @@ import type { Interpolation } from 'create-emotion'; /** * Based on https://github.com/reakit/reakit/blob/master/packages/reakit-utils/src/types.ts + * + * The `children` prop is being explicitely omitted since it is otherwise implicitly added + * by `ComponentPropsWithRef`. The context is that components should require the `children` + * prop explicitely when needed (see https://github.com/WordPress/gutenberg/pull/31817). */ export type PolymorphicComponentProps< P, T extends As > = P & Omit< React.ComponentPropsWithRef< T >, 'as' | keyof P | 'children' > & { From c742d8ad4c40f0f095a40e67f7049bd591018743 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 21 May 2021 17:15:47 +0200 Subject: [PATCH 09/12] Use `React.ElementType` instead of `As` from `reakit-utils` --- .../src/ui/context/polymorphic-component.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/components/src/ui/context/polymorphic-component.ts b/packages/components/src/ui/context/polymorphic-component.ts index 62b01376e6a9a3..488f3a6dc40f93 100644 --- a/packages/components/src/ui/context/polymorphic-component.ts +++ b/packages/components/src/ui/context/polymorphic-component.ts @@ -3,7 +3,6 @@ */ // eslint-disable-next-line no-restricted-imports import type * as React from 'react'; -import type { As } from 'reakit-utils/types'; import type { Interpolation } from 'create-emotion'; /** @@ -13,7 +12,7 @@ import type { Interpolation } from 'create-emotion'; * by `ComponentPropsWithRef`. The context is that components should require the `children` * prop explicitely when needed (see https://github.com/WordPress/gutenberg/pull/31817). */ -export type PolymorphicComponentProps< P, T extends As > = P & +export type PolymorphicComponentProps< P, T extends React.ElementType > = P & Omit< React.ComponentPropsWithRef< T >, 'as' | keyof P | 'children' > & { as?: T | keyof JSX.IntrinsicElements; }; @@ -26,8 +25,8 @@ export type PropsFromPolymorphicComponentProps< P > = P extends PolymorphicComponentProps< infer PP, any > ? PP : never; -export type PolymorphicComponent< T extends As, O > = { - < TT extends As >( +export type PolymorphicComponent< T extends React.ElementType, O > = { + < TT extends React.ElementType >( props: PolymorphicComponentProps< O, TT > & { as: TT } ): JSX.Element | null; ( props: PolymorphicComponentProps< O, T > ): JSX.Element | null; @@ -43,7 +42,7 @@ export type PolymorphicComponent< T extends As, O > = { selector: `.${ string }`; }; -export type CreatePolymorphicComponent< T extends As, P > = ( +export type CreatePolymorphicComponent< T extends React.ElementType, P > = ( template: TemplateStringsArray, ...styles: ( | Interpolation< undefined > @@ -65,5 +64,8 @@ type CreateStyledComponents = { }; export type CreateStyled = CreateStyledComponents & { - < T extends As >( component: T ): CreatePolymorphicComponent< T, {} >; + < T extends React.ElementType >( component: T ): CreatePolymorphicComponent< + T, + {} + >; }; From dcfb4d2180ab384a9145f44f72aead33a9f06dc4 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 24 May 2021 17:52:47 +0200 Subject: [PATCH 10/12] Add `children` prop to View --- packages/components/src/view/component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/view/component.js b/packages/components/src/view/component.js index ed106cab0f624e..571361e070c925 100644 --- a/packages/components/src/view/component.js +++ b/packages/components/src/view/component.js @@ -20,7 +20,7 @@ import styled from '@emotion/styled'; * } * ``` * - * @type {import('../ui/context').PolymorphicComponent<'div', {}>} + * @type {import('../ui/context').PolymorphicComponent<'div', { children?: import('react').ReactNode }>} */ // @ts-ignore const View = styled.div``; From 0cd9eb6609bd7c6425c187554643d905a47b7518 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 24 May 2021 17:54:50 +0200 Subject: [PATCH 11/12] Do not use `ComponentType` as it pulls in optional `children` prop implicitly --- packages/components/src/ui/context/with-next.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/ui/context/with-next.js b/packages/components/src/ui/context/with-next.js index 8b3ba55e29b3fa..1351869b5f56ce 100644 --- a/packages/components/src/ui/context/with-next.js +++ b/packages/components/src/ui/context/with-next.js @@ -8,7 +8,7 @@ import { useContextSystem } from './use-context-system'; * @template {{}} TCurrentProps * @template {{}} TNextProps * @param {import('react').ForwardRefExoticComponent} CurrentComponent - * @param {import('react').ComponentType} NextComponent + * @param {(props: TNextProps) => JSX.Element | null} NextComponent * @param {string} namespace * @param {(props: TCurrentProps) => TNextProps} adapter */ From a2f5ba733911ea680a3b27a783aed468b53b8ce0 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 24 May 2021 17:55:16 +0200 Subject: [PATCH 12/12] Mark `children` prop in as required --- packages/components/src/ui/visually-hidden/hook.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/ui/visually-hidden/hook.js b/packages/components/src/ui/visually-hidden/hook.js index ee0732bff1d86b..374c8c894fecbd 100644 --- a/packages/components/src/ui/visually-hidden/hook.js +++ b/packages/components/src/ui/visually-hidden/hook.js @@ -9,10 +9,10 @@ import { cx } from 'emotion'; import * as styles from './styles'; // duplicate this for the sake of being able to export it, it'll be removed when we replace VisuallyHidden in components/src anyway -/** @typedef {import('../context').PolymorphicComponentProps<{}, 'div'>} Props */ +/** @typedef {import('../context').PolymorphicComponentProps<{ children: import('react').ReactNode }, 'div'>} Props */ /** - * @param {import('../context').PolymorphicComponentProps<{ children?: import('react').ReactNode }, 'div'>} props + * @param {import('../context').PolymorphicComponentProps<{ children: import('react').ReactNode }, 'div'>} props */ export function useVisuallyHidden( { className, ...props } ) { // circumvent the context system and write the classnames ourselves