So you got a new ticket building a new project, at first, it seemed pretty easy, you could simply use the same table in all three views and be happy that the designer finally did something right.
const Table = (data) => {
return (
<div>
<Title>Data:</Title>
<div className='awesomeTable'>
{data.map((row) => <Row data={row} id={row.id} />)}
</div>
</div>
);
}
Then, the product wanted an additional subtitle in case of the dogs table. So, you added a condition specifically for the dogs' data.
const Table = (data) => {
return (
<div>
<Title>Data:</Title>
{data[0].siblings && <Subtitle>Dogs are cool</Subtitle>}
<div className='awesomeTable'>
{data.map((row) => <Row data={row} id={row.id} />)}
</div>
</div>
);
}
Some time passed, and a request came to change the rows in the cakes view. Now you can decide to keep the implicit solution and rely on the data attributes.
const Table = (data) => {
return (
<div>
<Title>Data:</Title>
{!!data[0].siblings && <Subtitle>Dogs are cool</Subtitle>}
<div className='awesomeTable'>
{data.map((row) => <Row data={row} id={row.id} withLinks={!!row.topping} />)}
</div>
</div>
);
}
But then you received a review on your PR stating that the component looks like a mess, so you had no choice but to put some order.
const Table = (data, type: 'dogs' | 'cakes' | 'cars') => {
return (
<div>
<Title>Data:</Title>
{type === types.DOGS && <Subtitle>Dogs are cool</Subtitle>}
<div className='awesomeTable'>
{data.map((row) => <Row data={row} id={row.id} withLinks={type === types.CAKES} />)}
</div>
</div>
);
}
And over time, this became a real clutter.
const Table = (data, type: 'dogs' | 'cakes' | 'cars') => {
return (
<div>
<Title>{type !== types.CARS ? Capitalize(type) : <a href={CARS_STORE_URL}>{Capitalize(type)}</a>}</Title>
{type !== types.CAKES && <Subtitle>{type} are cool</Subtitle>}
<div className='awesomeTable'>
{data.map((row) => <Row data={row} id={row.id} withLinks={type === types.CAKES} fullName={type === types.DOGS} />)}
</div>
</div>
);
}
Now, this has become a real shit show, and I have seen so many cases like this over the years. When I try to discuss this with a dev maintainer, I usually get the same response: "I think it's better to have all the logic in the same place."
In my opinion, this is really a bad practice and is wrong for two reasons:
- Those conditions are not part of the table logic, but rather the dogs view logic. It's their scope, just like rendering the table itself is in its scope.
- The example above is a difficult-to-read and maintain component but the worse part is testing it. It’s an excuse to write code in one big chunk in the name of reusability.
Instead, there are many techniques of component composition. It's out of scope to cover them all, but I will demonstrate my preference in such a use case:
const Dogs = () => {
// ...
return (
<div className='awesomeDogs'>
<Table>
<Table.Title><a href={CARS_STORE_URL}>Dogs</a></Table.Title>
<Table.Content>
{dogs.map((row) => <Table.Row data={row} id={row.id} fullName />)}
</Table.Content>
</Table>
</div>
);
}
const Cars = () => {
// ...
return (
<div className='awesomeCars'>
<Table>
<Table.Title>Cars</Table.Title>
<Table.Content>
{cars.map((row) => <Table.Row data={row} id={row.id} withLinks />)}
</Table.Content>
</Table>
</div>
);
}
TL;DR- components should not be aware of context, they should always be dumb and use composition for the different use cases.
Top comments (1)
Thanks.
Looking forward to reading the "component composition patterns" post..