TodoItem component type checking for optional prop

Question

I have a TodoItem component that looks like this:

type Todo = {
  id: string;
  text: string;
  isCompleted: boolean;
  createdAt: string;
};

type TodoProps = {
  todo: Todo;
  removeTodo: (id: string) => void;
  markCompletedTodo?: (id: string) => void;
};

const TodoItem = ({ todo, removeTodo, markCompletedTodo }: TodoProps) => {
  return (
    <div>
      <h2>{todo.text}</h2>
      <button type="button" onClick={() => removeTodo(todo.id)}>
        Remove
      </button>
      {typeof markCompletedTodo === 'undefined' ? null : (
        <button type="button" onClick={() => markCompletedTodo(todo.id)}>
          Mark completed
        </button>
      )}
    </div>
  );
};

export default TodoItem;

As you can see, it gets a todo with a isCompleted property. I want to render the 'Mark completed' button only is the isCompleted property is false, but currently I'm checking for the optional markCompletedTodo prop. That works but I have the feeling this can be done in a better way.

I'm using it to create two lists of TODOS, one for incomplete TODOS and one for complete TODOS, so I'm using it like this in another component:

return (
  <h2>Incomplete TODOS</h2>
  {incompleteTodos.map((todo) => (
    <TodoItem
    key={todo.text}
    todo={todo}
    removeTodo={removeTodoRequest}
    markCompletedTodo={markCompletedTodoRequest}
    />
  ))}
  <h2>Complete TODOS</h2>
  {completeTodos.map((todo) => (
    <TodoItem
      key={todo.text}
      todo={todo}
      removeTodo={removeTodoRequest}
    />
  ))}
)

Is there a better approach to this regarding the type checking and the prop types in the TodoItem component, or any other aspect?

Answer 1

Make the shapes the same In my experience, TypeScript is easiest when similar data has the same shape (type). Rather than conditionally passing a markCompletedTodo, consider passing that prop unconditionally, and in the child component, check todo.isCompleted when you need to identify whether to add the button or not.

Duplicate key bug You use:

{incompleteTodos.map((todo) => (
  <TodoItem
  key={todo.text}

What if the todo text is the same? Maybe someone adds "Get groceries" Tuesday, then comes back to the app on Thursday and adds "Get groceries" again for the next week. This means that you'll have duplicate mapped keys, which should not be done and will confuse React, possibly creating rendering problems.

Since it looks like todos have IDs, use those IDs as keys instead.

&& instead of conditional To conditionally render in React, you can use && or || to test, and put the element to be rendered on the right-hand side - it's a bit more concise than the conditional operator.

You could do something like:

// You could rename the definition of removeTodoRequest to removeTodo
// and markCompletedTodoRequest to markCompletedTodo
// so that you can use shorthand property names below:
const makeTodoItem = (todo: Todo) => (
  <TodoItem
    key={todo.id}
    {...{ todo, removeTodo, markCompletedTodo }}
  />
);
return (
  <h2>Incomplete TODOS</h2>
  {incompleteTodos.map(makeTodoItem)}
  <h2>Complete TODOS</h2>
  {completeTodos.map(makeTodoItem)}
); // don't forget this semicolon

type TodoProps = {
  todo: Todo;
  removeTodo: (id: string) => void;
  markCompletedTodo: (id: string) => void;
};

const TodoItem = ({ todo, removeTodo, markCompletedTodo }: TodoProps) => {
  return (
    <div>
      <h2>{todo.text}</h2>
      <button type="button" onClick={() => removeTodo(todo.id)}>
        Remove
      </button>
      {!todo.isCompleted && (
        <button type="button" onClick={() => markCompletedTodo(todo.id)}>
          Mark completed
        </button>
      )}
    </div>
  );
};