Skip to content

Commit

Permalink
♻️ (typescript) fixing strictNullChecks=true issues (#2212)
Browse files Browse the repository at this point in the history
MatissJanis authored Jan 12, 2024
1 parent 8b96857 commit e205344
Showing 12 changed files with 124 additions and 67 deletions.
2 changes: 1 addition & 1 deletion packages/crdt/src/crdt/merkle.ts
Original file line number Diff line number Diff line change
@@ -93,7 +93,7 @@ export function diff(trie1: TrieNode, trie2: TrieNode): number | null {
const keys = [...keyset.values()];
keys.sort();

let diffkey = null;
let diffkey: null | '0' | '1' | '2' = null;

// Traverse down the trie through keys that aren't the same. We
// traverse down the keys in order. Stop in two cases: either one
3 changes: 2 additions & 1 deletion packages/loot-core/src/client/actions/queries.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import throttle from 'throttleit';

import { send } from '../../platform/client/fetch';
import { type AccountEntity } from '../../types/models';
import * as constants from '../constants';

import { pushModal } from './modals';
@@ -260,7 +261,7 @@ export function getAccounts() {
};
}

export function updateAccount(account) {
export function updateAccount(account: AccountEntity) {
return async (dispatch: Dispatch) => {
dispatch({ type: constants.UPDATE_ACCOUNT, account });
await send('account-update', account);
33 changes: 19 additions & 14 deletions packages/loot-core/src/client/reducers/queries.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import memoizeOne from 'memoize-one';

import { groupById } from '../../shared/util';
import { type AccountEntity, type PayeeEntity } from '../../types/models';
import * as constants from '../constants';
import type { Action } from '../state-types';
import type { QueriesState } from '../state-types/queries';
@@ -59,9 +60,7 @@ export function update(state = initialState, action: Action): QueriesState {
return {
...state,
accounts: state.accounts.map(account => {
// @ts-expect-error Not typed yet
if (account.id === action.account.id) {
// @ts-expect-error Not typed yet
return { ...account, ...action.account };
}
return account;
@@ -83,8 +82,12 @@ export function update(state = initialState, action: Action): QueriesState {
return state;
}

export const getAccountsById = memoizeOne(accounts => groupById(accounts));
export const getPayeesById = memoizeOne(payees => groupById(payees));
export const getAccountsById = memoizeOne((accounts: AccountEntity[]) =>
groupById(accounts),
);
export const getPayeesById = memoizeOne((payees: PayeeEntity[]) =>
groupById(payees),
);
export const getCategoriesById = memoizeOne(categoryGroups => {
const res = {};
categoryGroups.forEach(group => {
@@ -95,14 +98,16 @@ export const getCategoriesById = memoizeOne(categoryGroups => {
return res;
});

export const getActivePayees = memoizeOne((payees, accounts) => {
const accountsById = getAccountsById(accounts);
export const getActivePayees = memoizeOne(
(payees: PayeeEntity[], accounts: AccountEntity[]) => {
const accountsById = getAccountsById(accounts);

return payees.filter(payee => {
if (payee.transfer_acct) {
const account = accountsById[payee.transfer_acct];
return account != null && !account.closed;
}
return true;
});
});
return payees.filter(payee => {
if (payee.transfer_acct) {
const account = accountsById[payee.transfer_acct];
return account != null && !account.closed;
}
return true;
});
},
);
3 changes: 2 additions & 1 deletion packages/loot-core/src/client/state-types/queries.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Handlers } from '../../types/handlers';
import { type AccountEntity } from '../../types/models';
import type * as constants from '../constants';

export type QueriesState = {
@@ -41,7 +42,7 @@ type LoadAccountsAction = {

type UpdateAccountAction = {
type: typeof constants.UPDATE_ACCOUNT;
account: unknown;
account: AccountEntity;
};

type LoadCategoriesAction = {
4 changes: 2 additions & 2 deletions packages/loot-core/src/mocks/budget.ts
Original file line number Diff line number Diff line change
@@ -12,13 +12,13 @@ import { q } from '../shared/query';
import type { Handlers } from '../types/handlers';
import type {
CategoryGroupEntity,
PayeeEntity,
NewPayeeEntity,
NewTransactionEntity,
} from '../types/models';

import { random } from './random';

type MockPayeeEntity = PayeeEntity & { bill?: boolean };
type MockPayeeEntity = NewPayeeEntity & { bill?: boolean };

function pickRandom<T>(list: T[]): T {
return list[Math.floor(random() * list.length) % list.length];
11 changes: 8 additions & 3 deletions packages/loot-core/src/server/db/index.ts
Original file line number Diff line number Diff line change
@@ -12,7 +12,11 @@ import { v4 as uuidv4 } from 'uuid';
import * as fs from '../../platform/server/fs';
import * as sqlite from '../../platform/server/sqlite';
import { groupById } from '../../shared/util';
import { CategoryEntity, CategoryGroupEntity } from '../../types/models';
import {
CategoryEntity,
CategoryGroupEntity,
PayeeEntity,
} from '../../types/models';
import {
schema,
schemaConfig,
@@ -478,9 +482,10 @@ export function updatePayee(payee) {
return update('payees', payee);
}

export async function mergePayees(target, ids) {
export async function mergePayees(target: string, ids: string[]) {
// Load in payees so we can check some stuff
const payees = groupById(await all('SELECT * FROM payees'));
const dbPayees: PayeeEntity[] = await all('SELECT * FROM payees');
const payees = groupById(dbPayees);

// Filter out any transfer payees
if (payees[target].transfer_acct != null) {
58 changes: 37 additions & 21 deletions packages/loot-core/src/shared/transactions.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { v4 as uuidv4 } from 'uuid';

import { TransactionEntity } from '../types/models';

import {
splitTransaction,
updateTransaction,
@@ -8,16 +10,26 @@ import {
makeChild,
} from './transactions';

// const data = {
// splitTransactions: generateTransaction({ amount: -5000, acct: 2 }, -2000)
// };

function makeTransaction(data) {
function makeTransaction(data: Partial<TransactionEntity>): TransactionEntity {
return {
id: uuidv4(),
amount: 2422,
date: '2020-01-05',
account: 'acct1',
account: {
id: 'acc-id-1',
name: 'account-1',
offbudget: 0,
closed: 0,
sort_order: 1,
tombstone: 0,
account_id: null,
bank: null,
mask: null,
official_name: null,
balance_current: null,
balance_available: null,
balance_limit: null,
},
...data,
};
}
@@ -27,7 +39,7 @@ function makeSplitTransaction(data, children) {
return [parent, ...children.map(t => makeChild(parent, t))];
}

function splitError(amount) {
function splitError(amount: number) {
return { difference: amount, type: 'SplitTransactionError', version: 1 };
}

@@ -38,15 +50,18 @@ describe('Transactions', () => {
makeTransaction({ id: 't1', amount: 4000 }),
makeTransaction({ amount: 3000 }),
];
const { data, diff } = updateTransaction(transactions, {
id: 't1',
amount: 5000,
});
const { data, diff } = updateTransaction(
transactions,
makeTransaction({
id: 't1',
amount: 5000,
}),
);
expect(data.find(d => d.subtransactions)).toBeFalsy();
expect(diff).toEqual({
added: [],
deleted: [],
updated: [{ id: 't1', amount: 5000 }],
updated: [expect.objectContaining({ id: 't1', amount: 5000 })],
});
expect(data.map(t => ({ id: t.id, amount: t.amount })).sort()).toEqual([
{ id: expect.any(String), amount: 5000 },
@@ -56,14 +71,12 @@ describe('Transactions', () => {
});

test('updating does nothing if value not changed', () => {
const updatedTransaction = makeTransaction({ id: 't1', amount: 5000 });
const transactions = [
makeTransaction({ id: 't1', amount: 5000 }),
updatedTransaction,
makeTransaction({ amount: 3000 }),
];
const { data, diff } = updateTransaction(transactions, {
id: 't1',
amount: 5000,
});
const { data, diff } = updateTransaction(transactions, updatedTransaction);
expect(diff).toEqual({ added: [], deleted: [], updated: [] });
expect(data.map(t => ({ id: t.id, amount: t.amount })).sort()).toEqual([
{ id: expect.any(String), amount: 5000 },
@@ -160,10 +173,13 @@ describe('Transactions', () => {
]),
makeTransaction({ amount: 3002 }),
];
const { data, diff } = updateTransaction(transactions, {
id: 't2',
amount: 2200,
});
const { data, diff } = updateTransaction(
transactions,
makeTransaction({
id: 't2',
amount: 2200,
}),
);
expect(data.find(d => d.subtransactions)).toBeFalsy();
expect(diff).toEqual({
added: [],
31 changes: 21 additions & 10 deletions packages/loot-core/src/shared/transactions.ts
Original file line number Diff line number Diff line change
@@ -111,7 +111,9 @@ export function applyTransactionDiff(
groupedTrans: Parameters<typeof ungroupTransaction>[0],
diff: Parameters<typeof applyChanges>[0],
) {
return groupTransaction(applyChanges(diff, ungroupTransaction(groupedTrans)));
return groupTransaction(
applyChanges(diff, ungroupTransaction(groupedTrans) || []),
);
}

function replaceTransactions(
@@ -131,7 +133,7 @@ function replaceTransactions(
const parentIndex = findParentIndex(transactions, idx);
if (parentIndex == null) {
console.log('Cannot find parent index');
return { diff: { deleted: [], updated: [] } };
return { data: [], diff: { deleted: [], updated: [] } };
}

const split = getSplit(transactions, parentIndex);
@@ -177,8 +179,8 @@ export function addSplitTransaction(
if (!trans.is_parent) {
return trans;
}
const prevSub = last(trans.subtransactions);
trans.subtransactions.push(
const prevSub = last(trans.subtransactions || []);
trans.subtransactions?.push(
makeChild(trans, {
amount: 0,
sort_order: num(prevSub && prevSub.sort_order) - 1,
@@ -188,11 +190,14 @@ export function addSplitTransaction(
});
}

export function updateTransaction(transactions, transaction) {
export function updateTransaction(
transactions: TransactionEntity[],
transaction: TransactionEntity,
) {
return replaceTransactions(transactions, transaction.id, trans => {
if (trans.is_parent) {
const parent = trans.id === transaction.id ? transaction : trans;
const sub = trans.subtransactions.map(t => {
const sub = trans.subtransactions?.map(t => {
// Make sure to update the children to reflect the updated
// properties (if the parent updated)

@@ -216,20 +221,23 @@ export function updateTransaction(transactions, transaction) {
});
}

export function deleteTransaction(transactions, id) {
export function deleteTransaction(
transactions: TransactionEntity[],
id: string,
) {
return replaceTransactions(transactions, id, trans => {
if (trans.is_parent) {
if (trans.id === id) {
return null;
} else if (trans.subtransactions.length === 1) {
} else if (trans.subtransactions?.length === 1) {
return {
...trans,
subtransactions: null,
is_parent: false,
error: null,
};
} else {
const sub = trans.subtransactions.filter(t => t.id !== id);
const sub = trans.subtransactions?.filter(t => t.id !== id);
return recalculateSplit({ ...trans, subtransactions: sub });
}
} else {
@@ -238,7 +246,10 @@ export function deleteTransaction(transactions, id) {
});
}

export function splitTransaction(transactions, id) {
export function splitTransaction(
transactions: TransactionEntity[],
id: string,
) {
return replaceTransactions(transactions, id, trans => {
if (trans.is_parent || trans.is_child) {
return trans;
28 changes: 18 additions & 10 deletions packages/loot-core/src/shared/util.ts
Original file line number Diff line number Diff line change
@@ -2,13 +2,17 @@ export function last<T>(arr: Array<T>) {
return arr[arr.length - 1];
}

export function getChangedValues(obj1, obj2) {
// Keep the id field because this is mostly used to diff database
// objects
const diff = obj1.id ? { id: obj1.id } : {};
export function getChangedValues<T extends { id?: string }>(obj1: T, obj2: T) {
const diff: Partial<T> = {};
const keys = Object.keys(obj2);
let hasChanged = false;

// Keep the id field because this is mostly used to diff database
// objects
if (obj1.id) {
diff.id = obj1.id;
}

for (let i = 0; i < keys.length; i++) {
const key = keys[i];

@@ -21,7 +25,11 @@ export function getChangedValues(obj1, obj2) {
return hasChanged ? diff : null;
}

export function hasFieldsChanged(obj1, obj2, fields) {
export function hasFieldsChanged<T extends object>(
obj1: T,
obj2: T,
fields: Array<keyof T>,
) {
let changed = false;
for (let i = 0; i < fields.length; i++) {
const field = fields[i];
@@ -101,7 +109,7 @@ export function groupBy<T, K extends keyof T>(data: T[], field: K) {
// different API and we need to go through and update everywhere that
// uses it.
function _groupById<T extends { id: string }>(data: T[]) {
const res = new Map();
const res = new Map<string, T>();
for (let i = 0; i < data.length; i++) {
const item = data[i];
res.set(item.id, item);
@@ -112,8 +120,8 @@ function _groupById<T extends { id: string }>(data: T[]) {
export function diffItems<T extends { id: string }>(items: T[], newItems: T[]) {
const grouped = _groupById(items);
const newGrouped = _groupById(newItems);
const added = [];
const updated = [];
const added: T[] = [];
const updated: Partial<T>[] = [];

const deleted = items
.filter(item => !newGrouped.has(item.id))
@@ -135,7 +143,7 @@ export function diffItems<T extends { id: string }>(items: T[], newItems: T[]) {
}

export function groupById<T extends { id: string }>(data: T[]) {
const res = {};
const res: { [key: string]: T } = {};
for (let i = 0; i < data.length; i++) {
const item = data[i];
res[item.id] = item;
@@ -358,7 +366,7 @@ export function looselyParseAmount(amount: string) {
}

const m = amount.match(/[.,][^.,]*$/);
if (!m || m.index === 0) {
if (!m || m.index === undefined || m.index === 0) {
return safeNumber(parseFloat(extractNumbers(amount)));
}

10 changes: 6 additions & 4 deletions packages/loot-core/src/types/models/payee.d.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import type { AccountEntity } from './account';

export interface PayeeEntity {
export interface NewPayeeEntity {
id?: string;
name: string;
transfer_acct?: AccountEntity;
transfer_acct?: string;
tombstone?: boolean;
}

export interface PayeeEntity extends NewPayeeEntity {
id: string;
}
2 changes: 2 additions & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -15,6 +15,8 @@
"downlevelIteration": true,
// TODO: enable once every file is ts
// "strict": true,
// TODO: enable once all issues fixed
// "strictNullChecks": true,
"strictFunctionTypes": true,
"noFallthroughCasesInSwitch": true,
"skipLibCheck": true,
6 changes: 6 additions & 0 deletions upcoming-release-notes/2212.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [MatissJanis]
---

TypeScript: fix some `strictNullChecks: true` issues

0 comments on commit e205344

Please sign in to comment.