Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(kitchen-sink): update dependencies and fix a key issue in the todo list #136

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

theborowski
Copy link

@theborowski theborowski commented Dec 17, 2024

changes

  • bump react and react-dom from the RC version to the official 19.0.0 release
  • remove unused babel-plugin-react-compiler dependency
  • move vite related dependencies into devDependencies
  • fix a tiny bug where adding two of the same text items in the list would result in a key error
  • prevent adding an item to the list with the enter key if it's text is blank

visual diff

Screen.Recording.2024-12-17.at.8.09.28.PM.mov

Copy link

vercel bot commented Dec 17, 2024

@theborowski is attempting to deploy a commit to the Million Team on Vercel.

A member of the Team first needs to authorize it.

{tasks.map((task) => (
<TaskItem key={task} task={task} onDelete={onDelete} />
{tasks.map((task, i) => (
<TaskItem key={`${task}_${i}`} task={task} onDelete={onDelete} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly this is the same even if we remove the key prop. Needs an alternative "id generation"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think just using a timestamp here as an "id" is sufficient?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need something like

interface Task {
  id: string;
  text: string;
}

and then use id as the key.

Copy link
Author

@theborowski theborowski Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored this to use an object as you suggested. This fixes the key problem as well as improves deletion with using the id of the task. I also moved the task creation in a function so that clicking and pressing enter have the same behavior. Added a little clip of the app working with these changes in the PR description.

@theborowski theborowski requested a review from lxsmnsyc December 18, 2024 03:08
@RobPruzan RobPruzan force-pushed the main branch 2 times, most recently from 7526d13 to 47c46ca Compare December 21, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants