From 841a148992381aceb3a87d232c24d073af432a78 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Thu, 25 Aug 2022 20:16:11 +0900 Subject: [PATCH 1/9] WordPressComponentProps: Add option to exclude ref --- .../src/ui/context/wordpress-component.ts | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/components/src/ui/context/wordpress-component.ts b/packages/components/src/ui/context/wordpress-component.ts index 1cf549c76f5c49..2eac810434a12b 100644 --- a/packages/components/src/ui/context/wordpress-component.ts +++ b/packages/components/src/ui/context/wordpress-component.ts @@ -3,19 +3,26 @@ */ import type * as React from 'react'; -/** - * 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). - */ +// Based on https://github.com/reakit/reakit/blob/master/packages/reakit-utils/src/types.ts export type WordPressComponentProps< + /** Prop types. */ P, + /** The HTML element to inherit props from. */ T extends React.ElementType, - IsPolymorphic extends boolean = true + /** Supports polymorphism through the `as` prop. */ + IsPolymorphic extends boolean = true, + /** Supports ref forwarding. */ + ForwardsRef extends boolean = true > = P & - Omit< React.ComponentPropsWithRef< T >, 'as' | keyof P | 'children' > & + // The `children` prop is being explicitly 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). + Omit< + ForwardsRef extends true + ? React.ComponentPropsWithRef< T > + : React.ComponentPropsWithoutRef< T >, + 'as' | keyof P | 'children' + > & ( IsPolymorphic extends true ? { /** The HTML element or React component to render the component as. */ From 014376460da67812307ad50d01a173a6e444e7f9 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Thu, 25 Aug 2022 20:19:45 +0900 Subject: [PATCH 2/9] Update other types --- .../components/src/ui/context/wordpress-component.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/components/src/ui/context/wordpress-component.ts b/packages/components/src/ui/context/wordpress-component.ts index 2eac810434a12b..09ec4331c85e4e 100644 --- a/packages/components/src/ui/context/wordpress-component.ts +++ b/packages/components/src/ui/context/wordpress-component.ts @@ -33,14 +33,15 @@ export type WordPressComponentProps< export type WordPressComponent< T extends React.ElementType, O, - IsPolymorphic extends boolean + IsPolymorphic extends boolean = true, + ForwardsRef extends boolean = true > = { < TT extends React.ElementType >( - props: WordPressComponentProps< O, TT, IsPolymorphic > & + props: WordPressComponentProps< O, TT, IsPolymorphic, ForwardsRef > & ( IsPolymorphic extends true ? { as: TT } : {} ) ): JSX.Element | null; ( - props: WordPressComponentProps< O, T, IsPolymorphic > + props: WordPressComponentProps< O, T, IsPolymorphic, ForwardsRef > ): JSX.Element | null; displayName?: string; /** @@ -55,6 +56,6 @@ export type WordPressComponent< }; export type WordPressComponentFromProps< Props > = - Props extends WordPressComponentProps< infer P, infer T, infer I > - ? WordPressComponent< T, P, I > + Props extends WordPressComponentProps< infer P, infer T, infer I, infer F > + ? WordPressComponent< T, P, I, F > : never; From d815601451acc6b23b6da62aa5efd2dd787d4ae1 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Thu, 25 Aug 2022 20:22:06 +0900 Subject: [PATCH 3/9] Remove refs from omitted components --- packages/components/src/checkbox-control/index.tsx | 10 +++++----- packages/components/src/disabled/index.tsx | 2 +- packages/components/src/form-toggle/index.tsx | 7 +------ packages/components/src/placeholder/index.tsx | 10 +++++----- packages/components/src/radio-control/index.tsx | 7 +------ packages/components/src/textarea-control/index.tsx | 10 +++++----- 6 files changed, 18 insertions(+), 28 deletions(-) diff --git a/packages/components/src/checkbox-control/index.tsx b/packages/components/src/checkbox-control/index.tsx index fd6f27551fea4e..f6a726dde9a793 100644 --- a/packages/components/src/checkbox-control/index.tsx +++ b/packages/components/src/checkbox-control/index.tsx @@ -40,11 +40,11 @@ import type { WordPressComponentProps } from '../ui/context'; * ``` */ export function CheckboxControl( - // ref is omitted until we have `WordPressComponentPropsWithoutRef` or add - // ref forwarding to CheckboxControl. - props: Omit< - WordPressComponentProps< CheckboxControlProps, 'input', false >, - 'ref' + props: WordPressComponentProps< + CheckboxControlProps, + 'input', + false, + false > ) { const { diff --git a/packages/components/src/disabled/index.tsx b/packages/components/src/disabled/index.tsx index 10618c19de93f2..43db5c525556a1 100644 --- a/packages/components/src/disabled/index.tsx +++ b/packages/components/src/disabled/index.tsx @@ -54,7 +54,7 @@ function Disabled( { children, isDisabled = true, ...props -}: Omit< WordPressComponentProps< DisabledProps, 'div' >, 'ref' > ) { +}: WordPressComponentProps< DisabledProps, 'div', true, false > ) { const ref = useDisabled(); if ( ! isDisabled ) { diff --git a/packages/components/src/form-toggle/index.tsx b/packages/components/src/form-toggle/index.tsx index d51a3ee571934d..d7d73f939db4a8 100644 --- a/packages/components/src/form-toggle/index.tsx +++ b/packages/components/src/form-toggle/index.tsx @@ -31,12 +31,7 @@ export const noop = () => {}; * ``` */ export function FormToggle( - // ref is omitted until we have `WordPressComponentPropsWithoutRef` or add - // ref forwarding to FormToggle. - props: Omit< - WordPressComponentProps< FormToggleProps, 'input', false >, - 'ref' - > + props: WordPressComponentProps< FormToggleProps, 'input', false, false > ) { const { className, diff --git a/packages/components/src/placeholder/index.tsx b/packages/components/src/placeholder/index.tsx index cc9f33fe4f6032..6cbf0025c68e7e 100644 --- a/packages/components/src/placeholder/index.tsx +++ b/packages/components/src/placeholder/index.tsx @@ -39,11 +39,11 @@ const PlaceholderIllustration = ( * ``` */ export function Placeholder< IconProps = unknown >( - // ref is omitted until we have `WordPressComponentPropsWithoutRef` or add - // ref forwarding to Placeholder. - props: Omit< - WordPressComponentProps< PlaceholderProps< IconProps >, 'div', false >, - 'ref' + props: WordPressComponentProps< + PlaceholderProps< IconProps >, + 'div', + false, + false > ) { const { diff --git a/packages/components/src/radio-control/index.tsx b/packages/components/src/radio-control/index.tsx index 6b4c1df8d30e0c..a40419bc4d0fa7 100644 --- a/packages/components/src/radio-control/index.tsx +++ b/packages/components/src/radio-control/index.tsx @@ -42,12 +42,7 @@ import type { RadioControlProps } from './types'; * ``` */ export function RadioControl( - // ref is omitted until we have `WordPressComponentPropsWithoutRef` or add - // ref forwarding to RadioControl. - props: Omit< - WordPressComponentProps< RadioControlProps, 'input', false >, - 'ref' - > + props: WordPressComponentProps< RadioControlProps, 'input', false, false > ) { const { label, diff --git a/packages/components/src/textarea-control/index.tsx b/packages/components/src/textarea-control/index.tsx index 3ce656af2ad7cc..44578deeb7ebf3 100644 --- a/packages/components/src/textarea-control/index.tsx +++ b/packages/components/src/textarea-control/index.tsx @@ -40,11 +40,11 @@ import type { WordPressComponentProps } from '../ui/context'; * ``` */ export function TextareaControl( - // ref is omitted until we have `WordPressComponentPropsWithoutRef` or add - // ref forwarding to TextareaControl. - props: Omit< - WordPressComponentProps< TextareaControlProps, 'textarea', false >, - 'ref' + props: WordPressComponentProps< + TextareaControlProps, + 'textarea', + false, + false > ) { const { From ce0d2d2d2a77e86cae8278f72c61fbf871f9785a Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 26 Aug 2022 22:00:28 +0900 Subject: [PATCH 4/9] Revert fourth parameter --- packages/components/src/checkbox-control/index.tsx | 7 +------ packages/components/src/disabled/index.tsx | 2 +- packages/components/src/form-toggle/index.tsx | 2 +- packages/components/src/placeholder/index.tsx | 1 - packages/components/src/radio-control/index.tsx | 2 +- packages/components/src/textarea-control/index.tsx | 7 +------ 6 files changed, 5 insertions(+), 16 deletions(-) diff --git a/packages/components/src/checkbox-control/index.tsx b/packages/components/src/checkbox-control/index.tsx index f6a726dde9a793..3ea0d7e0e7e777 100644 --- a/packages/components/src/checkbox-control/index.tsx +++ b/packages/components/src/checkbox-control/index.tsx @@ -40,12 +40,7 @@ import type { WordPressComponentProps } from '../ui/context'; * ``` */ export function CheckboxControl( - props: WordPressComponentProps< - CheckboxControlProps, - 'input', - false, - false - > + props: WordPressComponentProps< CheckboxControlProps, 'input', false > ) { const { label, diff --git a/packages/components/src/disabled/index.tsx b/packages/components/src/disabled/index.tsx index 43db5c525556a1..9b8fdd230fb206 100644 --- a/packages/components/src/disabled/index.tsx +++ b/packages/components/src/disabled/index.tsx @@ -54,7 +54,7 @@ function Disabled( { children, isDisabled = true, ...props -}: WordPressComponentProps< DisabledProps, 'div', true, false > ) { +}: WordPressComponentProps< DisabledProps, 'div' > ) { const ref = useDisabled(); if ( ! isDisabled ) { diff --git a/packages/components/src/form-toggle/index.tsx b/packages/components/src/form-toggle/index.tsx index d7d73f939db4a8..e7ee1ae0bebdee 100644 --- a/packages/components/src/form-toggle/index.tsx +++ b/packages/components/src/form-toggle/index.tsx @@ -31,7 +31,7 @@ export const noop = () => {}; * ``` */ export function FormToggle( - props: WordPressComponentProps< FormToggleProps, 'input', false, false > + props: WordPressComponentProps< FormToggleProps, 'input', false > ) { const { className, diff --git a/packages/components/src/placeholder/index.tsx b/packages/components/src/placeholder/index.tsx index 6cbf0025c68e7e..fe22bb5a3df0e5 100644 --- a/packages/components/src/placeholder/index.tsx +++ b/packages/components/src/placeholder/index.tsx @@ -42,7 +42,6 @@ export function Placeholder< IconProps = unknown >( props: WordPressComponentProps< PlaceholderProps< IconProps >, 'div', - false, false > ) { diff --git a/packages/components/src/radio-control/index.tsx b/packages/components/src/radio-control/index.tsx index a40419bc4d0fa7..f6b5fc1a1e2018 100644 --- a/packages/components/src/radio-control/index.tsx +++ b/packages/components/src/radio-control/index.tsx @@ -42,7 +42,7 @@ import type { RadioControlProps } from './types'; * ``` */ export function RadioControl( - props: WordPressComponentProps< RadioControlProps, 'input', false, false > + props: WordPressComponentProps< RadioControlProps, 'input', false > ) { const { label, diff --git a/packages/components/src/textarea-control/index.tsx b/packages/components/src/textarea-control/index.tsx index 44578deeb7ebf3..10b79f8188709e 100644 --- a/packages/components/src/textarea-control/index.tsx +++ b/packages/components/src/textarea-control/index.tsx @@ -40,12 +40,7 @@ import type { WordPressComponentProps } from '../ui/context'; * ``` */ export function TextareaControl( - props: WordPressComponentProps< - TextareaControlProps, - 'textarea', - false, - false - > + props: WordPressComponentProps< TextareaControlProps, 'textarea', false > ) { const { label, From 6cea0ec70ff2d94ee75d33d281197def0eaaf7cd Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 26 Aug 2022 22:05:57 +0900 Subject: [PATCH 5/9] View: Fixup for type verification --- .../src/view/{component.js => component.tsx} | 17 +++++++--- .../components/src/view/stories/index.tsx | 32 +++++++++++++++++++ packages/components/src/view/types.ts | 6 ++++ 3 files changed, 51 insertions(+), 4 deletions(-) rename packages/components/src/view/{component.js => component.tsx} (59%) create mode 100644 packages/components/src/view/stories/index.tsx create mode 100644 packages/components/src/view/types.ts diff --git a/packages/components/src/view/component.js b/packages/components/src/view/component.tsx similarity index 59% rename from packages/components/src/view/component.js rename to packages/components/src/view/component.tsx index 4d7808807bd1f3..02f79a2673d0e4 100644 --- a/packages/components/src/view/component.js +++ b/packages/components/src/view/component.tsx @@ -2,6 +2,13 @@ * External dependencies */ import styled from '@emotion/styled'; +import type { RefAttributes } from 'react'; + +/** + * Internal dependencies + */ +import type { WordPressComponent } from '../ui/context/wordpress-component'; +import type { ViewProps } from './types'; /** * `View` is a core component that renders everything in the library. @@ -19,11 +26,13 @@ import styled from '@emotion/styled'; * ); * } * ``` - * - * @type {import('../ui/context').WordPressComponent<'div', { children?: import('react').ReactNode }, true>} */ -// @ts-ignore -const View = styled.div``; +// @ts-expect-error +export const View: WordPressComponent< + 'div', + ViewProps & RefAttributes< any > +> = styled.div``; + View.selector = '.components-view'; View.displayName = 'View'; diff --git a/packages/components/src/view/stories/index.tsx b/packages/components/src/view/stories/index.tsx new file mode 100644 index 00000000000000..fb27e29433470a --- /dev/null +++ b/packages/components/src/view/stories/index.tsx @@ -0,0 +1,32 @@ +/** + * External dependencies + */ +import type { ComponentMeta, ComponentStory } from '@storybook/react'; + +/** + * Internal dependencies + */ +import { View } from '..'; + +const meta: ComponentMeta< typeof View > = { + component: View, + title: 'Components (Experimental)/View', + argTypes: { + as: { control: { type: null } }, + children: { control: { type: 'text' } }, + }, + parameters: { + controls: { expanded: true }, + docs: { source: { state: 'open' } }, + }, +}; +export default meta; + +const Template: ComponentStory< typeof View > = ( args ) => { + return ; +}; + +export const Default: ComponentStory< typeof View > = Template.bind( {} ); +Default.args = { + children: 'An example tip', +}; diff --git a/packages/components/src/view/types.ts b/packages/components/src/view/types.ts new file mode 100644 index 00000000000000..6c7e1508812c8c --- /dev/null +++ b/packages/components/src/view/types.ts @@ -0,0 +1,6 @@ +/** + * External dependencies + */ +import type { ReactNode } from 'react'; + +export type ViewProps = { children?: ReactNode }; From f5339b676f9c05a209f258ca1313d5d87a612cfb Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 26 Aug 2022 22:06:25 +0900 Subject: [PATCH 6/9] Simplify --- .../src/ui/context/wordpress-component.ts | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/packages/components/src/ui/context/wordpress-component.ts b/packages/components/src/ui/context/wordpress-component.ts index 09ec4331c85e4e..5c0c60978f88b1 100644 --- a/packages/components/src/ui/context/wordpress-component.ts +++ b/packages/components/src/ui/context/wordpress-component.ts @@ -10,19 +10,12 @@ export type WordPressComponentProps< /** The HTML element to inherit props from. */ T extends React.ElementType, /** Supports polymorphism through the `as` prop. */ - IsPolymorphic extends boolean = true, - /** Supports ref forwarding. */ - ForwardsRef extends boolean = true + IsPolymorphic extends boolean = true > = P & // The `children` prop is being explicitly 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). - Omit< - ForwardsRef extends true - ? React.ComponentPropsWithRef< T > - : React.ComponentPropsWithoutRef< T >, - 'as' | keyof P | 'children' - > & + Omit< React.ComponentPropsWithoutRef< T >, 'as' | keyof P | 'children' > & ( IsPolymorphic extends true ? { /** The HTML element or React component to render the component as. */ @@ -33,15 +26,14 @@ export type WordPressComponentProps< export type WordPressComponent< T extends React.ElementType, O, - IsPolymorphic extends boolean = true, - ForwardsRef extends boolean = true + IsPolymorphic extends boolean > = { < TT extends React.ElementType >( - props: WordPressComponentProps< O, TT, IsPolymorphic, ForwardsRef > & + props: WordPressComponentProps< O, TT, IsPolymorphic > & ( IsPolymorphic extends true ? { as: TT } : {} ) ): JSX.Element | null; ( - props: WordPressComponentProps< O, T, IsPolymorphic, ForwardsRef > + props: WordPressComponentProps< O, T, IsPolymorphic > ): JSX.Element | null; displayName?: string; /** @@ -55,7 +47,13 @@ export type WordPressComponent< selector: `.${ string }`; }; -export type WordPressComponentFromProps< Props > = - Props extends WordPressComponentProps< infer P, infer T, infer I, infer F > - ? WordPressComponent< T, P, I, F > - : never; +export type WordPressComponentFromProps< + Props, + ForwardsRef extends boolean = true +> = Props extends WordPressComponentProps< infer P, infer T, infer I > + ? WordPressComponent< + T, + P & ( ForwardsRef extends true ? React.RefAttributes< any > : {} ), + I + > + : never; From 251fd9d1a38239791e42c8a4ead291d1a05efd06 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 26 Aug 2022 22:40:27 +0900 Subject: [PATCH 7/9] Add TS tests --- .../ui/context/test/wordpress-component.tsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 packages/components/src/ui/context/test/wordpress-component.tsx diff --git a/packages/components/src/ui/context/test/wordpress-component.tsx b/packages/components/src/ui/context/test/wordpress-component.tsx new file mode 100644 index 00000000000000..e648dd78fc9c75 --- /dev/null +++ b/packages/components/src/ui/context/test/wordpress-component.tsx @@ -0,0 +1,36 @@ +/** + * External dependencies + */ +import type { ForwardedRef } from 'react'; + +/** + * WordPress dependencies + */ +import { forwardRef } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import type { WordPressComponentProps } from '../wordpress-component'; + +// Static TypeScript checks +describe( 'WordPressComponentProps', () => { + it( 'should not accept a ref', () => { + const Foo = ( props: WordPressComponentProps< {}, 'div' > ) => ( +
+ ); + + // @ts-expect-error The ref prop should trigger an error. + ; + } ); + + it( 'should accept a ref if wrapped by a forwardRef()', () => { + const Foo = ( + props: WordPressComponentProps< {}, 'div' >, + ref: ForwardedRef< any > + ) =>
; + const ForwardedFoo = forwardRef( Foo ); + + ; + } ); +} ); From 0eaaab860b4b182ae50c78e6caab08d37385cc16 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 26 Aug 2022 23:05:05 +0900 Subject: [PATCH 8/9] Update snapshots --- .../card/test/__snapshots__/index.tsx.snap | 70 +++++++++---------- .../flex/test/__snapshots__/index.tsx.snap | 8 +-- .../test/__snapshots__/index.js.snap | 32 ++++----- .../surface/test/__snapshots__/index.tsx.snap | 20 +++--- .../test/__snapshots__/index.tsx.snap | 6 +- 5 files changed, 68 insertions(+), 68 deletions(-) diff --git a/packages/components/src/card/test/__snapshots__/index.tsx.snap b/packages/components/src/card/test/__snapshots__/index.tsx.snap index 10083db0505176..8004af489ee7ce 100644 --- a/packages/components/src/card/test/__snapshots__/index.tsx.snap +++ b/packages/components/src/card/test/__snapshots__/index.tsx.snap @@ -8,8 +8,8 @@ Snapshot Diff: @@ -1,8 +1,8 @@
@@ -25,8 +25,8 @@ Snapshot Diff: @@ -1,8 +1,8 @@
@@ -42,8 +42,8 @@ Snapshot Diff: @@ -1,8 +1,8 @@
@@ -59,8 +59,8 @@ Snapshot Diff: @@ -1,8 +1,8 @@
@@ -76,8 +76,8 @@ Snapshot Diff: @@ -1,8 +1,8 @@
@@ -93,19 +93,19 @@ Snapshot Diff: @@ -5,18 +5,18 @@ >
Header
@@ -138,27 +138,27 @@ Snapshot Diff: @@ -6,25 +6,25 @@ >
Header
Body
@@ -174,33 +174,33 @@ Snapshot Diff: @@ -1,30 +1,30 @@
Header
Body
@@ -786,15 +786,15 @@ Snapshot Diff: