fix number formatter not listening to state changes (#1423)

Closes #1415

Fix number formatter not listening to state changes. This is not a fully
comprehensive solution. We will need to run a migration campaign to port
over from direct usage of `format` util to `useFormat`, but this is a
first step that solves the most glaring issue.

packages/desktop-client/src/components/spreadsheet/CellValue.tsx

@@ -9,7 +9,7 @@ import {
   type ConditionalPrivacyFilterProps,
 } from '../PrivacyFilter';
 
-import format from './format';
+import { useFormat } from './format';
 import useSheetName from './useSheetName';
 import useSheetValue from './useSheetValue';
 
@@ -34,6 +34,7 @@ function CellValue({
 }: CellValueProps) {
   let { fullSheetName } = useSheetName(binding);
   let sheetValue = useSheetValue(binding);
+  let format = useFormat();
 
   return useMemo(
     () => (
@@ -66,6 +67,7 @@ function CellValue({
       getStyle,
       fullSheetName,
       formatter,
+      format,
       sheetValue,
     ],
   );

packages/desktop-client/src/components/spreadsheet/format.ts

+import { useCallback } from 'react';
+import { useSelector } from 'react-redux';
+
+import { selectNumberFormat } from 'loot-core/src/client/selectors';
 import { integerToCurrency } from 'loot-core/src/shared/util';
 
-export default function format(value, type = 'string'): string {
+/**
+ * @deprecated Please do not use this directly. Use `useFormat` hook
+ */
+export default function format(
+  value: unknown,
+  type = 'string',
+  formatter?: Intl.NumberFormat,
+): string {
   switch (type) {
     case 'string':
       const val = JSON.stringify(value);
@@ -12,14 +23,14 @@ export default function format(value, type = 'string'): string {
     case 'number':
       return '' + value;
     case 'financial-with-sign':
-      let formatted = format(value, 'financial');
-      if (value >= 0) {
+      let formatted = format(value, 'financial', formatter);
+      if (typeof value === 'number' && value >= 0) {
         return '+' + formatted;
       }
       return formatted;
     case 'financial':
       if (value == null || value === '' || value === 0) {
-        return integerToCurrency(0);
+        return integerToCurrency(0, formatter);
       } else if (typeof value === 'string') {
         const parsed = parseFloat(value);
         value = isNaN(parsed) ? 0 : parsed;
@@ -31,8 +42,18 @@ export default function format(value, type = 'string'): string {
         );
       }
 
-      return integerToCurrency(value);
+      return integerToCurrency(value, formatter);
     default:
       throw new Error('Unknown format type: ' + type);
   }
 }
+
+export function useFormat() {
+  const numberFormat = useSelector(selectNumberFormat);
+
+  return useCallback(
+    (value: unknown, type = 'string') =>
+      format(value, type, numberFormat.formatter),
+    [numberFormat],
+  );
+}

packages/loot-core/package.json

@@ -37,6 +37,7 @@
     "node-libofx": "*",
     "path-browserify": "^1.0.1",
     "process": "^0.11.10",
+    "reselect": "^4.1.8",
     "stream-browserify": "^3.0.0",
     "ws": "8.13.0"
   },

packages/loot-core/src/client/selectors.ts

+import { createSelector } from 'reselect';
+
+import { getNumberFormat } from '../shared/util';
+
+import type { State } from './state-types';
+
+const getState = (state: State) => state;
+
+const getPrefsState = createSelector(getState, state => state.prefs);
+const getLocalPrefsState = createSelector(getPrefsState, prefs => prefs.local);
+
+export const selectNumberFormat = createSelector(getLocalPrefsState, prefs =>
+  getNumberFormat({
+    format: prefs.numberFormat,
+    hideFraction: prefs.hideFraction,
+  }),
+);

packages/loot-core/src/shared/util.ts

@@ -183,26 +183,38 @@ export function titleFirst(str) {
   return str[0].toUpperCase() + str.slice(1);
 }
 
-export let numberFormats = [
+type NumberFormats =
+  | 'comma-dot'
+  | 'dot-comma'
+  | 'space-comma'
+  | 'space-dot'
+  | 'comma-dot-in';
+
+export const numberFormats: Array<{
+  value: NumberFormats;
+  label: string;
+  labelNoFraction: string;
+}> = [
   { value: 'comma-dot', label: '1,000.33', labelNoFraction: '1,000' },
   { value: 'dot-comma', label: '1.000,33', labelNoFraction: '1.000' },
   { value: 'space-comma', label: '1 000,33', labelNoFraction: '1 000' },
   { value: 'space-dot', label: '1 000.33', labelNoFraction: '1 000' },
   { value: 'comma-dot-in', label: '1,00,000.33', labelNoFraction: '1,00,000' },
-] as const;
+];
 
-let numberFormat: {
-  value: string | null;
-  formatter: Intl.NumberFormat | null;
-  regex: RegExp | null;
-  separator?: string;
+let numberFormatConfig: {
+  format: NumberFormats;
+  hideFraction: boolean;
 } = {
-  value: null,
-  formatter: null,
-  regex: null,
+  format: 'comma-dot',
+  hideFraction: false,
 };
 
-export function setNumberFormat({ format, hideFraction }) {
+export function setNumberFormat(config: typeof numberFormatConfig) {
+  numberFormatConfig = config;
+}
+
+export function getNumberFormat({ format, hideFraction } = numberFormatConfig) {
   let locale, regex, separator;
 
   switch (format) {
@@ -233,7 +245,7 @@ export function setNumberFormat({ format, hideFraction }) {
       separator = '.';
   }
 
-  numberFormat = {
+  return {
     value: format,
     separator,
     formatter: new Intl.NumberFormat(locale, {
@@ -244,12 +256,6 @@ export function setNumberFormat({ format, hideFraction }) {
   };
 }
 
-export function getNumberFormat() {
-  return numberFormat;
-}
-
-setNumberFormat({ format: 'comma-dot', hideFraction: false });
-
 // Number utilities
 
 // We dont use `Number.MAX_SAFE_NUMBER` and such here because those
@@ -280,17 +286,19 @@ export function toRelaxedNumber(value) {
   return integerToAmount(currencyToInteger(value) || 0);
 }
 
-export function integerToCurrency(n) {
-  return numberFormat.formatter.format(safeNumber(n) / 100);
+export function integerToCurrency(n, formatter = getNumberFormat().formatter) {
+  return formatter.format(safeNumber(n) / 100);
 }
 
 export function amountToCurrency(n) {
-  return numberFormat.formatter.format(n);
+  return getNumberFormat().formatter.format(n);
 }
 
 export function currencyToAmount(str) {
   let amount = parseFloat(
-    str.replace(numberFormat.regex, '').replace(numberFormat.separator, '.'),
+    str
+      .replace(getNumberFormat().regex, '')
+      .replace(getNumberFormat().separator, '.'),
   );
   return isNaN(amount) ? null : amount;
 }

upcoming-release-notes/1423.md

+---
+category: Bugfix
+authors: [MatissJanis]
+---
+
+Fix number formatting setting not affecting sidenav

yarn.lock

@@ -12595,6 +12595,7 @@ __metadata:
     path-browserify: ^1.0.1
     peggy: 3.0.2
     process: ^0.11.10
+    reselect: ^4.1.8
     slash: 3.0.0
     snapshot-diff: ^0.10.0
     source-map: ^0.7.3
@@ -15965,6 +15966,13 @@ __metadata:
   languageName: node
   linkType: hard
+"reselect@npm:^4.1.8":
+  version: 4.1.8
+  resolution: "reselect@npm:4.1.8"
+  checksum: a4ac87cedab198769a29be92bc221c32da76cfdad6911eda67b4d3e7136dca86208c3b210e31632eae31ebd2cded18596f0dd230d3ccc9e978df22f233b5583e
+  languageName: node
+  linkType: hard
+
 "resolve-alpn@npm:^1.0.0":
   version: 1.2.1
   resolution: "resolve-alpn@npm:1.2.1"