Skip to content

Commit a6c199f

Browse files
authored
fix(data): immutably delete an entity (#3040)
Closes #2553
1 parent bf2bd1a commit a6c199f

File tree

3 files changed

+39
-11
lines changed

3 files changed

+39
-11
lines changed

modules/data/spec/reducers/entity-change-tracker-base.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,22 @@ describe('EntityChangeTrackerBase', () => {
580580
const collection = tracker.trackDeleteMany(['1234', 456], origCollection);
581581
expect(collection).toBe(origCollection);
582582
});
583+
584+
it('should not mutate changeState when called on a tracked "updated" entity', () => {
585+
const existingEntity = getFirstExistingEntity();
586+
const updatedEntity = toUpdate({
587+
...existingEntity,
588+
name: 'test update',
589+
});
590+
const collection = tracker.trackUpdateOne(updatedEntity, origCollection);
591+
const change = collection.changeState[existingEntity!.id];
592+
expect(change).toBeDefined();
593+
expectChangeType(change, ChangeType.Updated);
594+
Object.freeze(change);
595+
expect(() => {
596+
tracker.trackDeleteMany([existingEntity!.id], collection);
597+
}).not.toThrowError();
598+
});
583599
});
584600

585601
describe('#trackUpdateOne', () => {

modules/data/src/reducers/entity-change-tracker-base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ export class EntityChangeTrackerBase<T> implements EntityChangeTracker<T> {
454454
} else if (trackedChange.changeType === ChangeType.Updated) {
455455
// Special case: switch change type from Updated to Deleted.
456456
cloneChgStateOnce();
457-
trackedChange.changeType = ChangeType.Deleted;
457+
chgState[id] = { ...chgState[id], changeType: ChangeType.Deleted };
458458
}
459459
} else {
460460
// Start tracking this entity

modules/store/spec/runtime_checks.spec.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,6 @@ describe('Runtime checks:', () => {
138138
flush();
139139
}).not.toThrow();
140140
}));
141-
142-
it('should not throw for NgRx actions', fakeAsync(() => {
143-
const store = setupStore({ strictStateSerializability: true });
144-
145-
expect(() => {
146-
store.dispatch(makeNgrxAction(invalidAction()));
147-
flush();
148-
}).not.toThrow();
149-
}));
150141
});
151142

152143
describe('Action Serialization:', () => {
@@ -172,6 +163,15 @@ describe('Runtime checks:', () => {
172163
flush();
173164
}).not.toThrow();
174165
}));
166+
167+
it('should not throw for NgRx actions', fakeAsync(() => {
168+
const store = setupStore({ strictActionSerializability: true });
169+
170+
expect(() => {
171+
store.dispatch(makeNgrxAction(invalidAction()));
172+
flush();
173+
}).not.toThrow();
174+
}));
175175
});
176176

177177
describe('State Mutations', () => {
@@ -338,19 +338,31 @@ function reducerWithBugs(state: any = {}, action: any) {
338338
invalidSerializationState: true,
339339
invalid: new Date(),
340340
};
341-
341+
342342
case ErrorTypes.UnserializableAction: {
343343
return {
344344
invalidSerializationAction: true,
345345
};
346346
}
347+
348+
case '@ngrx ' + ErrorTypes.UnserializableAction: {
349+
return {
350+
invalidSerializationAction: true,
351+
};
352+
}
347353

348354
case ErrorTypes.MutateAction: {
349355
action.foo = 'foo';
350356
return {
351357
invalidMutationAction: true,
352358
};
353359
}
360+
case '@ngrx ' + ErrorTypes.MutateAction: {
361+
action.foo = 'foo';
362+
return {
363+
invalidMutationAction: true,
364+
};
365+
}
354366

355367
case ErrorTypes.MutateState: {
356368
state.invalidMutationState = true;

0 commit comments

Comments
 (0)