-
-
Save cchaos/56d0bcfe0c4f4fb47148056bf1627b77 to your computer and use it in GitHub Desktop.
import React from 'react'; | |
import { ExclusiveUnion } from '../common'; | |
/** | |
* 1. Use the EUI provided ExclusiveUnion to separate unique props. | |
*/ | |
type _RestrictedProps = ExclusiveUnion< | |
{ | |
/** | |
* 2. At least one of the unique props must be required, usually the one containing more options | |
*/ | |
position: 'fixed'; | |
/** | |
* 3. These next props are only available when `position` is `fixed` | |
*/ | |
foo?: boolean; | |
bar?: boolean; | |
}, | |
{ | |
/** | |
* 4. These are the other options that can't be used in conjunction with the above props | |
*/ | |
position?: 'static' | 'sticky'; | |
} | |
>; | |
export interface EuiComponentProps | |
extends _RestrictedProps { | |
/** | |
* 5. Add all your other props that are allowed no matter the other configurations | |
*/ | |
stuff?: string; | |
} | |
export const EuiComponent: FunctionComponent<EuiComponentProps> = ({ | |
position = 'fixed', | |
foo = true, | |
bar = true, | |
stuff, | |
...rest | |
}) => { | |
/** | |
* 6. Still ensure the props values are correct for the particular configuration | |
*/ | |
foo = position !== 'fixed' ? false : foo; | |
bar = position !== 'fixed' ? false : bar; | |
return (); | |
} |
import React from 'react'; | |
import { ExclusiveUnion } from '@elastic/eui'; | |
export interface MyComponentProps { | |
/** | |
* 7. This component allows consumers to override defaults supplied to the resctricted component | |
*/ | |
overrides?: EuiComponentProps; | |
} | |
export const MyComponent: FunctionComponent<MyComponentProps> = ({ | |
overrides, | |
}) => { | |
return ( | |
<EuiComponent | |
position="sticky" | |
// 8. Typescript will always complain, so using unknown here, | |
// but it won't restrict consumers from providing incompatible props, so ensure the EUI component handles that (see #6) | |
{...(overrides as unknown)} | |
/> | |
); | |
} |
Honestly, I don't know when it's supposed to be type
vs interface
. From other docs I've ready, people prefer interface
, but I don't really know the difference. Also, I've truncated that quite a bit. There's a lot more props in there, but I just wanted to simplify for the gist.
interface
s are more restricted, because they can only express object types. AFAIK the only thing possible with interface
s that plain object type
s can't do is declaration merging. In my experience this is mostly used to deal with the dynamic nature of javascript code that was not written with strong typing in mind.
Converting EuiBottomBarProps
to a type is fine; the ExclusiveUnion
usage means that TS cannot, without more information, know the exact shape of the interface because it depends on if the input contains position: fixed
, which is what TS is somewhat cryptically referring to with
TS2312: An interface can only extend an object type or intersection of object types with statically known members.
So we convert to (dropping point #5 as well because its redundancy is not needed),
export type EuiBottomBarProps = CommonProps &
HTMLAttributes<HTMLElement> &
_BottomBarExclusivePositions;
Which looks like it resolves the rest of the issues you encountered. My goto rule with choosing interface vs. type: start with interface
but upgrade to type
when you have to. The exact differences are difficult to follow and often change subtly between TS releases.
My goto rule with choosing interface vs. type: start with interface but upgrade to type when you have to.
Same.
I think it's 👍 for these to be intersection types.
Thanks! I've converted it all to use type
. And it's working well, except for one use case. Where I'm using this component inside of another component with a few default props setup but the user can override them. The way we normally handle this, is just by applying the props we want then spreading the overridable props at the end. Like so:
<EuiBottomBar
paddingSize={paddingSize}
position="sticky"
{...bottomBarProps}
/>
But this particular setup blurps an error:
TS isn't able to understand the ExclusiveUnion through another component's rest
handling, unfortunately. We cast those spreads as the props, so: {...(bottomBarProps as EuiBottomBarProps)}
(example at https://github.com/elastic/eui/blob/master/src/components/header/header_links/header_link.tsx#L52)
Try moving position="sticky"
below the bottomBarProps
spread
At this location, https://github.com/cchaos/eui/blob/feat/bottom_bar_in_page/src/components/bottom_bar/bottom_bar.tsx#L52, the following change is needed: position?: 'fixed';
-> position: 'fixed';
(removed the ?
)
Reason: When TS validates an incoming value against the two sides of a union, it looks for unique key definitions. As it is right now, the two sides for position are undefined | 'fixed'
and undefined | 'sticky' | 'static'
, and because both share the undefined
value it is not unique. This prevents the expected validation, and also confused TS when it tried to understand the shape of bottomBarProps
.
Which opens up a second issue, where the Partial
in bottomBarProps?: Partial<EuiBottomBarProps>;
again removes that distinction by making adding undefined
back to position: 'sticky'
. In this specific case the fix is simple: bottomBarProps?: EuiBottomBarProps;
. This works because every prop in EuiBottomBarProps is already optional - except for position
when opting into usePortal
or affordForDisplacement
. We'll need to explore other approaches for the case where a component provides default or un-overridable values.
Haha, apparently this one is being super tricky. Everything that you just said works and makes sense. Except now when I want to JUST adjust the prop allowed when position="fixed"
. So this:
<EuiBottomBar usePortal={false} />
But, even though position="fixed"
is the default, TS complains telling me I need to add it.
❤️ Thank you all for your help and input. I've updated this gist to exemplify both the "finalized" component and usage that we can link to in our EUI wiki.
Maybe I'm missing some context, but is there any reason why
EuiBottomBarProps
needs to be aninterface
? Would an intersection type likebe an option too?