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.

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

TypeScript migration (partial). #1671

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion packages/desktop-client/src/components/AnimatedRefresh.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ export default function AnimatedRefresh({
}: AnimatedRefreshProps) {
return (
<View
style={{ animation: animating ? `${spin} 1s infinite linear` : null }}
style={{
animation: animating ? `${spin} 1s infinite linear` : undefined,
}}
>
<Refresh width={14} height={14} style={iconStyle} />
</View>
Expand Down
4 changes: 3 additions & 1 deletion packages/desktop-client/src/components/FatalError.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ function FatalError({ buttonText, error }: FatalErrorProps) {
>
{showSimpleRender ? <RenderSimple error={error} /> : <RenderUIError />}
<Paragraph>
<Button onClick={() => window.Actual.relaunch()}>{buttonText}</Button>
<Button onClick={() => window.Actual?.relaunch()}>
{buttonText}
</Button>
</Paragraph>
<Paragraph isLast={true} style={{ fontSize: 11 }}>
<LinkButton onClick={() => setShowError(true)}>Show Error</LinkButton>
Expand Down
6 changes: 3 additions & 3 deletions packages/desktop-client/src/components/LoggedInUser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function LoggedInUser({

async function onChangePassword() {
await closeBudget();
window.__navigate('/change-password');
window.__navigate?.('/change-password');
}

async function onMenuSelect(type) {
Expand All @@ -45,14 +45,14 @@ export default function LoggedInUser({
break;
case 'sign-in':
await closeBudget();
window.__navigate('/login');
window.__navigate?.('/login');
break;
case 'sign-out':
signOut();
break;
case 'config-server':
await closeBudget();
window.__navigate('/config-server');
window.__navigate?.('/config-server');
break;
default:
}
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop-client/src/components/Notifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function compileMessage(
if (actions[actionName]) {
setLoading(true);
await actions[actionName]();
onRemove();
onRemove?.();
}
}}
>
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop-client/src/components/alerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Text from './common/Text';
import View from './common/View';

type AlertProps = {
icon?: ComponentType<{ width?: number; style?: CSSProperties }>;
icon: ComponentType<{ width?: number; style?: CSSProperties }>;
color?: string;
backgroundColor?: string;
style?: CSSProperties;
Expand Down
6 changes: 5 additions & 1 deletion packages/desktop-client/src/components/common/MenuButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import DotsHorizontalTriple from '../../icons/v1/DotsHorizontalTriple';

import Button from './Button';

export default function MenuButton({ onClick }) {
type MenuButtonProps = {
onClick: () => void;
};

export default function MenuButton({ onClick }: MenuButtonProps) {
return (
<Button type="bare" onClick={onClick} aria-label="Menu">
<DotsHorizontalTriple
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default function ConfigServer() {
let currentUrl = useServerURL();
let setServerUrl = useSetServerURL();
useEffect(() => {
setUrl(currentUrl);
setUrl(currentUrl || '');
}, [currentUrl]);
let [loading, setLoading] = useState(false);
let [error, setError] = useState<string | null>(null);
Expand Down Expand Up @@ -77,15 +77,15 @@ export default function ConfigServer() {
}

async function onSkip() {
await setServerUrl(null);
await setServerUrl('');
Copy link
Member

Choose a reason for hiding this comment

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

🔨 warning: ‏we need to keep null here.

if (url == null) {

await loggedIn();
navigate('/');
}

async function onCreateTestFile() {
await setServerUrl(null);
await setServerUrl('');
Copy link
Member

Choose a reason for hiding this comment

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

Same here

await createBudget({ testMode: true });
window.__navigate('/');
window.__navigate?.('/');
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function Import({ modalProps }: ImportProps) {
const [importing, setImporting] = useState(false);

async function onImport() {
const res = await window.Actual.openFileDialog({
const res = await window.Actual?.openFileDialog({
properties: ['openFile'],
filters: [{ name: 'actual', extensions: ['zip', 'blob'] }],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import { ConfirmPasswordForm } from './ConfirmPasswordForm';

export default function Bootstrap() {
let dispatch = useDispatch();
let [error, setError] = useState(null);
let [error, setError] = useState<string | null>(null);

let { checked } = useBootstrapped();

function getErrorMessage(error) {
function getErrorMessage(error: string) {
switch (error) {
case 'invalid-password':
return 'Password cannot be empty';
Expand All @@ -34,7 +34,7 @@ export default function Bootstrap() {
}
}

async function onSetPassword(password) {
async function onSetPassword(password: string) {
setError(null);
let { error } = await send('subscribe-bootstrap', { password });

Expand Down Expand Up @@ -94,7 +94,7 @@ export default function Bootstrap() {
</Button>
}
onSetPassword={onSetPassword}
onError={setError}
onError={err => setError(err)}
Copy link
Member

Choose a reason for hiding this comment

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

🥜 nitpick: ‏I don't think this change is necessary

/>
</View>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import { ConfirmPasswordForm } from './ConfirmPasswordForm';

export default function ChangePassword() {
let navigate = useNavigate();
let [error, setError] = useState(null);
let [msg, setMessage] = useState(null);
let [error, setError] = useState<string | null>(null);
let [msg, setMessage] = useState<string | null>(null);

function getErrorMessage(error) {
function getErrorMessage(error: string) {
switch (error) {
case 'invalid-password':
return 'Password cannot be empty';
Expand All @@ -29,7 +29,7 @@ export default function ChangePassword() {
}
}

async function onSetPassword(password) {
async function onSetPassword(password: string) {
setError(null);
let { error } = await send('subscribe-change-password', { password });

Expand Down Expand Up @@ -93,7 +93,7 @@ export default function ChangePassword() {
</Button>
}
onSetPassword={onSetPassword}
onError={setError}
onError={err => setError(err)}
Copy link
Member

Choose a reason for hiding this comment

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

Same here

/>
</View>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
import React, { type ChangeEvent, useState } from 'react';
import React, { type ChangeEvent, type ReactNode, useState } from 'react';

import { ButtonWithLoading } from '../../common/Button';
import { BigInput } from '../../common/Input';
import View from '../../common/View';

export function ConfirmPasswordForm({ buttons, onSetPassword, onError }) {
type ConfirmPasswordFormProps = {
buttons: ReactNode;
onSetPassword: (password: string) => void;
onError: (error: string) => void;
};

export function ConfirmPasswordForm({
buttons,
onSetPassword,
onError,
}: ConfirmPasswordFormProps) {
let [password1, setPassword1] = useState('');
let [password2, setPassword2] = useState('');
let [showPassword, setShowPassword] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function ScheduleAmountCell({ amount, op }) {
<Text
style={{
flex: 1,
color: num > 0 ? colors.g5 : null,
color: num > 0 ? colors.g5 : undefined,
whiteSpace: 'nowrap',
overflow: 'hidden',
textOverflow: 'ellipsis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function renderResults(results: Results) {

export default function FixSplitsTool() {
let [loading, setLoading] = useState(false);
let [results, setResults] = useState<Results>(null);
let [results, setResults] = useState<Results | null>(null);

async function onFix() {
setLoading(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import View from '../common/View';

type ItemContentProps = {
style: CSSProperties;
to: string;
onClick: MouseEventHandler<HTMLDivElement>;
to?: string;
onClick?: MouseEventHandler<HTMLDivElement>;
Comment on lines +9 to +10
Copy link
Member

Choose a reason for hiding this comment

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

❓ question: ‏is it that there can be either to or onClick? But not both at the same time? If so - we could solve it by changing the types slightly.

(very poor naming chosen here intentionally as this is just an example)

type BaseProps = { style: CSSProperties };
type ItemContentProps = AProps | BProps;

type AProps = BaseProps & { to: string };
type BProps = BaseProps & { onClick: () => void }; 

activeStyle: CSSProperties;
children: ReactNode;
forceActive?: boolean;
Expand All @@ -21,7 +21,7 @@ function ItemContent({
forceActive,
children,
}: ItemContentProps) {
return onClick ? (
return onClick || !to ? (
Copy link
Member

Choose a reason for hiding this comment

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

💭 thought: ‏I don't think we need this change.

<View
role="button"
tabIndex={0}
Expand Down
24 changes: 1 addition & 23 deletions packages/desktop-client/src/components/sort.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,17 @@ import React, {
useEffect,
useRef,
useLayoutEffect,
useMemo,
useState,
useContext,
type RefCallback,
type MutableRefObject,
type Context,
type Ref,
} from 'react';
import { useDrag, useDrop } from 'react-dnd';

import { useMergedRefs } from '../hooks/useMergedRefs';
import { theme } from '../style';

import View from './common/View';

function useMergedRefs<T>(
ref1: RefCallback<T> | MutableRefObject<T>,
ref2: RefCallback<T> | MutableRefObject<T>,
): Ref<T> {
return useMemo(() => {
function ref(value) {
[ref1, ref2].forEach(ref => {
if (typeof ref === 'function') {
ref(value);
} else if (ref != null) {
ref.current = value;
}
});
}

return ref;
}, [ref1, ref2]);
}

type DragState = {
state: 'start-preview' | 'start' | 'end';
type?: string;
Expand Down
17 changes: 0 additions & 17 deletions packages/desktop-client/src/hooks/useMergedRefs.js

This file was deleted.

25 changes: 25 additions & 0 deletions packages/desktop-client/src/hooks/useMergedRefs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {
type MutableRefObject,
type Ref,
type RefCallback,
useMemo,
} from 'react';

export function useMergedRefs<T>(
ref1: RefCallback<T> | MutableRefObject<T>,
ref2: RefCallback<T> | MutableRefObject<T>,
): Ref<T> {
return useMemo(() => {
function ref(value: T) {
[ref1, ref2].forEach(ref => {
if (typeof ref === 'function') {
ref(value);
} else if (ref != null) {
ref.current = value;
}
});
}

return ref;
}, [ref1, ref2]);
}
2 changes: 1 addition & 1 deletion packages/loot-core/src/client/actions/budgets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export function importBudget(
dispatch(closeModal());

await dispatch(loadPrefs());
window.__navigate('/budget');
window.__navigate?.('/budget');
};
}

Expand Down
4 changes: 2 additions & 2 deletions packages/loot-core/src/shared/transactions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { v4 as uuidv4 } from 'uuid';

import { last, diffItems, applyChanges } from './util';
import { diffItems, applyChanges } from './util';

export function isPreviewId(id) {
return id.indexOf('preview/') !== -1;
Expand Down Expand Up @@ -164,7 +164,7 @@ export function addSplitTransaction(transactions, id) {
if (!trans.is_parent) {
return trans;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making last into a typed generic in util.ts bubbled up type issues to transactions.ts. This is the only location in the codebase using last, so it seemed most practical to just remove it.

let prevSub = last(trans.subtransactions);
let prevSub = trans.subtransactions[trans.subtransactions.length - 1];
trans.subtransactions.push(
makeChild(trans, {
amount: 0,
Expand Down
4 changes: 0 additions & 4 deletions packages/loot-core/src/shared/util.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
export function last(arr) {
return arr[arr.length - 1];
}

export function getChangedValues(obj1, obj2) {
// Keep the id field because this is mostly used to diff database
// objects
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/1671.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [doggan]
---

TypeScript migration and type hardening.