Skip to content

Commit

Permalink
WordPressComponentProps: Only add refs when they are actually added (#…
Browse files Browse the repository at this point in the history
…43610)

* WordPressComponentProps: Add option to exclude ref

* Update other types

* Remove refs from omitted components

* Revert fourth parameter

* View: Fixup for type verification

* Simplify

* Add TS tests

* Update snapshots

* Remove ref-forwarding in TS migration guide
  • Loading branch information
mirka authored Aug 31, 2022
1 parent a805b47 commit 3c679aa
Show file tree
Hide file tree
Showing 17 changed files with 185 additions and 168 deletions.
57 changes: 3 additions & 54 deletions packages/components/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -531,58 +531,7 @@ Given a component folder (e.g. `packages/components/src/unit-control`):
) { /* ... */ }
```

7. If the component doesn't forwards its ref yet, wrap the component in a `forwardRed` call. Alternatively, if you want to take advantage of the [Context system](#context-system), you can use the `contextConnect` utility function (which also takes care of adding ref forwarding)

```tsx
// With `forwardRef`
import type { ForwardedRef } from 'react';
import { forwardRef } from '@wordpress/element';
import type { WordPressComponentProps } from '../ui/context';
import type { ComponentOwnProps } from './types';

function UnforwardedMyComponent(
props: WordPressComponentProps< ComponentOwnProps, 'div', true >,
forwardedRef: ForwardedRef< any >
) { /* ... */ }


/**
* MyComponent JSDoc
*/
export const MyComponent = forwardRef( UnforwardedMyComponent );

export default MyComponent;
```

```tsx
// With `contextConnect`
import type { ForwardedRef } from 'react';
import {
contextConnect,
useContextSystem,
WordPressComponentProps,
} from '../ui/context';
import type { ComponentOwnProps } from './types';

function UnconnectedMyComponent(
props: WordPressComponentProps< ComponentOwnProps, 'div', true >,
forwardedRef: ForwardedRef< any >
) {
const contextProps = useContextSystem( props, 'MyComponent' );

/* ... */
}


/**
* MyComponent JSDoc
*/
export const MyComponent = contextConnect( UnconnectedMyComponent, 'MyComponent' );

export default MyComponent;
```

8. As shown in the previous examples, make sure you have a **named** export for the component, not just the default export ([example](https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/divider/component.tsx)). This ensures that the docgen can properly extract the types data. The naming should be so that the connected/forwarded component has the plain component name (`MyComponent`), and the raw component is prefixed (`UnconnectedMyComponent` or `UnforwardedMyComponent`). This makes the component's `displayName` look nicer in React devtools and in the autogenerated Storybook code snippets.
7. As shown in the previous examples, make sure you have a **named** export for the component, not just the default export ([example](https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/divider/component.tsx)). This ensures that the docgen can properly extract the types data. The naming should be so that the connected/forwarded component has the plain component name (`MyComponent`), and the raw component is prefixed (`UnconnectedMyComponent` or `UnforwardedMyComponent`). This makes the component's `displayName` look nicer in React devtools and in the autogenerated Storybook code snippets.

```jsx
function UnconnectedMyComponent() { /* ... */ }
Expand All @@ -592,8 +541,8 @@ Given a component folder (e.g. `packages/components/src/unit-control`):
export default MyComponent;
```

9. Use JSDocs syntax for each TypeScript property that is part of the public API of a component. The docs used here should be aligned with the component’s README. Add `@default` values where appropriate.
10. Prefer `unknown` to `any`, and in general avoid it when possible.
8. Use JSDocs syntax for each TypeScript property that is part of the public API of a component. The docs used here should be aligned with the component’s README. Add `@default` values where appropriate.
9. Prefer `unknown` to `any`, and in general avoid it when possible.
8. On the component's main named export, add a JSDoc comment that includes the main description and the example code snippet from the README ([example](https://github.com/WordPress/gutenberg/blob/43d9c82922619c1d1ff6b454f86f75c3157d3de6/packages/components/src/date-time/date-time/index.tsx#L193-L217)). _At the time of writing, the `@example` JSDoc keyword is not recognized by StoryBook's docgen, so please avoid using it_.
9. Make sure that:
1. tests still pass;
Expand Down
70 changes: 35 additions & 35 deletions packages/components/src/card/test/__snapshots__/index.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ Snapshot Diff:
@@ -1,8 +1,8 @@
<div>
<div
- class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium em57xhy0"
+ class="components-scrollable components-card__body components-card-body css-1w878rm-View-Scrollable-scrollableScrollbar-scrollY-Body-borderRadius-medium em57xhy0"
- class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium e19lxcc00"
+ class="components-scrollable components-card__body components-card-body css-1w878rm-View-Scrollable-scrollableScrollbar-scrollY-Body-borderRadius-medium e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardBody"
>
Expand All @@ -25,8 +25,8 @@ Snapshot Diff:
@@ -1,8 +1,8 @@
<div>
<div
- class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium em57xhy0"
+ class="components-card__body components-card-body css-kqnlb3-View-Body-borderRadius-medium-shady em57xhy0"
- class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium e19lxcc00"
+ class="components-card__body components-card-body css-kqnlb3-View-Body-borderRadius-medium-shady e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardBody"
>
Expand All @@ -42,8 +42,8 @@ Snapshot Diff:
@@ -1,8 +1,8 @@
<div>
<div
- class="components-flex components-card__footer components-card-footer css-p1v47q-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium em57xhy0"
+ class="components-flex components-card__footer components-card-footer css-1xx98hc-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium-shady em57xhy0"
- class="components-flex components-card__footer components-card-footer css-p1v47q-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium e19lxcc00"
+ class="components-flex components-card__footer components-card-footer css-1xx98hc-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium-shady e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardFooter"
>
Expand All @@ -59,8 +59,8 @@ Snapshot Diff:
@@ -1,8 +1,8 @@
<div>
<div
- class="components-flex components-card__footer components-card-footer css-p1v47q-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium em57xhy0"
+ class="components-flex components-card__footer components-card-footer css-gg08fg-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium em57xhy0"
- class="components-flex components-card__footer components-card-footer css-p1v47q-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium e19lxcc00"
+ class="components-flex components-card__footer components-card-footer css-gg08fg-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-medium e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardFooter"
>
Expand All @@ -76,8 +76,8 @@ Snapshot Diff:
@@ -1,8 +1,8 @@
<div>
<div
- class="components-flex components-card__header components-card-header css-1g5oj2q-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-medium em57xhy0"
+ class="components-flex components-card__header components-card-header css-121pfkj-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-medium-shady em57xhy0"
- class="components-flex components-card__header components-card-header css-1g5oj2q-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-medium e19lxcc00"
+ class="components-flex components-card__header components-card-header css-121pfkj-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-medium-shady e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardHeader"
>
Expand All @@ -93,19 +93,19 @@ Snapshot Diff:
@@ -5,18 +5,18 @@
>
<div
class="css-mgwsf4-View-Content em57xhy0"
class="css-mgwsf4-View-Content e19lxcc00"
>
<div
- class="components-flex components-card__header components-card-header css-1g5oj2q-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-medium em57xhy0"
+ class="components-flex components-card__header components-card-header css-1e8ifbz-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-large em57xhy0"
- class="components-flex components-card__header components-card-header css-1g5oj2q-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-medium e19lxcc00"
+ class="components-flex components-card__header components-card-header css-1e8ifbz-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-large e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardHeader"
>
Header
</div>
<div
- class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium em57xhy0"
+ class="components-card__body components-card-body css-1nonx1n-View-Body-borderRadius-large em57xhy0"
- class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium e19lxcc00"
+ class="components-card__body components-card-body css-1nonx1n-View-Body-borderRadius-large e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardBody"
>
Expand Down Expand Up @@ -138,27 +138,27 @@ Snapshot Diff:
@@ -6,25 +6,25 @@
>
<div
class="css-mgwsf4-View-Content em57xhy0"
class="css-mgwsf4-View-Content e19lxcc00"
>
<div
- class="components-flex components-card__header components-card-header css-1uuauve-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-large-borderless em57xhy0"
+ class="components-flex components-card__header components-card-header css-yf9nll-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-small em57xhy0"
- class="components-flex components-card__header components-card-header css-1uuauve-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-large-borderless e19lxcc00"
+ class="components-flex components-card__header components-card-header css-yf9nll-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-small e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardHeader"
>
Header
</div>
<div
- class="components-card__body components-card-body css-1nonx1n-View-Body-borderRadius-large em57xhy0"
+ class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium em57xhy0"
- class="components-card__body components-card-body css-1nonx1n-View-Body-borderRadius-large e19lxcc00"
+ class="components-card__body components-card-body css-1nwhnu3-View-Body-borderRadius-medium e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardBody"
>
Body
</div>
<div
- class="components-flex components-card__footer components-card-footer css-s0icc3-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-large-borderless em57xhy0"
+ class="components-flex components-card__footer components-card-footer css-cwhos2-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-xSmallCardPadding em57xhy0"
- class="components-flex components-card__footer components-card-footer css-s0icc3-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-large-borderless e19lxcc00"
+ class="components-flex components-card__footer components-card-footer css-cwhos2-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-xSmallCardPadding e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardFooter"
>
Expand All @@ -174,33 +174,33 @@ Snapshot Diff:
@@ -1,30 +1,30 @@
<div>
<div
- class="components-surface components-card css-hkvggq-View-Surface-getBorders-primary-Card-boxShadowless-rounded em57xhy0"
+ class="components-surface components-card css-nsno0f-View-Surface-getBorders-primary-Card-rounded em57xhy0"
- class="components-surface components-card css-hkvggq-View-Surface-getBorders-primary-Card-boxShadowless-rounded e19lxcc00"
+ class="components-surface components-card css-nsno0f-View-Surface-getBorders-primary-Card-rounded e19lxcc00"
data-wp-c16t="true"
data-wp-component="Card"
>
<div
class="css-mgwsf4-View-Content em57xhy0"
class="css-mgwsf4-View-Content e19lxcc00"
>
<div
- class="components-flex components-card__header components-card-header css-1uuauve-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-large-borderless em57xhy0"
+ class="components-flex components-card__header components-card-header css-yf9nll-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-small em57xhy0"
- class="components-flex components-card__header components-card-header css-1uuauve-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-large-borderless e19lxcc00"
+ class="components-flex components-card__header components-card-header css-yf9nll-View-Flex-sx-Base-sx-Items-ItemsRow-Header-borderRadius-borderColor-small e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardHeader"
>
Header
</div>
<div
- class="components-card__body components-card-body css-1nonx1n-View-Body-borderRadius-large em57xhy0"
+ class="components-card__body components-card-body css-5spj1a-View-Body-borderRadius-small em57xhy0"
- class="components-card__body components-card-body css-1nonx1n-View-Body-borderRadius-large e19lxcc00"
+ class="components-card__body components-card-body css-5spj1a-View-Body-borderRadius-small e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardBody"
>
Body
</div>
<div
- class="components-flex components-card__footer components-card-footer css-s0icc3-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-large-borderless em57xhy0"
+ class="components-flex components-card__footer components-card-footer css-oc4v7j-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-small em57xhy0"
- class="components-flex components-card__footer components-card-footer css-s0icc3-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-large-borderless e19lxcc00"
+ class="components-flex components-card__footer components-card-footer css-oc4v7j-View-Flex-sx-Base-sx-Items-ItemsRow-Footer-borderRadius-borderColor-small e19lxcc00"
data-wp-c16t="true"
data-wp-component="CardFooter"
>
Expand Down Expand Up @@ -786,15 +786,15 @@ Snapshot Diff:
</div>
<div
aria-hidden="true"
- class="components-elevation css-1lsfy80-View-Elevation-sx-Base-elevationClassName em57xhy0"
+ class="components-elevation css-15t1t3g-View-Elevation-sx-Base-elevationClassName em57xhy0"
- class="components-elevation css-1lsfy80-View-Elevation-sx-Base-elevationClassName e19lxcc00"
+ class="components-elevation css-15t1t3g-View-Elevation-sx-Base-elevationClassName e19lxcc00"
data-wp-c16t="true"
data-wp-component="Elevation"
/>
<div
aria-hidden="true"
- class="components-elevation css-18cl04p-View-Elevation-sx-Base-elevationClassName em57xhy0"
+ class="components-elevation css-15t1t3g-View-Elevation-sx-Base-elevationClassName em57xhy0"
- class="components-elevation css-18cl04p-View-Elevation-sx-Base-elevationClassName e19lxcc00"
+ class="components-elevation css-15t1t3g-View-Elevation-sx-Base-elevationClassName e19lxcc00"
data-wp-c16t="true"
data-wp-component="Elevation"
/>
Expand Down
7 changes: 1 addition & 6 deletions packages/components/src/checkbox-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,7 @@ 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 >
) {
const {
label,
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/disabled/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function Disabled( {
children,
isDisabled = true,
...props
}: Omit< WordPressComponentProps< DisabledProps, 'div' >, 'ref' > ) {
}: WordPressComponentProps< DisabledProps, 'div' > ) {
const ref = useDisabled();

if ( ! isDisabled ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,25 @@ Snapshot Diff:
@@ -1,22 +1,18 @@
<div
class="components-flex css-1giw1wa-View-Flex-sx-Base-sx-Items-ItemsRow em57xhy0"
class="components-flex css-1giw1wa-View-Flex-sx-Base-sx-Items-ItemsRow e19lxcc00"
- data-testid="flex"
+ data-testid="base-flex"
data-wp-c16t="true"
data-wp-component="Flex"
>
<div
class="components-flex-item css-mw3lhz-View-Item-sx-Base em57xhy0"
class="components-flex-item css-mw3lhz-View-Item-sx-Base e19lxcc00"
data-wp-c16t="true"
data-wp-component="FlexItem"
>
Item
</div>
- <div
- class="css-1mm2cvy-View em57xhy0"
- class="css-1mm2cvy-View e19lxcc00"
- />
- <div />
<div
class="components-flex-item components-flex-block css-14wzr73-View-Item-sx-Base-block em57xhy0"
class="components-flex-item components-flex-block css-14wzr73-View-Item-sx-Base-block e19lxcc00"
data-wp-c16t="true"
data-wp-component="FlexBlock"
>
Expand Down
7 changes: 1 addition & 6 deletions packages/components/src/form-toggle/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 >
) {
const {
className,
Expand Down
Loading

0 comments on commit 3c679aa

Please sign in to comment.