Skip to content

Commit

Permalink
fix: making a change, going offline, then making another change to a …
Browse files Browse the repository at this point in the history
…different field of the same row was saving the value twice
  • Loading branch information
jmeistrich committed Nov 4, 2024
1 parent 339f7e8 commit d2ef82e
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/sync/syncObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -974,18 +974,19 @@ export function syncObservable<T>(
if (pending && !isEmpty(pending)) {
localState.isApplyingPending = true;
const keys = Object.keys(pending);
const value = getNodeValue(node);

// Bundle up all the changes from pending
const changes: Change[] = [];
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
const path = key.split('/').filter((p) => p !== '');
const { p, v, t } = pending[key];
changes.push({ path, valueAtPath: v, prevAtPath: p, pathTypes: t });
const { p, t } = pending[key];
const valueAtPath = getValueAtPath(value, path);
changes.push({ path, valueAtPath, prevAtPath: p, pathTypes: t });
}

// Send the changes into onObsChange so that they get synced remotely
const value = getNodeValue(node);
onObsChange(obs$, syncState$, localState, syncOptions, {
value,
isFromPersist: false,
Expand Down Expand Up @@ -1276,9 +1277,8 @@ export function syncObservable<T>(
}
if (!isSynced) {
isSynced = true;
// Wait for remote to be ready before saving pending
await when(syncState$.isLoaded);

// Apply pending to queue it up to sync
applyPending(pending);
}
};
Expand Down
68 changes: 68 additions & 0 deletions tests/crud.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { observable, ObservableHint, observe, syncState, when } from '@legendapp/state';
import { syncObservable } from '@legendapp/state/sync';
import { syncedCrud, SyncedCrudPropsMany, SyncedCrudPropsSingle } from '@legendapp/state/sync-plugins/crud';
import { expectTypeOf } from 'expect-type';
import { clone, getNode, symbolDelete } from '../src/globals';
Expand Down Expand Up @@ -3083,4 +3084,71 @@ describe('Misc', () => {
expect(didCreateRun).toEqual(true);
expect(numLists).toEqual(1);
});
test('pending when a change is made before sync', async () => {
const persistName = getPersistName();
localStorage.setItem(persistName, '{"id":"id1","test":"hi"}');
localStorage.setItem(
persistName + '__m',
'{"pending":{"":{"p":{"id":"id1","test":"hello"},"t":[],"v":{"id":"id1","test":"hi"}}}}',
);
const obs$ = observable<{ test: string }>({ test: '' });
const canSync$ = observable(false);
const saved: { id: string; test: string }[] = [];
syncObservable(
obs$,
syncedCrud({
get: () => {
if (canSync$.get()) {
return { id: 'id1', test: 'test' };
} else {
throw new Error('test error');
}
},
update: async (value: any) => {
saved.push(value);
},
persist: {
name: persistName,
plugin: ObservablePersistLocalStorage,
retrySync: true,
},
retry: {
delay: 1,
times: 2,
},
mode: 'merge',
}),
);

expect(obs$.get()).toEqual({ id: 'id1', test: 'hi' });

obs$.test.set('hi2');

await promiseTimeout(0);

// Setting hi2 should have cached it to pending
expect(localStorage.getItem(persistName + '__m')!).toEqual(
JSON.stringify({
pending: {
'': {
p: { id: 'id1', test: 'hello' },
t: [],
v: { test: 'hi2', id: 'id1' },
},
},
}),
);

await promiseTimeout(1);

// It should not save until it's loaded
expect(saved.length).toEqual(0);

canSync$.set(true);

await promiseTimeout(1);

// Once loaded it should save it once
expect(saved).toEqual([{ id: 'id1', test: 'hi2' }]);
});
});

0 comments on commit d2ef82e

Please sign in to comment.