Reputation: 153
Imagine that I have a List of Cards on my react application. So I will have two components, the <ListCard />
and <Card />
.
My doubt is that where I work, when we have something like this, we usually declare a variable inside the <ListCard />
named Card
that receives a JSX, but I don't know if this is a good thing to do. Recently I faced some problems with this approach, but I didn't find anyone saying to do it or not.
Imagine that I'm not using the <Card />
anywhere in the application
The way we would declare the ListCard
const ListCard = () => {
const cards = [
{ title: '1', id: 1 },
{ title: '2', id: 2 },
];
const Card = ({ title }) => <div>{title}</div>;
return (
<div>
<h1>List of Cards</h1>
{cards.map(card => (
<Card title={card.title} key={card.id} />
))}
</div>
);
};
I want to know if it's better to declare outside of the ListCard, like this.
const Card = ({ title }) => <div>{title}</div>;
const ListCard = () => {
const cards = [
{ title: '1', id: 1 },
{ title: '2', id: 2 },
];
return (
<div>
<h1>List of Cards</h1>
{cards.map(card => (
<Card title={card.title} key={card.id} />
))}
</div>
);
};
Upvotes: 15
Views: 15518
Reputation: 93
The React docs, which should be a good authority regarding this, explicitly advises against nested component definitions.
The main issue is with hooks. Taking the example from the second link:
import { useState } from 'react';
export default function MyComponent() {
const [counter, setCounter] = useState(0);
function MyTextField() {
const [text, setText] = useState('');
return (
<input
value={text}
onChange={e => setText(e.target.value)}
/>
);
}
return (
<>
<MyTextField />
<button onClick={() => {
setCounter(counter + 1)
}}>Clicked {counter} times</button>
</>
);
}
Whatever state is held by MyTextField
gets lost when MyComponent
rerenders, because MyTextField
is redefined every render. From React's perspective, the MyTextField
from a previous render is an entirely different thing from the MyTextField
of the current render, and thus won't reassociate their states.
Upvotes: 4
Reputation: 6075
Contrary to what is said in the other answers, it might be perfectly fine to declare components inside a component. It can even be quite useful in some situations where you need to access functions from the parent scope, and when rendering each item would be costly. The only thing is that you need to understand what you're doing, be careful, and do it correctly. But you will learn that, with React hooks, any approach always need you to have a good understanding of the render process.
In your case, to prevent remounting the component on every render, you need to keep the same reference on the child component you are creating. For that, we use the useMemo
hook. Be careful, because each time a dependency of that hook updates, your reference will change and so the children will remount. There are ways to work around this and always keep the same reference, but that might be a bit off-topic.
Here is a common example:
const List = ({ items, onItemSelected }) => {
// If you don't use useMemo, your component will be recreated every time and
// React will destroy and recreate every item at every render.
const Item = React.useMemo(() => ({ index, item }) => {
const onClick = React.useCallback(() => onItemSelected(index), [index])
return <li onClick={onClick}>{item}</li>
}, [onItemSelected])
return (<ul>
{items.map((item, index) => <Item key={index} index={index} item={item} />)}
</ul>)
}
You can find a complete working example here
Here is the full code with some comments you might want to pay attention to.
import React from "react";
const items = ["apple", "banana", "orange"];
export default function App() {
const [selected, setSelected] = React.useState(-1);
const onItemSelected = React.useCallback((index) => setSelected(index), []);
return (
<div>
{/* Add some logs and see what happens if you add "selected={selected}" to the props of List */}
<List items={items} onItemSelected={onItemSelected} />
<p>
You pressed <b>{items[selected]}</b>
</p>
</div>
);
}
// We used React.memo so the list won't rerender when the parent state changes
// This is a good optimization, but nothing mandatory, here
const List = React.memo(({ items, onItemSelected }) => {
// If you don't use useMemo, your component will be recreated every time and
// React will destroy and recreate every item at every render.
const Item = React.useMemo(() => {
// We use this syntax only for eslint to detect that this is a component.
// There is nothing mandatory, here, neither.
const Component = ({ index, item }) => {
// The main interess here, is that "onItemSelected" is not a prop anymore.
// This way, we can make onClick a callback that won't change between renders
const onClick = React.useCallback(() => onItemSelected(index), [index]);
return <li onClick={onClick}>{item}</li>;
};
Component.displayName = "Item";
return Component;
// be very carefull with what you put here. The component should not depend on things
// That might change often, because every items will be destroyed and recreated when they change
// Try to add "selected" in this dependency list and see the impacts on rerender
}, [onItemSelected]);
return (
<ul>
{items.map((item, index) => (
<Item key={index} index={index} item={item} />
))}
</ul>
);
});
Upvotes: 2
Reputation: 14355
In a functional component, every variable gets destroyed and recreated again every render. This is what makes the useState
hook so valuable, as it is smart enough to recreate it's variables with the previous or updated values.
By declaring a component inside another component, you are not only re-rendering both components, but completely redeclaring one. This won't be very performant, especially if the component is more complex.
So the answer to your question is always declare it separate. It would work declared inside, but there's no benefits from it, only drawbacks.
Upvotes: 8
Reputation: 421
It is totally fine to declare another Component
in the same file, but declaring it inside another one's function would be inefficient. Imagine your app 'recreating' the second Component
during each render
.
So, feel free to declare it in the same file, but don't do it inside another Component
's function.
Upvotes: 22