From 8ee4768f58181652e232d908c7394e634121c347 Mon Sep 17 00:00:00 2001 From: Matiss Janis Aboltins <matiss@mja.lv> Date: Sat, 5 Aug 2023 21:02:14 +0100 Subject: [PATCH] :recycle: (crdt) adding more strict typings (#1461) Making the `crdt` package fully TypeScript-strict. --- packages/crdt/package.json | 2 +- packages/crdt/src/crdt/merkle.test.ts | 44 +++++++++++++-------------- packages/crdt/src/crdt/merkle.ts | 33 +++++++++++++++----- packages/crdt/src/crdt/timestamp.ts | 15 ++++++++- packages/crdt/tsconfig.dist.json | 3 +- upcoming-release-notes/1461.md | 6 ++++ 6 files changed, 69 insertions(+), 34 deletions(-) create mode 100644 upcoming-release-notes/1461.md diff --git a/packages/crdt/package.json b/packages/crdt/package.json index 423b504bf..4e7f39676 100644 --- a/packages/crdt/package.json +++ b/packages/crdt/package.json @@ -1,6 +1,6 @@ { "name": "@actual-app/crdt", - "version": "2.0.2", + "version": "2.1.0", "license": "MIT", "description": "CRDT layer of Actual", "main": "dist/index.js", diff --git a/packages/crdt/src/crdt/merkle.test.ts b/packages/crdt/src/crdt/merkle.test.ts index 21f02d571..48f300f45 100644 --- a/packages/crdt/src/crdt/merkle.test.ts +++ b/packages/crdt/src/crdt/merkle.test.ts @@ -55,7 +55,7 @@ describe('merkle trie', () => { trie2 = merkle.insert(trie2, messages[4].timestamp); expect(trie2.hash).toBe(108); - expect(new Date(merkle.diff(trie1, trie2)).toISOString()).toBe( + expect(new Date(merkle.diff(trie1, trie2)!).toISOString()).toBe( '2018-11-02T17:15:00.000Z', ); @@ -126,10 +126,10 @@ describe('merkle trie', () => { // Case 0: It always returns a base time when comparing with an // empty trie - expect(new Date(merkle.diff(merkle.emptyTrie(), trie)).toISOString()).toBe( + expect(new Date(merkle.diff(merkle.emptyTrie(), trie)!).toISOString()).toBe( '1970-01-01T00:00:00.000Z', ); - expect(new Date(merkle.diff(trie, merkle.emptyTrie())).toISOString()).toBe( + expect(new Date(merkle.diff(trie, merkle.emptyTrie())!).toISOString()).toBe( '1970-01-01T00:00:00.000Z', ); @@ -140,52 +140,52 @@ describe('merkle trie', () => { message('2018-11-01T00:59:00.000Z-0000-0123456789ABCDEF', 900), ]); - // Normal comparision works - expect(new Date(merkle.diff(trie1, trie)).toISOString()).toBe( + // Normal comparison works + expect(new Date(merkle.diff(trie1, trie)!).toISOString()).toBe( '2018-11-01T00:54:00.000Z', ); // Comparing the pruned new trie is lossy, so it returns an even older time - expect(new Date(merkle.diff(merkle.prune(trie1), trie)).toISOString()).toBe( - '2018-11-01T00:45:00.000Z', - ); + expect( + new Date(merkle.diff(merkle.prune(trie1), trie)!).toISOString(), + ).toBe('2018-11-01T00:45:00.000Z'); // Comparing the pruned original trie is just as lossy - expect(new Date(merkle.diff(trie1, merkle.prune(trie))).toISOString()).toBe( - '2018-11-01T00:45:00.000Z', - ); + expect( + new Date(merkle.diff(trie1, merkle.prune(trie))!).toISOString(), + ).toBe('2018-11-01T00:45:00.000Z'); // Pruning both tries is just as lossy as well, since the changed // key is pruned away in both cases and it won't find a changed // key so it bails at the point expect( new Date( - merkle.diff(merkle.prune(trie1), merkle.prune(trie)), + merkle.diff(merkle.prune(trie1), merkle.prune(trie))!, ).toISOString(), ).toBe('2018-11-01T00:45:00.000Z'); // Case 2: Add two messages similar to the above case, but the // second message modifies the 2nd key at the same level as the - // first message modifiying the 1st key + // first message modifying the 1st key let trie2 = insertMessages(trie, [ message('2018-11-01T00:59:00.000Z-0000-0123456789ABCDEF', 900), message('2018-11-01T01:15:00.000Z-0000-0123456789ABCDEF', 1422), ]); - // Normal comparision works - expect(new Date(merkle.diff(trie2, trie)).toISOString()).toBe( + // Normal comparison works + expect(new Date(merkle.diff(trie2, trie)!).toISOString()).toBe( '2018-11-01T00:54:00.000Z', ); // Same as case 1 - expect(new Date(merkle.diff(merkle.prune(trie2), trie)).toISOString()).toBe( - '2018-11-01T00:45:00.000Z', - ); + expect( + new Date(merkle.diff(merkle.prune(trie2), trie)!).toISOString(), + ).toBe('2018-11-01T00:45:00.000Z'); // Same as case 1 - expect(new Date(merkle.diff(trie2, merkle.prune(trie))).toISOString()).toBe( - '2018-11-01T00:45:00.000Z', - ); + expect( + new Date(merkle.diff(trie2, merkle.prune(trie))!).toISOString(), + ).toBe('2018-11-01T00:45:00.000Z'); // Pruning both tries is very lossy and this ends up returning a // time that only covers the second message. Syncing will need @@ -194,7 +194,7 @@ describe('merkle trie', () => { // ignores the first message. expect( new Date( - merkle.diff(merkle.prune(trie2), merkle.prune(trie)), + merkle.diff(merkle.prune(trie2), merkle.prune(trie))!, ).toISOString(), ).toBe('2018-11-01T01:12:00.000Z'); }); diff --git a/packages/crdt/src/crdt/merkle.ts b/packages/crdt/src/crdt/merkle.ts index ca987bf81..3413a471b 100644 --- a/packages/crdt/src/crdt/merkle.ts +++ b/packages/crdt/src/crdt/merkle.ts @@ -19,12 +19,18 @@ export type TrieNode = { hash?: number; }; +type NumberTrieNodeKey = keyof Omit<TrieNode, 'hash'>; + export function emptyTrie(): TrieNode { return { hash: 0 }; } -export function getKeys(trie: TrieNode): ('0' | '1' | '2')[] { - return Object.keys(trie).filter(x => x !== 'hash') as ('0' | '1' | '2')[]; +function isNumberTrieNodeKey(input: string): input is NumberTrieNodeKey { + return ['0', '1', '2'].includes(input); +} + +export function getKeys(trie: TrieNode): NumberTrieNodeKey[] { + return Object.keys(trie).filter(isNumberTrieNodeKey); } export function keyToTimestamp(key: string): number { @@ -43,7 +49,7 @@ export function insert(trie: TrieNode, timestamp: Timestamp) { let hash = timestamp.hash(); let key = Number(Math.floor(timestamp.millis() / 1000 / 60)).toString(3); - trie = Object.assign({}, trie, { hash: trie.hash ^ hash }); + trie = Object.assign({}, trie, { hash: (trie.hash || 0) ^ hash }); return insertKey(trie, key, hash); } @@ -52,10 +58,11 @@ function insertKey(trie: TrieNode, key: string, hash: number): TrieNode { return trie; } const c = key[0]; - const n = trie[c] || {}; + const t = isNumberTrieNodeKey(c) ? trie[c] : undefined; + const n = t || {}; return Object.assign({}, trie, { [c]: Object.assign({}, n, insertKey(n, key.slice(1), hash), { - hash: n.hash ^ hash, + hash: (n.hash || 0) ^ hash, }), }); } @@ -68,7 +75,7 @@ export function build(timestamps: Timestamp[]) { return trie; } -export function diff(trie1: TrieNode, trie2: TrieNode): number { +export function diff(trie1: TrieNode, trie2: TrieNode): number | null { if (trie1.hash === trie2.hash) { return null; } @@ -126,6 +133,8 @@ export function diff(trie1: TrieNode, trie2: TrieNode): number { node1 = node1[diffkey] || emptyTrie(); node2 = node2[diffkey] || emptyTrie(); } + + return null; } export function prune(trie: TrieNode, n = 2): TrieNode { @@ -141,7 +150,13 @@ export function prune(trie: TrieNode, n = 2): TrieNode { // Prune child nodes. for (let k of keys.slice(-n)) { - next[k] = prune(trie[k], n); + const node = trie[k]; + + if (!node) { + throw new Error(`TrieNode for key ${k} could not be found`); + } + + next[k] = prune(node, n); } return next; @@ -156,7 +171,9 @@ export function debug(trie: TrieNode, k = '', indent = 0): string { str + getKeys(trie) .map(key => { - return debug(trie[key], key, indent + 2); + const node = trie[key]; + if (!node) return ''; + return debug(node, key, indent + 2); }) .join('') ); diff --git a/packages/crdt/src/crdt/timestamp.ts b/packages/crdt/src/crdt/timestamp.ts index 1ff88aeaa..ff2fb3496 100644 --- a/packages/crdt/src/crdt/timestamp.ts +++ b/packages/crdt/src/crdt/timestamp.ts @@ -64,8 +64,14 @@ export function deserializeClock(clock: string): Clock { }; } + const ts = Timestamp.parse(data.timestamp); + + if (!ts) { + throw new Timestamp.InvalidError(data.timestamp); + } + return { - timestamp: MutableTimestamp.from(Timestamp.parse(data.timestamp)), + timestamp: MutableTimestamp.from(ts), merkle: data.merkle, }; } @@ -320,6 +326,13 @@ export class Timestamp { this.name = 'OverflowError'; } }; + + static InvalidError = class InvalidError extends Error { + constructor(...args: unknown[]) { + super(['timestamp is not valid'].concat(args.map(String)).join(' ')); + this.name = 'InvalidError'; + } + }; } class MutableTimestamp extends Timestamp { diff --git a/packages/crdt/tsconfig.dist.json b/packages/crdt/tsconfig.dist.json index 25b6118e0..d17f9341b 100644 --- a/packages/crdt/tsconfig.dist.json +++ b/packages/crdt/tsconfig.dist.json @@ -7,8 +7,7 @@ "module": "CommonJS", "noEmit": false, "declaration": true, - // TODO: enable - // "strict": true, + "strict": true, "outDir": "dist" }, "include": ["."], diff --git a/upcoming-release-notes/1461.md b/upcoming-release-notes/1461.md new file mode 100644 index 000000000..5a7efcaef --- /dev/null +++ b/upcoming-release-notes/1461.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [MatissJanis] +--- + +crdt: making the package fully TypeScript strict -- GitLab