Skip to content
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

Jump to bottom

fix: update history when a state property is set to undefined #14

Merged
merged 9 commits into from
Apr 28, 2024
Merged

fix: update history when a state property is set to undefined #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
be83807
fix: update history when a state property is set to undefined
Apr 24, 2024
64ff0ac
use type for empty wip snapshot
Apr 24, 2024
3068c23
simplify test case
Apr 25, 2024
cbccfa1
add test case to check behavior with delete keyword
Apr 25, 2024
fe0d61c
move tests to vanilla suite
Apr 28, 2024
1239a3d
use symbol for empty history wip
Apr 28, 2024
9a8e550
avoid updating version
Apr 28, 2024
e881a74
remove unused import
Apr 28, 2024
59e6c6a
apply formatting
Apr 28, 2024
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
48 changes: 48 additions & 0 deletions packages/history-utility/src/__tests__/history-utility.vanilla.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,54 @@ describe('proxyWithHistory: vanilla', () => {
});
});

describe('edge cases', () => {
it('should update history when property is deleted', async () => {
const state = proxyWithHistory<
Partial<{ prop0: number; prop1: number; prop2: number; prop3: number }>
>({ prop0: 0, prop1: 1, prop2: 2, prop3: 3 });

await Promise.resolve();
expect(state.value.prop0).toEqual(0);
expect(state.value.prop1).toEqual(1);
expect(state.value.prop2).toEqual(2);
expect(state.value.prop3).toEqual(3);
expect(state.history.nodes.length).toEqual(1);

delete state.value.prop0;

await Promise.resolve();

expect(state.value.prop0).toEqual(undefined);
expect(state.value.prop1).toEqual(1);
expect(state.value.prop2).toEqual(2);
expect(state.value.prop3).toEqual(3);
expect(state.history.nodes.length).toEqual(2);
});

it('should update history when property is set to undefined', async () => {
const state = proxyWithHistory<
Partial<{ prop0: number; prop1: number; prop2: number; prop3: number }>
>({ prop0: 0, prop1: 1, prop2: 2, prop3: 3 });

await Promise.resolve();
expect(state.value.prop0).toEqual(0);
expect(state.value.prop1).toEqual(1);
expect(state.value.prop2).toEqual(2);
expect(state.value.prop3).toEqual(3);
expect(state.history.nodes.length).toEqual(1);

state.value.prop0 = undefined;

await Promise.resolve();

expect(state.value.prop0).toEqual(undefined);
expect(state.value.prop1).toEqual(1);
expect(state.value.prop2).toEqual(2);
expect(state.value.prop3).toEqual(3);
expect(state.history.nodes.length).toEqual(2);
});
});

describe('remove', () => {
it('should remove no items in history when only one item', async () => {
const state = proxyWithHistory({ count: 0 });
Expand Down
23 changes: 14 additions & 9 deletions packages/history-utility/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ export type HistoryNode<T> = {
updatedAt?: Date;
};

const EMPTY_WIP = Symbol('valtio-history-wip-empty');

export type History<T> = {
/**
* field for holding sandbox changes; used to avoid infinite loops
*/
wip?: Snapshot<T>;
wip: Snapshot<T> | typeof EMPTY_WIP;
/**
* the nodes of the history for each change
*/
Expand Down Expand Up @@ -134,7 +136,7 @@ export function proxyWithHistory<V>(
* - history.wip: field for holding sandbox changes; used to avoid infinite loops<br>
*/
history: ref<History<V>>({
wip: undefined, // to avoid infinite loop
wip: EMPTY_WIP, // to avoid infinite loop
nodes: [],
index: -1,
}),
Expand Down Expand Up @@ -228,9 +230,10 @@ export function proxyWithHistory<V>(
*/
undo: () => {
if (proxyObject.canUndo()) {
proxyObject.history.wip = proxyObject.clone(
proxyObject.history.nodes[--proxyObject.history.index]?.snapshot
);
proxyObject.history.wip =
proxyObject.clone(
proxyObject.history.nodes[--proxyObject.history.index]?.snapshot
) ?? EMPTY_WIP;
proxyObject.value = proxyObject.history.wip as V;
}
},
Expand All @@ -247,9 +250,10 @@ export function proxyWithHistory<V>(
*/
redo: () => {
if (proxyObject.canRedo()) {
proxyObject.history.wip = proxyObject.clone(
proxyObject.history.nodes[++proxyObject.history.index]?.snapshot
);
proxyObject.history.wip =
proxyObject.clone(
proxyObject.history.nodes[++proxyObject.history.index]?.snapshot
) ?? EMPTY_WIP;
proxyObject.value = proxyObject.history.wip as V;
}
},
Expand Down Expand Up @@ -308,7 +312,8 @@ export function proxyWithHistory<V>(
const resolvedIndex = isLastIndex ? index - 1 : index + 1;
const resolvedNode = proxyObject.history.nodes[resolvedIndex];

proxyObject.history.wip = proxyObject.clone(resolvedNode?.snapshot);
proxyObject.history.wip =
proxyObject.clone(resolvedNode?.snapshot) ?? EMPTY_WIP;
proxyObject.value = proxyObject.history.wip as V;

if (isLastIndex) proxyObject.history.index--;
Expand Down
Loading