updateMany ignores early updates if later updates have the same ID

[jakeboone02]

The updateMany action generated by the createEntityAdapter function is not working as intended when there are multiple updates to the same entity.

Consider the following code:

const slice = createSlice({
  name: 'books',
  initialState: booksAdapter.getInitialState(),
  reducers: {
    bookAdd: adapter.addOne,
    bookUpdates: adapter.updateMany,
  },
});

// configure store and add books ...

store.dispatch(
  slice.actions.bookUpdates([
    { id: 1, changes: { title: 'New Title 1' } },
    { id: 2, changes: { title: 'New Title 2' } },
    { id: 1, changes: { author: 'New Author 1' } },
    { id: 2, changes: { author: 'New Author 2' } },
  ])
);

The outcome of that last statement should be that entity 1 has a title of "New Title 1" and a new author of "New Author 1". But what actually happens is that the author is changed but not the title.

The docs say, "If updateMany() is called with multiple updates targeted to the same ID, they will be merged into a single update, with later updates overwriting the earlier ones." That's a little ambiguous. It could mean that the later updates should be merged with earlier updates (not conflicting if the fields are different), or that the later updates would completely replace earlier updates with the same ID. It's doing the latter, but should do the former.

See the codesandbox here:

https://codesandbox.io/s/redux-toolkit-updatemany-test-verzl?file=/src/App.tsx

[markerikson]

Yeah, I think this is an actual bug. Here's the offending bit of code:

redux-toolkit/src/entities/unsorted_state_adapter.ts

Lines 106 to 117 in 9a410e7

	updates.forEach(update => {
	// Only apply updates to entities that currently exist
	if (update.id in state.entities) {
	// If there are multiple updates to one entity, merge them together
	updatesPerEntity[update.id] = {
	// Spreads ignore falsy values, so this works even if there isn't
	// an existing update already at this key
	...updatesPerEntity[update.id],
	...update
	}
	}
	})

The issue is that the updates are nested in update.changes, and the spread there isn't merging that nested field together.

I'm busy atm, but I'd happily take a PR and some unit tests to fix this.

[jakeboone02]

Thanks for taking a look, @markerikson. I've opened PR #621 to fix it.

[markerikson]

Live in https://github.com/reduxjs/redux-toolkit/releases/tag/v1.4.0

[karlsoderback]

I am using redux-toolkit v1.7.0 and I am still experiencing issues with several updates of the same id. I experience the reverse problem of what is described in the example above, the first update for the id takes effect but not the following.

[markerikson]

@karlsoderback if you're seeing problems, could you file a new issue with a CodeSandbox or a project that reproduces the problem? (fwiw we haven't changed any of this behavior since this issue was closed.)

[jakeboone02]

I tried to reproduce the issue that @karlsoderback reported here, but it appears to be working fine, or at least working as expected.

[karlsoderback]

Thanks for your replies, I'll put together a reproducer as soon as I can!

[karlsoderback]

I refactored the sandbox provided by @jakeboone02 here to closer reflect my project where the issue is experienced. In this example you should see the effect I described.

[markerikson]

@karlsoderback If you're actually seeing a bug, please open up a new issue so we can keep track of it.

[jakeboone02]

@karlsoderback When I remove the sortComparer declaration in the createEntityAdapter call, the updates work as expected: the second one takes priority. See here.

@markerikson I can't tell what's going on here because the updateManyMutably function doesn't seem to have anything to do with sortComparer (even after being wrapped by createStateOperator). Probably a new bug, but maybe his sortComparer is configured incorrectly?

[markerikson]

new issue please :)

[karlsoderback]

Posted in new issue: #1853