fix: prevent duplicate ids when updating id of an element with the id of an existing element #2020
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e86ed60:
|
✅ Deploy Preview for redux-starter-kit-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| @@ -158,7 +158,9 @@ export function createUnsortedStateAdapter<T>( | |||
| 0 | |||
|
|
|||
| if (didMutateIds) { | |||
| state.ids = state.ids.map((id) => newKeys[id] || id) | |||
| state.ids = Array.from( | |||
There was a problem hiding this comment.
The state.ids iteration is necessary? What do you think about:
state.ids = Object.keys(state.entities) ?
There was a problem hiding this comment.
I think that could work, actually. Worth a shot?
There was a problem hiding this comment.
Updated and all the tests are passing too
| @@ -158,7 +158,9 @@ export function createUnsortedStateAdapter<T>( | |||
| 0 | |||
|
|
|||
| if (didMutateIds) { | |||
| state.ids = state.ids.map((id) => newKeys[id] || id) | |||
| state.ids = Array.from( | |||
There was a problem hiding this comment.
I works well. Here's my suggestion:
| state.ids = Array.from( | |
| state.ids = Object.keys(state.entities) |
| @@ -158,7 +158,9 @@ export function createUnsortedStateAdapter<T>( | |||
| 0 | |||
|
|
|||
| if (didMutateIds) { | |||
| state.ids = state.ids.map((id) => newKeys[id] || id) | |||
| state.ids = Array.from( | |||
| new Set(state.ids.map((id) => newKeys[id] || id)) | |||
There was a problem hiding this comment.
| new Set(state.ids.map((id) => newKeys[id] || id)) |
| state.ids = state.ids.map((id) => newKeys[id] || id) | ||
| state.ids = Array.from( | ||
| new Set(state.ids.map((id) => newKeys[id] || id)) | ||
| ) |
markerikson
force-pushed
the
fix/duplicate-ids-in-create-entity-adapter
branch
from
a4e7fa3 to
e86ed60
Compare
|
Just rebased this and updated the tests. Thanks! |
Fixes #2018
Think I'll probably have to update the sorted_state adapter too, but I'm unfamiliar with redux-toolkit and will need to take a closer look. Any pointers would be great 😛. Also let me know if it's too "expensive" to create a set from the array of ids and then changing it back to an array again.