Todo List - Project Feedback Please!

Hey guys,

This isn’t a FCC project but I would highly appreciate if you give me ANY feedback on this project.

Here is a link to my code in case anyone wants to look at it.

Thank you!

Complete and absolute noob here, so I can’t give constructive comment on technicalities and code (sorry), but I think it looks super nice!

What technologies did you use to build it.


"react": "^16.8.3",
"styled-components": "^4.1.3"


const { state, toggleCompleted, removeTodo, handleVisibility, handlePriorityVisibility, addTodo } = this;
const props = { state, toggleCompleted, removeTodo, handleVisibility, handlePriorityVisibility, addTodo };    
<Dashboard {...props} />

You are spreading all properties into variables, then you make an object of them and call it props.

You could write <Dashboard {...this} /> and delete the two other lines.
This would save around 10 formatted lines & improve the readability “Oh, he passes all class properties to Dashboard” vs. “WTF is he doing?”.
No need for thinking about performance.

Looking good.

Hey thanks! I knew about this and was debating.

Like you said, I could simply do {…this} and pass down props like that but I noticed that it also passes down other built-in methods of this besides the ones that I coded up.

So does performance barely get any hit if any even if I pass down props that child components won’t use?


:arrow_up: comment and for fading effects I used react-reveal library.

@pipestream Hey thanks for the compliment!

Don’t do this. You should pass only stuff you use not because of performance concerns (I doubt there are any), but because when you use it in combination with stuff like prop-types or TypeScript types/interfaces you’ll get nice warnings when you forget to pass a prop or pass a wrong one.

The way you did it was horrible. “WTF is he doing?” was my exact thoughts.

Btw, kudos for using hooks :wink: And I see you gave up on Transitions :unamused:

How would you solve it?

For using Typescript, I think in this rare case we can use an any,
instead of sacrificing readability by passing 6 or more props.

I would get rid of separate Dasboard component altogether (it’s just styling), put Header and Todos in App and pass relevant props directly by doing verbose, but more readable <Todos todos={todos} visible addTodo={} ... />.
And prettier will format all 6 (or whatever) props nicely.

For TS I would create an interface that describes passed props to each component. With relevant type. No any!!!

1 Like

That sounds like a good approach. What about a scenario where there are a lot more props to pass down like 10? and looks ridiculous. Would you still do what you suggested? Or is that a point where I start breaking it into smaller components? I think using Redux would not be an option since it’s just one or two components down.

Hard to tell. There is no “one-size-fits-all” solution. One approach is to keep as may as possible components in parent components.

Instead of:

  <Dashboard props1 props2 props3 props4/>

  <Header props1>
  <Tods props2 props3 />
  <Footer props4 />

you can do:

    <Header props1>
    <Tods props2 props3 />
    <Footer props4 />

You could check popular projects on github and see every possible approach.

1 Like


Thanks, it makes sense.

Probably not a good idea to treat styled-component style like a react component and move nested react components up one level will be better.