fix(mixed): correct types to match propTypes #550
Conversation
layershifter
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
layershifter
changed the title
| import Icon from '../Icon/Icon' | ||
| import Button from '../Button/Button' | ||
| import Text from '../Text/Text' | ||
| import Slot from '../Slot/Slot' | ||
|
|
||
| export interface AttachmentProps extends UIComponentProps, ChildrenComponentProps { | ||
| /** Button shorthand for the action slot. */ | ||
| action?: ShorthandValue | ||
| action?: Nullable<ShorthandValue> |
There was a problem hiding this comment.
maybe it would be easier to use generic type that will be able to transform entire props type, like that:
export interface AttachmentProps {
action?: ShorthandValue,
...
}
class Attachment extends UIComponent<Nullable<AttachmentProps>, ...>
layershifter
force-pushed
the
fix/update-typings
branch
from
7990eac
to
6085b93
Compare
layershifter
force-pushed
the
fix/update-typings
branch
2 times, most recently
from
6bb0eba
to
a95e382
Compare
|
I don't understand this:
🦀 |
layershifter
changed the title
| @@ -24,6 +27,10 @@ export type ObjectOrFunc<TResult, TArg = {}> = ((arg: TArg) => TResult) | TResul | |||
|
|
|||
| export type Props = ObjectOf<any> | |||
| export type ReactChildren = React.ReactNodeArray | React.ReactNode | |||
|
|
|||
| export type ReactPropsStrict<T> = { [K in keyof T]: NullableIfUndefined<T[K]> } | |||
| export type ReactProps<T> = Extendable<ReactPropsStrict<T>> | |||
There was a problem hiding this comment.
There is a small difference, ReactPropsStrict cannot be extended it's very useful for Portal, Ref and other component that don't allow to spread props.
layershifter
force-pushed
the
fix/update-typings
branch
2 times, most recently
from
283ee27
to
8ca9f2a
Compare
layershifter
force-pushed
the
fix/update-typings
branch
from
8ca9f2a
to
393f7e9
Compare
|
#608 showed that a build can be passed, I will try to fix all remaining errors to be compatible with the latest TS. |
| @@ -1,7 +1,7 @@ | |||
| import React from 'react' | |||
| import { Grid, Ref, Segment } from '@stardust-ui/react' | |||
|
|
|||
| const ExampleButton = React.forwardRef<HTMLButtonElement>((props, ref) => ( | |||
| const ExampleButton = React.forwardRef<HTMLButtonElement, { children: string }>((props, ref) => ( | |||
There was a problem hiding this comment.
It becomes more problematic, we should force #495.
|
|
||
| return ( | ||
| <ElementType> | ||
| <li> |
There was a problem hiding this comment.
This didn't make any reasonable sense, but caused my favourite error, microsoft/TypeScript#28768
| @@ -1,3 +1,5 @@ | |||
| import * as React from 'react' | |||
There was a problem hiding this comment.
This import was absent before
| @@ -24,6 +27,10 @@ export type ObjectOrFunc<TResult, TArg = {}> = ((arg: TArg) => TResult) | TResul | |||
|
|
|||
| export type Props = ObjectOf<any> | |||
| export type ReactChildren = React.ReactNodeArray | React.ReactNode | |||
|
|
|||
| export type ReactPropsStrict<T> = { [K in keyof T]: NullableIfUndefined<T[K]> } | |||
| export type ReactProps<T> = Extendable<ReactPropsStrict<T>> | |||
There was a problem hiding this comment.
So when I should use Extendable vs ReactProps? My understanding is I should always use ReactProps (or ReactPropsStrict in special cases).
But for example Dropdown is still using Extendable.
Shouldn't we remove Extendable completely?
There was a problem hiding this comment.
Dropdown should use ReactProps, too. I missed it, will check all Dropdown's components.
Shouldn't we remove Extendable completely?
Note sure, it's a quite generic type, we're using it in theme/types.ts for colors and props.
…m/stardust-ui/react into fix/update-typings # Conflicts: # docs/src/examples/components/Ref/Types/RefForwardingExample.tsx # src/components/List/List.tsx # src/components/List/ListItem.tsx # src/components/Menu/Menu.tsx # src/components/Menu/MenuItem.tsx
…m/stardust-ui/react into fix/update-typings
Fixes #548.
TS sandbox with types, don't forget to enable
strictNullChecksin options 👍