Problem
I’ve implemented the logic of my app, but I’m not sure whether it appropriate way or not.
Task: create a react app using redux, react-router v4 which has a page with a form for adding new post and list of posts
and a page with a detail view of one of the posts
I’ve added a unique id for each post with help of uuid and pass id of each post to the corresponding link(react-router-dom link) in the post list
<Link to={props.id} className='post__header'>{props.title}</Link>
When link is clicked it passes id to the DetailView component. DetailView component takes id and seek the post with the same id in the whole state then render the post
import React from 'react';
import { connect } from 'react-redux';
let DetailView = (props) => {
console.log(props);
var currentPost = props.posts.find(post => post.id === props.match.params.id);
return (
<div>
<h1>{currentPost.topic}</h1>
<p>{currentPost.text}</p>
</div>
)
}
const mapStateToProps = (state) => {
return state;
};
DetailView = connect(mapStateToProps)(DetailView);
export default DetailView;
Github link: https://github.com/LKTN/Branch/tree/dev
Solution
Misc
I’ll start with these because to be able to use them in the next sections.
I personally prefer using const
, but I won’t get into the details, there has been enough debate in the media about it. In this case, I think that declaring the component with let
is a bit misleading, because usually, you do this when expanding the API exported with the component, namely:
let List = props => { ... };
List.Item = props => { ... };
export default List;
Which then gets used like this:
const Showcase = () => (
<List>
<List.Item>First</List.Item>
<List.Item>Second</List.Item>
<List.Item>Third</List.Item>
</List>
);
So, I would change the declaration of the component to:
const DetailView = (props) => { ... }
export default connect(mapStateToProps)(DetailView);
Props
Destructuring props makes the code a bit more readable because from a glance you can see what the component expects:
let DetailView = ({ posts, match }) => {
var currentPost = posts.find(post => post.id === match.params.id);
}
var
const
and let
were introduced two fix a couple of gotchas that javascript had, mostly related to var
. You shouldn’t use var
except in very special cases where you need its behavior (Take this with some salt, because I think that this behavior is confusing and using the alternatives improves the code).
You can read more about it running a simple query;
const currentPost = posts.find(post => post.id === match.params.id);
Redux
State to Props
I see you are mapping your whole redux
state to your props
. This is a bad practice because that will make your component re-render on every state change. Instead, as recommended in the official docs, you should return only the slice of state that your component uses.
const mapStateToProps = (state) => ({
props: state.props
});
Hooks
The official docs for react-redux recommend using hooks instead of the connect
API. Changing your included code to use hooks would leave use with the following:
import React from 'react';
import { useSelector } from 'react-redux';
const DetailView = ({ match }) => {
const posts = useSelector(state => state.posts);
const post = posts.find(post => post.id === match.params.id);
return (
<div>
<h1>{post.topic}</h1>
<p>{post.text}</p>
</div>
)
}
export default DetailView;
Router
I see you are expecting a match
prop in your component. You can avoid that prop if you take advantage of the useParams
hook that react-router-dom
provides:
import React from 'react';
import { useSelector } from 'react-redux';
import { useParams } from 'react-router-dom';
const DetailView = () => {
const posts = useSelector(state => state.posts);
const { id } = useParams();
const post = posts.find(post => post.id === id);
return (
<div>
<h1>{post.topic}</h1>
<p>{post.text}</p>
</div>
)
}
export default DetailView;
```
Looks good after a quick skim. Just one point. I think this project could benefit from following the Container Component pattern so that the markup components don’t directly depend on redux and are reusable.
Read more at https://medium.com/@learnreact/container-components-c0e67432e005