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
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
types/utils.d.ts
Outdated
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. |
| 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
| 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
4 participants
Fixes #548.
TS sandbox with types, don't forget to enable
strictNullChecksin options 👍