Skip to content

Conversation

@damiensedgwick
Copy link
Contributor

@damiensedgwick damiensedgwick commented Oct 6, 2025

This merge request fixes an issue where images were being duplicated when navigating indefinitely. This I think is down to how React reconciles elements that do not have unique keys.

The fix is to use the note image id for the map keys so that the uniqueness required, exists. I have also added additional code to ensure that when seeding the database, a note is not created using the same image multiple times. The second bit might be beyond the scope of this so I am happy to remove it, I've included it as a nice to have (in my opinion).

Tests have not been updated but no functionality has been lost although I don't believe this area of the code is currently covered.

Test Plan

A bit of a tedious one to test because a new project is needed with a seeded database where a note is created with the same image being used multiple times. Once you have this, you will be able to navigate back and forth and see the issue as shown in #1046

Pulling down my changes, you will see that this issue is no longer happening as we have swapped out using the objectKey for the map key in favour of the image id.

Checklist

  • Tests updated
  • Docs updated

Screenshots

Before:

Screen.Recording.2025-10-06.at.21.19.00.mp4

After:

Screen.Recording.2025-10-06.at.21.19.44.mp4

Note

Switch note image list keys to image.id and update seeding to select unique images per note.

  • Notes page (app/routes/users+/$username_+/notes.$noteId.tsx):
    • Loader now selects images.id.
    • Image list key changed from image.objectKey to image.id.
  • Seeding (prisma/seed.ts):
    • Select unique images per note via faker.helpers.arrayElements and create with Promise.all to avoid duplicates.

Written by Cursor Bugbot for commit 670ba5f. This will update automatically on new commits. Configure here.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Nice 👍 Thanks!

@kentcdodds kentcdodds merged commit 833a057 into epicweb-dev:main Oct 7, 2025
5 checks passed
const selectedImages = faker.helpers.arrayElements(
noteImages,
noteImageCount,
)
Copy link

Choose a reason for hiding this comment

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

Bug: Array Element Count Mismatch

The faker.helpers.arrayElements call uses a direct number for the count, which, while accepted, can lead to unexpected behavior if noteImages has fewer elements than noteImageCount. This may result in fewer images being created than intended.

Fix in Cursor Fix in Web

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