Add option to `react/destructuring-assignment` to skip destructuring on union types on `always`
SimonSimCity opened this issue · 6 comments
Going further on #3658, I'd propose to add a new option to the table:
Add a new setting in which unions are not required to be destructed.
See for you an option which would require you to always destruct unless it is a union you are about to receive.
Given the discussion there, can you elaborate on when it’s actually preferable to pass a union, and why you wouldn’t want to still destructure it out of props so you can type-narrow it?
One example is if you have one property where you (based on its value) have another property. An example of HTML is the input
type, where the property max
is not allowed if the type
is checkbox
, but it is allowed on number
.
We have one component which needs either two additional props or none of the two. For this, we created a third boolean prop, by which we can determine if both are present and a union interface around it. This way we can ensure that never only one of them is present.
Another way I see is to create an optional property as object, but in that case we'd have to find a way around Reacts way of rendering, because the prop would always get a new object on every render, which could be solved by memo
, which again requires additional bloat to the places we use this component.
We currently disable this rule on the components we use unions.
I'm not sure that input
is the best component to model an API off of :-)
I think I understand, though - you're saying that you'd destructure the third boolean, and then inside one of the type-narrowed branches, destructure or access the two other props, and the rule is complaining about the latter?
If you can provide a more concrete code example of such a component it might help me either suggest a better alternative API, or, become convinced of the need.
The default input props do not have props check for each type of the input of the input, for example the input of type checkbox
can have text props like maxLength
and this will be acceptable by react prop type ComponentPropsWithoutRef<'input'>
so to do so you can build your own input with some type check using union and types like this (simple example not complete):
type InputProp =
| ({ type: 'checkbox' } & Omit<React.ComponentPropsWithoutRef<'input'>, 'min' | 'max' | 'type'>)
| ({ type: 'number' } & React.ComponentPropsWithoutRef<'input'>);
I can give you much clearer example by using a real use case I am facing now:
I have a component that has text in it and show some views and images inside. In some places I may not need the text inside so I created a prop called hideText
and if this prop exists it should not accept the label, value, or anything that has to do with text data. To do so, I created this union type
:
type ViewWithoutTextProps = Omit<ViewData,'title' | 'powerValue' | 'unit' | 'type'> & {
hideText: true;
type: ViewDataType['type'];
};
type ViewWithTextProps = Omit<
ViewData,
'energyType'
> & {
hideText?: false;
hideBoundaryBorder?: boolean;
};
type ViewProps = (ViewWithoutTextProps | ViewWithTextProps) & {
isSelected?: boolean;
};
In this if the hideText = true
it you will be able to add the powerValue
... etc. but if it is false
it does not exist on the type. By that when I use Type Narawing
in the component I can determine what to show and what to hide with the type Narawing
the type will change based on the hideText
condition:
{!props.hideText && (
<>
{props.powerValue != null ? (
<p
className={`absolute top-full max-w-[70%] text-center text-xs font-bold
${props.type === 'in' ? 'left-0' : 'right-0'}`}
>
{`${props.powerValue}
</p>
) : null}
</>
)}
In this, it will be defined because of the Type Narwaing
but if I use destructure to the power value at the top of the component using:
const { powerValue } = props;
it will raise type error
:
I've read your comment a number of times, and I keep coming to the same conclusion - this should be two separate components, one which takes text and one which doesn't.
Also for what I see now, #3658 (comment) might be very much true by saying that this seems rather like an anti-pattern in react.
I'll close this for now and check how I can resolve my use cases by the code example given in the comment. It anyways fits reacts pattern better of having rather more (smaller) than less (big and bloated) components. I'll close this issue.