Skip to content

fix #190: make withUndoRedo sync with watchState instead of effect #207

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.

Sign up for GitHub

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

Merged
rainerhahnekamp merged 2 commits into from
Aug 11, 2025
Merged

fix #190: make withUndoRedo sync with watchState instead of effect #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40fd13f
fix #190: make withUndoRedo sync with watchState instead of effect
Aug 9, 2025
f6e2c16
make tests synchronous and remove fakeAsync and tick
Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 18 additions & 31 deletions libs/ngrx-toolkit/src/lib/with-undo-redo.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { computed, inject } from '@angular/core';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { TestBed } from '@angular/core/testing';
import {
patchState,
signalStore,
Expand Down Expand Up @@ -73,7 +73,7 @@ describe('withUndoRedo', () => {
});

describe('undo and redo', () => {
it('restores previous state for regular store key', fakeAsync(() => {
it('restores previous state for regular store key', () => {
TestBed.runInInjectionContext(() => {
const Store = signalStore(
withState(testState),
Expand All @@ -85,24 +85,22 @@ describe('withUndoRedo', () => {
);

const store = new Store();
tick(1);

store.updateTest(newValue);
tick(1);

expect(store.test()).toEqual(newValue);
expect(store.canUndo()).toBe(true);
expect(store.canRedo()).toBe(false);

store.undo();
tick(1);

expect(store.test()).toEqual('');
expect(store.canUndo()).toBe(false);
expect(store.canRedo()).toBe(true);
});
}));
});

it('restores previous state for regular store key and respects skip', fakeAsync(() => {
it('restores previous state for regular store key and respects skip', () => {
TestBed.runInInjectionContext(() => {
const Store = signalStore(
withState(testState),
Expand All @@ -114,30 +112,26 @@ describe('withUndoRedo', () => {
);

const store = new Store();
tick(1);

store.updateTest(newValue);
tick(1);

expect(store.test()).toEqual(newValue);

store.updateTest(newerValue);
tick(1);

store.undo();
tick(1);

expect(store.test()).toEqual(newValue);
expect(store.canUndo()).toBe(false);

store.undo();
tick(1);

// should not change
expect(store.test()).toEqual(newValue);
});
}));
});

it('undoes and redoes previous state for entity', fakeAsync(() => {
it('undoes and redoes previous state for entity', () => {
const Store = signalStore(
withEntities({ entity: type<{ id: string }>() }),
withMethods((store) => ({
Expand All @@ -149,19 +143,19 @@ describe('withUndoRedo', () => {
TestBed.configureTestingModule({ providers: [Store] });
TestBed.runInInjectionContext(() => {
const store = inject(Store);
tick(1);

expect(store.entities()).toEqual([]);
expect(store.canUndo()).toBe(false);
expect(store.canRedo()).toBe(false);

store.addEntity(newValue);
tick(1);

expect(store.entities()).toEqual([{ id: newValue }]);
expect(store.canUndo()).toBe(true);
expect(store.canRedo()).toBe(false);

store.addEntity(newerValue);
tick(1);

expect(store.entities()).toEqual([
{ id: newValue },
{ id: newerValue },
Expand All @@ -182,21 +176,20 @@ describe('withUndoRedo', () => {
expect(store.canRedo()).toBe(true);

store.redo();
tick(1);

expect(store.entities()).toEqual([{ id: newValue }]);
expect(store.canUndo()).toBe(true);
expect(store.canRedo()).toBe(true);

// should return canRedo=false after a change
store.addEntity('newest');
tick(1);

expect(store.canUndo()).toBe(true);
expect(store.canRedo()).toBe(false);
});
}));
});

it('restores previous state for named entity', fakeAsync(() => {
it('restores previous state for named entity', () => {
TestBed.runInInjectionContext(() => {
const Store = signalStore(
withEntities({
Expand All @@ -215,22 +208,20 @@ describe('withUndoRedo', () => {
);

const store = new Store();
tick(1);

store.addEntity(newValue);
tick(1);

expect(store.flightEntities()).toEqual([{ id: newValue }]);
expect(store.canUndo()).toBe(true);
expect(store.canRedo()).toBe(false);

store.undo();
tick(1);

expect(store.flightEntities()).toEqual([]);
expect(store.canUndo()).toBe(false);
expect(store.canRedo()).toBe(true);
});
}));
});

it('clears undo redo stack', () => {
const Store = signalStore(
Expand All @@ -253,7 +244,7 @@ describe('withUndoRedo', () => {
expect(store.canRedo()).toBe(false);
});

it('cannot undo after clearing and setting a new value', fakeAsync(() => {
it('cannot undo after clearing and setting a new value', () => {
const Store = signalStore(
{ providedIn: 'root' },
withState(testState),
Expand All @@ -266,22 +257,18 @@ describe('withUndoRedo', () => {
const store = TestBed.inject(Store);

store.update('Alan');
tick(1);

store.update('Gordon');
tick(1);

store.clearStack();
tick(1);

// After clearing the undo/redo stack, there is no previous item anymore.
// The following update becomes the first value.
// Since there is no other value before, it cannot be undone.
store.update('Hugh');
tick(1);

expect(store.canUndo()).toBe(false);
expect(store.canRedo()).toBe(false);
}));
});
});
});
5 changes: 3 additions & 2 deletions libs/ngrx-toolkit/src/lib/with-undo-redo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Signal, effect, isSignal, signal, untracked } from '@angular/core';
import { Signal, isSignal, signal, untracked } from '@angular/core';
import {
EmptyFeatureResult,
SignalStoreFeature,
SignalStoreFeatureResult,
patchState,
signalStoreFeature,
watchState,
withComputed,
withHooks,
withMethods,
Expand Down Expand Up @@ -143,7 +144,7 @@ export function withUndoRedo<Input extends EmptyFeatureResult>(
})),
withHooks({
onInit(store) {
effect(() => {
watchState(store, () => {
Copy link
Contributor

@GregOnNet GregOnNet Aug 11, 2025

Choose a reason for hiding this comment

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

This change is great and allows us to make all tests in with-und-redo.spec.ts synchronous.

Can we omit all fakeAsync- and tick-usages there?

const cand = keys.reduce((acc, key) => {
const s = (store as Record<string | keyof Input['state'], unknown>)[
key
Expand Down