From 2c845ee27acd05c01834a658d65255f4b6c8adb5 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sat, 24 Sep 2022 19:56:23 -0400 Subject: [PATCH 1/9] Simpler validation that uses zod for loading json --- package-lock.json | 5 + package.json | 1 + src/main/app.ts | 8 +- src/main/config.ts | 13 - src/main/index.ts | 13 +- src/main/json.ts | 164 ++++------ src/main/log.ts | 40 +++ .../migrations/config/1_initialDefinition.ts | 29 -- src/main/migrations/config/index.ts | 7 - .../appState/1_initialDefinition.ts | 0 .../{migrations => schemas}/appState/index.ts | 0 .../schemas/config/1_initialDefinition.ts | 26 ++ src/main/schemas/config/2_addLogDirectory.ts | 34 +++ src/main/schemas/config/index.ts | 7 + .../shortcuts/1_initialDefinition.ts | 0 .../shortcuts/index.ts | 0 src/main/shortcuts.ts | 4 +- src/renderer/App.tsx | 2 +- .../components/SidebarNewNoteButton.tsx | 2 + src/renderer/logger.ts | 20 ++ src/renderer/preload.ts | 8 +- src/shared/domain/config.ts | 1 + src/shared/ipc.ts | 11 + src/shared/logger.ts | 6 + test/main/app.spec.ts | 36 +-- test/main/index.spec.ts | 100 +++---- test/main/json.spec.ts | 280 ++++++++++++------ test/main/shortcuts.spec.ts | 220 +++++++------- test/shared/domain/index.spec.ts | 6 +- tsconfig.json | 1 + tsconfig.test.json | 1 + 31 files changed, 594 insertions(+), 451 deletions(-) create mode 100644 src/main/log.ts delete mode 100644 src/main/migrations/config/1_initialDefinition.ts delete mode 100644 src/main/migrations/config/index.ts rename src/main/{migrations => schemas}/appState/1_initialDefinition.ts (100%) rename src/main/{migrations => schemas}/appState/index.ts (100%) create mode 100644 src/main/schemas/config/1_initialDefinition.ts create mode 100644 src/main/schemas/config/2_addLogDirectory.ts create mode 100644 src/main/schemas/config/index.ts rename src/main/{migrations => schemas}/shortcuts/1_initialDefinition.ts (100%) rename src/main/{migrations => schemas}/shortcuts/index.ts (100%) create mode 100644 src/renderer/logger.ts create mode 100644 src/shared/logger.ts diff --git a/package-lock.json b/package-lock.json index 9631caf7..16cd675b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12541,6 +12541,11 @@ "punycode": "^2.1.1" } }, + "tozod": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/tozod/-/tozod-3.0.0.tgz", + "integrity": "sha512-R03/moNPEGgWKoTdCsEJuyEoP6NtXyhWg9L9lJhF2XyCR2Ce9XE+yXaswaVmqyBpHsRPC2Pk38mK8Ex7iHaXcg==" + }, "tr46": { "version": "0.0.3", "resolved": "https://registry.npmjs.org/tr46/-/tr46-0.0.3.tgz", diff --git a/package.json b/package.json index 12c45c69..95088ca5 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "sass": "^1.43.4", "sass-loader": "^12.3.0", "styled-components": "^5.3.3", + "tozod": "^3.0.0", "unist-util-visit": "^2.0.3", "zod": "^3.17.10" } diff --git a/src/main/app.ts b/src/main/app.ts index 015998c2..fc49182c 100644 --- a/src/main/app.ts +++ b/src/main/app.ts @@ -9,7 +9,6 @@ import { IpcChannel, IpcMainTS } from "../shared/ipc"; import { openInBrowser } from "./utils"; import { UIEventType, UIEventInput } from "../shared/ui/events"; import { AppState, DEFAULT_SIDEBAR_WIDTH, Section } from "../shared/ui/app"; -import { parseJSON } from "date-fns"; import { z } from "zod"; import { NoteSort, @@ -17,10 +16,10 @@ import { } from "../shared/domain/note"; import { JsonFile, loadJsonFile } from "./json"; import { Config } from "../shared/domain/config"; -import { APP_STATE_MIGRATIONS } from "./migrations/appState"; import p from "path"; import { MissingDataDirectoryError } from "../shared/errors"; import { DATE_OR_STRING_SCHEMA } from "../shared/domain"; +import { APP_STATE_MIGRATIONS } from "./schemas/appState"; export const APP_STATE_PATH = "ui.json"; @@ -78,9 +77,10 @@ export function appIpcs(ipc: IpcMainTS, config: JsonFile): void { throw new MissingDataDirectoryError(); } - appStateFile = await loadJsonFile( + appStateFile = await loadJsonFile( p.join(config.content.dataDirectory, APP_STATE_PATH), - APP_STATE_SCHEMA, + // APP_STATE_SCHEMA, + // [appStateSchemaV1] APP_STATE_MIGRATIONS ); }); diff --git a/src/main/config.ts b/src/main/config.ts index 75aaf44b..122ceada 100644 --- a/src/main/config.ts +++ b/src/main/config.ts @@ -1,21 +1,8 @@ import { BrowserWindow, dialog, shell } from "electron"; import { IpcMainTS } from "../shared/ipc"; -import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "."; import { Config } from "../shared/domain/config"; -import { z } from "zod"; import { JsonFile } from "./json"; -export const CONFIG_SCHEMA = z - .object({ - version: z.literal(1).optional().default(1), - windowHeight: z.number().min(1).optional().default(DEFAULT_WINDOW_HEIGHT), - windowWidth: z.number().min(1).optional().default(DEFAULT_WINDOW_WIDTH), - dataDirectory: z.string().optional(), - }) - .default({ - version: 1, - }); - export function configIpcs(ipc: IpcMainTS, config: JsonFile): void { ipc.handle( "config.hasDataDirectory", diff --git a/src/main/index.ts b/src/main/index.ts index 1f91a313..25bca7b4 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2,16 +2,17 @@ import { app, BrowserWindow, ipcMain, Menu, session } from "electron"; import { getProcessType, isDevelopment, isTest } from "../shared/env"; import { IpcMainTS } from "../shared/ipc"; import { appIpcs } from "./app"; -import { configIpcs, CONFIG_SCHEMA } from "./config"; +import { configIpcs } from "./config"; import { noteIpcs } from "./notes"; import { openInBrowser } from "./utils"; import * as path from "path"; import * as fs from "fs"; import * as fsp from "fs/promises"; -import { loadJsonFile } from "./json"; -import { CONFIG_MIGRATIONS } from "./migrations/config"; +import { loadJsonFile2 } from "./json"; +import { CONFIG_SCHEMAS } from "./schemas/config"; import { Config } from "../shared/domain/config"; import { shortcutIpcs } from "./shortcuts"; +import { logIpcs } from "./log"; if (getProcessType() !== "main") { throw Error( @@ -33,10 +34,9 @@ export const DEFAULT_WINDOW_WIDTH = 800; let mainWindow: BrowserWindow; export async function main(): Promise { - const configFile = await loadJsonFile( + const configFile = await loadJsonFile2( getConfigPath(), - CONFIG_SCHEMA, - CONFIG_MIGRATIONS + CONFIG_SCHEMAS ); if (isDevelopment() && configFile.content.dataDirectory == null) { @@ -55,6 +55,7 @@ export async function main(): Promise { const typeSafeIpc = ipcMain as IpcMainTS; configIpcs(typeSafeIpc, configFile); + logIpcs(typeSafeIpc, configFile); appIpcs(typeSafeIpc, configFile); shortcutIpcs(typeSafeIpc, configFile); noteIpcs(typeSafeIpc, configFile); diff --git a/src/main/json.ts b/src/main/json.ts index 762c3874..4c258a5e 100644 --- a/src/main/json.ts +++ b/src/main/json.ts @@ -1,145 +1,85 @@ -import { cloneDeep, last, uniq } from "lodash"; +import { cloneDeep, last } from "lodash"; import * as fsp from "fs/promises"; import * as fs from "fs"; -import { ZodTypeAny } from "zod"; -import { deepUpdate } from "../shared/deepUpdate"; +import { ZodSchema } from "zod"; import { DeepPartial } from "tsdef"; - -type Versioned = T & { version: number }; +import { deepUpdate } from "../shared/deepUpdate"; export interface JsonFile { - // Makes it easy to send off over IPC if it's in it's own prop. content: Readonly; update(partial: DeepPartial): Promise; } -/** - * Migration to assist in converting JSON content. - */ -export abstract class JsonMigration { - /** - * Json version of the input. - */ - abstract version: number; - - /** - * Validate the input and ensure it's meant for this migration. - * Throws if invalid. - * @param input The original input to validate. - */ - abstract validateInput(input: unknown): Promise; - - /** - * Migrate the JSON content and return it's updated version. - * @param input The input to migrate. - */ - protected abstract migrate(input: Input): Promise; - - /** - * Attempt to migrate the JSON. Throws if the input was invalid. - * @param input The JSON content to migrate. - * @returns The updated content. - */ - async run(input: unknown): Promise> { - const validated = await this.validateInput(input); - const output = await this.migrate(validated); - - return { - version: this.version, - ...cloneDeep(output), - }; - } -} - -export async function loadJsonFile( +export async function loadJsonFile( filePath: string, - schema: ZodTypeAny, - migrations: JsonMigration>[] + schemas: { + [version: number]: ZodSchema; + }, + defaultContent: Content ): Promise> { - if (migrations.length === 0) { - throw new Error( - `Expected a least 1 migration. Otherwise we can't validate the content at all.` - ); + const schemaArray = Object.entries(schemas) + .map<[number, ZodSchema]>(([version, schema]) => [ + Number.parseInt(version, 10), + schema, + ]) + .sort(([a], [b]) => (a > b ? 1 : -1)); + + if (schemaArray.length === 0) { + throw new Error(`Expected at least 1 schema in order to validate content.`); } - const migrationVersions = migrations.map((m) => m.version); - if (uniq(migrationVersions).length !== migrationVersions.length) { - throw new Error(`Duplicate migration numbers detected for ${filePath}`); - } - - for (let i = 1; i < migrations.length; i++) { - const prev = migrations[i - 1]; - const curr = migrations[i]; - - if (prev.version > curr.version) { - throw new Error( - `Migration versions are out of order for ${filePath}. ${prev.version} comes before ${curr.version}` - ); - } - } - - let originalContent; + let originalContent: Content | undefined; if (fs.existsSync(filePath)) { const raw = await fsp.readFile(filePath, { encoding: "utf-8" }); originalContent = JSON.parse(raw); } - let versioned: Versioned; - if (originalContent == null || typeof originalContent !== "object") { - versioned = { version: migrations[0].version }; - } else { - versioned = originalContent as Versioned; + // Apply default content if no content found, or if it had no versioning. + if ( + originalContent == null || + !originalContent.hasOwnProperty("version") || + typeof originalContent.version !== "number" + ) { + originalContent = defaultContent; + } + + // Always include current version schema so we can validate against it. + const relevantSchemas = schemaArray.filter( + ([version]) => originalContent!.version <= version + ); + + let content = cloneDeep(originalContent); + for (const [, schema] of relevantSchemas) { + content = await schema.parseAsync(content); } - const migratedContent = await runMigrations(versioned, migrations); + // Save changes to file if any were made while migrating to latest. + if (content.version !== originalContent.version) { + const jsonContent = JSON.stringify(content); + await fsp.writeFile(filePath, jsonContent, { encoding: "utf-8" }); + } - // We always want to run this because it'll apply defaults for any missing - // values, and in the event the json file has been modified to the point - // where it's unusable, it'll throw an error instead of proceeding. - const content = await schema.parseAsync(migratedContent); + const [latestVersion, latestSchema] = last(relevantSchemas)!; + if (defaultContent.version !== latestVersion) { + throw new Error( + `Default content doesn't match latest version ${latestVersion}. Did you mean to update the default?` + ); + } const update = async (partial: DeepPartial) => { const updated = deepUpdate(content, partial); - const validated = await schema.parseAsync(updated); + + // Validate against latest schema when saving to ensure we have valid content. + const validated = await latestSchema.parseAsync(updated); const jsonString = JSON.stringify(updated); - obj.content = validated; + fileHandler.content = validated; await fsp.writeFile(filePath, jsonString, { encoding: "utf-8" }); }; - const obj = { + const fileHandler = { content, update, }; - - return obj; -} - -// Should not be used outside of this file. -async function runMigrations( - input: { version: number }, - migrations: JsonMigration>[] -): Promise> { - const latestMigration = last(migrations)!; - - if (input.version > latestMigration.version) { - throw new Error( - `Input version ${input.version} is higher than latest migration ${latestMigration.version}` - ); - } - - if (input.version === latestMigration.version) { - return input as Versioned; - } - - let current = input; - for (let i = 0; i < migrations.length; i++) { - if (current.version > migrations[i].version) { - continue; - } - - current = await migrations[i].run(current); - } - - return current as Versioned; + return fileHandler; } diff --git a/src/main/log.ts b/src/main/log.ts new file mode 100644 index 00000000..88775e2f --- /dev/null +++ b/src/main/log.ts @@ -0,0 +1,40 @@ +import { format } from "date-fns"; +import { Config } from "../shared/domain/config"; +import { IpcMainTS } from "../shared/ipc"; +import { JsonFile } from "./json"; +import * as fs from "fs"; + +// TODO: Add logging to file +// Have it manage files and delete anything older than 2 weeks + +export function logIpcs(ipc: IpcMainTS, config: JsonFile): void { + // let fileStream; + + ipc.on("init", async () => { + // fileStream = fs.createWriteStream(); + }); + + ipc.handle("log.info", async (_, message) => { + const fullMessage = `${getTimeStamp()} [RENDERER] (info): ${message}`; + console.log(fullMessage); + }); + + ipc.handle("log.debug", async (_, message) => { + const fullMessage = `${getTimeStamp()} [RENDERER] (debug): ${message}`; + console.log(fullMessage); + }); + + ipc.handle("log.warn", async (_, message) => { + const fullMessage = `${getTimeStamp()} [RENDERER] (warn): ${message}`; + console.warn(fullMessage); + }); + + ipc.handle("log.error", async (_, message) => { + const fullMessage = `${getTimeStamp()} [RENDERER] (ERROR): ${message}`; + console.error(fullMessage); + }); +} + +export function getTimeStamp() { + return format(new Date(), "L/d H:mm"); +} diff --git a/src/main/migrations/config/1_initialDefinition.ts b/src/main/migrations/config/1_initialDefinition.ts deleted file mode 100644 index f36cea02..00000000 --- a/src/main/migrations/config/1_initialDefinition.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { z } from "zod"; -import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "../.."; -import { Config } from "../../../shared/domain/config"; -import { JsonMigration } from "../../json"; - -export const configSchemaV1 = z.object({ - version: z.literal(1).optional().default(1), - windowHeight: z.number().min(1).default(DEFAULT_WINDOW_HEIGHT), - windowWidth: z.number().min(1).default(DEFAULT_WINDOW_WIDTH), - dataDirectory: z.string().optional(), -}); - -type ConfigV1 = z.infer; - -export class ConfigInitialDefinition extends JsonMigration { - version = 1; - - async validateInput(input: unknown): Promise { - return await configSchemaV1.parseAsync(input); - } - - protected async migrate(input: ConfigV1): Promise { - return { - windowHeight: input.windowHeight, - windowWidth: input.windowWidth, - dataDirectory: input.dataDirectory, - }; - } -} diff --git a/src/main/migrations/config/index.ts b/src/main/migrations/config/index.ts deleted file mode 100644 index 8b6275ce..00000000 --- a/src/main/migrations/config/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { Config } from "../../../shared/domain/config"; -import { JsonMigration } from "../../json"; -import { ConfigInitialDefinition } from "./1_initialDefinition"; - -export const CONFIG_MIGRATIONS: JsonMigration[] = [ - new ConfigInitialDefinition(), -]; diff --git a/src/main/migrations/appState/1_initialDefinition.ts b/src/main/schemas/appState/1_initialDefinition.ts similarity index 100% rename from src/main/migrations/appState/1_initialDefinition.ts rename to src/main/schemas/appState/1_initialDefinition.ts diff --git a/src/main/migrations/appState/index.ts b/src/main/schemas/appState/index.ts similarity index 100% rename from src/main/migrations/appState/index.ts rename to src/main/schemas/appState/index.ts diff --git a/src/main/schemas/config/1_initialDefinition.ts b/src/main/schemas/config/1_initialDefinition.ts new file mode 100644 index 00000000..c1efc748 --- /dev/null +++ b/src/main/schemas/config/1_initialDefinition.ts @@ -0,0 +1,26 @@ +import { z } from "zod"; +import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "../.."; + +export interface ConfigV1 { + version: number; + windowHeight: number; + windowWidth: number; + dataDirectory?: string; +} + +export const configSchemaV1: z.Schema = z.preprocess( + (obj: unknown) => { + const config = obj as Partial; + + config.windowHeight ??= DEFAULT_WINDOW_HEIGHT; + config.windowWidth ??= DEFAULT_WINDOW_WIDTH; + + return obj; + }, + z.object({ + version: z.literal(1), + windowHeight: z.number().min(1), + windowWidth: z.number().min(1), + dataDirectory: z.string().optional(), + }) +); diff --git a/src/main/schemas/config/2_addLogDirectory.ts b/src/main/schemas/config/2_addLogDirectory.ts new file mode 100644 index 00000000..bd22bcfe --- /dev/null +++ b/src/main/schemas/config/2_addLogDirectory.ts @@ -0,0 +1,34 @@ +import { app } from "electron"; +import { z } from "zod"; +import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "../.."; +import { ConfigV1 } from "./1_initialDefinition"; + +export interface ConfigV2 { + version: number; + windowHeight: number; + windowWidth: number; + dataDirectory?: string; + logDirectory?: string; +} + +export const configSchemaV2 = z.preprocess( + (obj) => { + const config = obj as ConfigV1 | ConfigV2; + + if (config.version === 1) { + return { + ...config, + logDirectory: app.getPath("logs"), + }; + } + + return config; + }, + z.object({ + version: z.literal(1).transform(() => 2), + windowHeight: z.number().min(1).default(DEFAULT_WINDOW_HEIGHT), + windowWidth: z.number().min(1).default(DEFAULT_WINDOW_WIDTH), + dataDirectory: z.string().optional(), + logDirectory: z.string(), + }) +); diff --git a/src/main/schemas/config/index.ts b/src/main/schemas/config/index.ts new file mode 100644 index 00000000..21a83559 --- /dev/null +++ b/src/main/schemas/config/index.ts @@ -0,0 +1,7 @@ +import { configSchemaV1 } from "./1_initialDefinition"; +import { configSchemaV2 } from "./2_addLogDirectory"; + +export const CONFIG_SCHEMAS = { + 1: configSchemaV1, + 2: configSchemaV2, +}; diff --git a/src/main/migrations/shortcuts/1_initialDefinition.ts b/src/main/schemas/shortcuts/1_initialDefinition.ts similarity index 100% rename from src/main/migrations/shortcuts/1_initialDefinition.ts rename to src/main/schemas/shortcuts/1_initialDefinition.ts diff --git a/src/main/migrations/shortcuts/index.ts b/src/main/schemas/shortcuts/index.ts similarity index 100% rename from src/main/migrations/shortcuts/index.ts rename to src/main/schemas/shortcuts/index.ts diff --git a/src/main/shortcuts.ts b/src/main/shortcuts.ts index 14a038f0..b75da4c6 100644 --- a/src/main/shortcuts.ts +++ b/src/main/shortcuts.ts @@ -8,8 +8,8 @@ import { Section } from "../shared/ui/app"; import { UIEventInput, UIEventType } from "../shared/ui/events"; import { JsonFile, loadJsonFile } from "./json"; import p from "path"; -import { SHORTCUT_FILE_MIGRATIONS } from "./migrations/shortcuts"; -import { OVERRIDE_SCHEMA } from "./migrations/shortcuts/1_initialDefinition"; +import { SHORTCUT_FILE_MIGRATIONS } from "./schemas/shortcuts"; +import { OVERRIDE_SCHEMA } from "./schemas/shortcuts/1_initialDefinition"; export interface Shortcuts { version: number; diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 2b846ab3..f73ed9ab 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -17,7 +17,6 @@ import { useContextMenu } from "./menus/contextMenu"; import { Editor } from "./components/Editor"; import { h100, HEADER_SIZES, mb2, w100 } from "./css"; import { AppState } from "../shared/ui/app"; -import { DEFAULT_SHORTCUTS } from "../shared/io/defaultShortcuts"; const { ipc } = window; async function main() { @@ -72,6 +71,7 @@ export function App(props: AppProps): JSX.Element { store.on("focus.push", push); store.on("focus.pop", pop); + return () => { store.off("app.quit", quit); store.off("app.toggleSidebar", toggleSidebar); diff --git a/src/renderer/components/SidebarNewNoteButton.tsx b/src/renderer/components/SidebarNewNoteButton.tsx index 9e6e2546..545393b6 100644 --- a/src/renderer/components/SidebarNewNoteButton.tsx +++ b/src/renderer/components/SidebarNewNoteButton.tsx @@ -4,6 +4,7 @@ import React from "react"; import { PropsWithChildren } from "react"; import styled from "styled-components"; import { pb2 } from "../css"; +import { logger } from "../logger"; import { Store } from "../store"; import { Icon } from "./shared/Icon"; @@ -15,6 +16,7 @@ export function SidebarNewNoteButton( props: PropsWithChildren ): JSX.Element { const onClick = (ev: React.MouseEvent) => { + logger.info("HI"); // Stop prop otherwise we'll mess up switching focus ev.stopPropagation(); props.store.dispatch("sidebar.createNote", null); diff --git a/src/renderer/logger.ts b/src/renderer/logger.ts new file mode 100644 index 00000000..b2c634ee --- /dev/null +++ b/src/renderer/logger.ts @@ -0,0 +1,20 @@ +import { Logger } from "../shared/logger"; + +export const logger: Logger = { + info: async (message) => { + console.log(message); + window.ipc("log.info", message); + }, + warn: async (message) => { + console.warn(message); + window.ipc("log.warn", message); + }, + error: async (message) => { + console.error(message); + window.ipc("log.error", message); + }, + debug: async (message) => { + console.log(message); + window.ipc("log.debug", message); + }, +}; diff --git a/src/renderer/preload.ts b/src/renderer/preload.ts index 81122f81..1a9638ea 100644 --- a/src/renderer/preload.ts +++ b/src/renderer/preload.ts @@ -1,6 +1,6 @@ import { contextBridge, ipcRenderer } from "electron"; import { getProcessType, isDevelopment } from "../shared/env"; -import { Ipc, IpcChannel, IPCS, IpcSchema, IpcType } from "../shared/ipc"; +import { Ipc, IpcChannel, IPCS, IpcType } from "../shared/ipc"; if (getProcessType() === "main") { throw Error( @@ -8,15 +8,15 @@ if (getProcessType() === "main") { ); } -contextBridge.exposeInMainWorld("ipc", (channel: IpcType, ...data: []) => { +contextBridge.exposeInMainWorld("ipc", (ipcType: IpcType, ...data: []) => { // We need to be really careful about what we expose in the renderer. // ipcRenderer shouldn't be directly exposed to the renderer and we also want // to perform some validation to ensure the action is a known one. // See for more info: // https://stackoverflow.com/questions/57807459/how-to-use-preload-js-properly-in-electron - if (IPCS.includes(channel)) { - return ipcRenderer.invoke(channel, ...data); + if (IPCS.includes(ipcType)) { + return ipcRenderer.invoke(ipcType, ...data); } }); diff --git a/src/shared/domain/config.ts b/src/shared/domain/config.ts index 08fe5d81..7d457d3b 100644 --- a/src/shared/domain/config.ts +++ b/src/shared/domain/config.ts @@ -2,4 +2,5 @@ export interface Config { windowHeight: number; windowWidth: number; dataDirectory?: string; + logDirectory: string; } diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index 61796999..59495ae0 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -31,6 +31,11 @@ export const IPCS = [ "config.hasDataDirectory", "config.selectDataDirectory", "config.openDataDirectory", + + "log.info", + "log.debug", + "log.warn", + "log.error", ] as const; export type IpcType = typeof IPCS[number]; @@ -66,6 +71,12 @@ export interface IpcSchema extends Record any> { "config.hasDataDirectory"(): Promise; "config.selectDataDirectory"(): Promise; "config.openDataDirectory"(): Promise; + + // Logging + "log.info"(message: string): Promise; + "log.debug"(message: string): Promise; + "log.warn"(message: string): Promise; + "log.error"(message: string): Promise; } export type Ipc = >( diff --git a/src/shared/logger.ts b/src/shared/logger.ts new file mode 100644 index 00000000..6d085c7c --- /dev/null +++ b/src/shared/logger.ts @@ -0,0 +1,6 @@ +export interface Logger { + info(message: string): Promise; + warn(message: string): Promise; + error(message: string): Promise; + debug(message: string): Promise; +} diff --git a/test/main/app.spec.ts b/test/main/app.spec.ts index 914efc6e..eec4523f 100644 --- a/test/main/app.spec.ts +++ b/test/main/app.spec.ts @@ -1,21 +1,21 @@ -import { createConfig } from "../__factories__/config"; -import { createIpcMainTS } from "../__factories__/ipc"; -import { appIpcs } from "../../src/main/app"; -import { createJsonFile } from "../__factories__/jsonFile"; -import { BrowserWindow } from "../__mocks__/electron"; +// import { createConfig } from "../__factories__/config"; +// import { createIpcMainTS } from "../__factories__/ipc"; +// import { appIpcs } from "../../src/main/app"; +// import { createJsonFile } from "../__factories__/jsonFile"; +// import { BrowserWindow } from "../__mocks__/electron"; -test("app.inspectElement rounds floats", async () => { - const ipc = createIpcMainTS(); - const config = createJsonFile(createConfig()); - appIpcs(ipc, config); +// test("app.inspectElement rounds floats", async () => { +// const ipc = createIpcMainTS(); +// const config = createJsonFile(createConfig()); +// appIpcs(ipc, config); - const inspectElement = jest.fn(); - BrowserWindow.getFocusedWindow.mockImplementationOnce(() => ({ - webContents: { - inspectElement, - }, - })); +// const inspectElement = jest.fn(); +// BrowserWindow.getFocusedWindow.mockImplementationOnce(() => ({ +// webContents: { +// inspectElement, +// }, +// })); - await ipc.invoke("app.inspectElement", { x: 1.23, y: 2.67 }); - expect(inspectElement).toBeCalledWith(1, 3); -}); +// await ipc.invoke("app.inspectElement", { x: 1.23, y: 2.67 }); +// expect(inspectElement).toBeCalledWith(1, 3); +// }); diff --git a/test/main/index.spec.ts b/test/main/index.spec.ts index edfccd9f..caf6705b 100644 --- a/test/main/index.spec.ts +++ b/test/main/index.spec.ts @@ -1,50 +1,50 @@ -import * as json from "../../src/main/json"; -import { DEFAULT_DEV_DATA_DIRECTORY, main } from "../../src/main/index"; -import fsp from "fs/promises"; -import fs from "fs"; -import * as env from "../../src/shared/env"; -import { loadJsonFile } from "../../src/main/json"; - -jest.mock("fs/promises"); -jest.mock("fs"); -jest.mock("../../src/main/json"); - -test("main defaults data directory in development", async () => { - const update = jest.fn(); - - (loadJsonFile as jest.Mock).mockResolvedValueOnce({ - content: { - dataDirectory: null, - windowHeight: 800, - windowWidth: 600, - }, - update, - }); - - // We use spyOn instead of mocking entire module because we need getProcessType - // to function normal. - jest.spyOn(env, "isDevelopment").mockReturnValue(true); - - await main(); - expect(update).toHaveBeenCalledWith({ - dataDirectory: DEFAULT_DEV_DATA_DIRECTORY, - }); -}); - -test("main creates data directory if directory is missing.", async () => { - jest.spyOn(json, "loadJsonFile").mockResolvedValueOnce({ - content: { - dataDirectory: "foo", - windowHeight: 800, - windowWidth: 600, - }, - update: jest.fn(), - }); - - const mkdir = jest.fn(); - (fsp.mkdir as jest.Mock).mockImplementation(mkdir); - (fs.existsSync as jest.Mock).mockReturnValue(false); - - await main(); - expect(mkdir).toHaveBeenCalledWith("foo"); -}); +// import * as json from "../../src/main/json"; +// import { DEFAULT_DEV_DATA_DIRECTORY, main } from "../../src/main/index"; +// import fsp from "fs/promises"; +// import fs from "fs"; +// import * as env from "../../src/shared/env"; +// import { loadJsonFile } from "../../src/main/json"; + +// jest.mock("fs/promises"); +// jest.mock("fs"); +// jest.mock("../../src/main/json"); + +// test("main defaults data directory in development", async () => { +// const update = jest.fn(); + +// (loadJsonFile as jest.Mock).mockResolvedValueOnce({ +// content: { +// dataDirectory: null, +// windowHeight: 800, +// windowWidth: 600, +// }, +// update, +// }); + +// // We use spyOn instead of mocking entire module because we need getProcessType +// // to function normal. +// jest.spyOn(env, "isDevelopment").mockReturnValue(true); + +// await main(); +// expect(update).toHaveBeenCalledWith({ +// dataDirectory: DEFAULT_DEV_DATA_DIRECTORY, +// }); +// }); + +// test("main creates data directory if directory is missing.", async () => { +// jest.spyOn(json, "loadJsonFile").mockResolvedValueOnce({ +// content: { +// dataDirectory: "foo", +// windowHeight: 800, +// windowWidth: 600, +// }, +// update: jest.fn(), +// }); + +// const mkdir = jest.fn(); +// (fsp.mkdir as jest.Mock).mockImplementation(mkdir); +// (fs.existsSync as jest.Mock).mockReturnValue(false); + +// await main(); +// expect(mkdir).toHaveBeenCalledWith("foo"); +// }); diff --git a/test/main/json.spec.ts b/test/main/json.spec.ts index d689d798..ddacd6fe 100644 --- a/test/main/json.spec.ts +++ b/test/main/json.spec.ts @@ -1,121 +1,217 @@ -import { JsonMigration, loadJsonFile } from "../../src/main/json"; -import { z } from "zod"; +import { loadJsonFile } from "../../src/main/json"; import fsp from "fs/promises"; import fs from "fs"; +import { z, ZodSchema } from "zod"; jest.mock("fs"); jest.mock("fs/promises"); -const mockSchema = z.object({ - version: z.number(), - name: z.string(), - age: z.number(), -}); +interface FooV1 { + version: 1; + foo: string; +} -test("loadJsonFile throws on duplicate version numbers", async () => { - expect(() => - loadJsonFile("foo.json", mockSchema, [ - { version: 1 } as JsonMigration, - { version: 1 } as JsonMigration, - ]) - ).rejects.toThrow(/Duplicate migration numbers detected for foo.json/); -}); +interface FooV2 { + version: 2; + foo: string; + bar: number; +} -test("loadJsonFile throws if migrations are out of order", async () => { - expect(() => - loadJsonFile("foo.json", mockSchema, [ - { version: 2 } as JsonMigration, - { version: 1 } as JsonMigration, - ]) - ).rejects.toThrow( - /Migration versions are out of order for foo.json. 2 comes before 1/ - ); +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +//@ts-ignore foo! +const fooV1: z.Schema = z.object({ + version: z.literal(1), + foo: z.string(), }); - -test("loadJsonFile throws if content has newer version than latest migration", async () => { - (fs.existsSync as jest.Mock).mockReturnValueOnce(true); - (fsp.readFile as jest.Mock).mockResolvedValueOnce('{ "version": 10 }'); - - expect(() => - loadJsonFile("foo.json", mockSchema, [ - { version: 1 } as JsonMigration, - { version: 2 } as JsonMigration, - ]) - ).rejects.toThrow(/Input version 10 is higher than latest migration 2/); +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +//@ts-ignore foo! +const fooV2: z.Schema = z.preprocess( + (obj) => { + const foo = obj as FooV1 | FooV2; + + if (foo.version === 1) { + return { + ...foo, + bar: 3, + }; + } + + return foo; + }, + z.object({ + version: z.literal(2), + foo: z.string(), + bar: z.number(), + }) +); + +test("loadJsonFile throws if no migrations passed", async () => { + await expect(async () => { + await loadJsonFile("fake-file-path.json", {}, null!); + }).rejects.toThrow(/Expected at least 1 schema/); }); -test("loadJsonFile handles null value", async () => { - const migrations = [new InitialDefinition(), new AddAge()]; +test("loadJsonFile loads default content if no file found", async () => { + (fs.existsSync as jest.Mock).mockReturnValueOnce(false); + + const { content } = await loadJsonFile( + "fake-file-path.json", + { + 1: fooV1, + 2: fooV2, + }, + { + version: 2, + foo: "cat", + bar: 42, + } + ); + expect(content).toEqual({ + version: 2, + foo: "cat", + bar: 42, + }); +}); +test("loadJsonFile loads content and validates it", async () => { (fs.existsSync as jest.Mock).mockReturnValueOnce(true); - (fsp.readFile as jest.Mock).mockResolvedValueOnce(null); + (fsp.readFile as jest.Mock).mockResolvedValueOnce( + ` + { + "version": 2, + "foo": "dog", + "bar": 24 + } + ` + ); - const foo = await loadJsonFile("foo.json", mockSchema, migrations); + const schema1 = { parseAsync: jest.fn() } as unknown as ZodSchema; + const schema2 = { + parseAsync: jest.fn().mockResolvedValue({ + version: 2, + foo: "dog", + bar: 24, + }), + } as unknown as ZodSchema; + + const { content } = await loadJsonFile( + "fake-file-path.json", + { + 1: schema1, + 2: schema2, + }, + { + version: 2, + foo: "cat", + bar: 42, + } + ); - expect(foo.content.version).toBe(2); - expect(foo.content.name).toBe("name"); - expect(foo.content.age).toBe(42); + expect(content).toEqual({ + version: 2, + foo: "dog", + bar: 24, + }); + expect(schema1.parseAsync).not.toBeCalled(); + expect(schema2.parseAsync).toBeCalled(); + expect(fsp.writeFile).not.toBeCalled(); }); -test("loadJsonFile update partial update", async () => { - const migrations = [new InitialDefinition(), new AddAge()]; - +test("loadJsonFile update validates content before saving to file.", async () => { (fs.existsSync as jest.Mock).mockReturnValueOnce(true); - (fsp.readFile as jest.Mock).mockResolvedValueOnce(null); + (fsp.readFile as jest.Mock).mockResolvedValueOnce( + ` + { + "version": 2, + "foo": "dog", + "bar": 24 + } + ` + ); - const foo = await loadJsonFile("foo.json", mockSchema, migrations); + const schema2 = { + parseAsync: jest.fn().mockResolvedValue({ + version: 2, + foo: "horse", + bar: 24, + }), + } as unknown as ZodSchema; + + const fileHandler = await loadJsonFile( + "fake-file-path.json", + { + 1: fooV1, + 2: schema2, + }, + { + version: 2, + foo: "cat", + bar: 42, + } + ); + + expect(fsp.writeFile).not.toBeCalled(); - await foo.update({ - age: 23, + await fileHandler.update({ + foo: "horse", }); - expect(foo.content).toEqual({ + expect(fileHandler.content.foo).toBe("horse"); + expect(fsp.writeFile).toBeCalledWith( + "fake-file-path.json", + `{"version":2,"foo":"horse","bar":24}`, + { encoding: "utf-8" } + ); + expect(schema2.parseAsync).toBeCalledWith({ version: 2, - age: 23, - name: "name", + foo: "horse", + bar: 24, }); }); -const fooV1 = z.object({ - version: z.literal(1), - name: z.string().default("name"), -}); -type FooV1 = z.infer; - -const fooV2 = z.object({ - version: z.literal(2), - name: z.string().default("name"), - age: z.number().default(42), -}); -type FooV2 = z.infer & { version: number }; - -class InitialDefinition extends JsonMigration { - version = 1; - - async validateInput(input: unknown): Promise { - return await fooV1.parseAsync(input); - } +test("loadJsonFile migrates when loading older content", async () => { + (fs.existsSync as jest.Mock).mockReturnValueOnce(true); + (fsp.readFile as jest.Mock).mockResolvedValueOnce( + `{"version": 1,"foo": "dog"}` + ); - protected async migrate(input: FooV1): Promise { - return { + const schema1 = { + parseAsync: jest.fn().mockResolvedValue({ version: 1, - name: input.name, - }; - } -} - -class AddAge extends JsonMigration { - version = 2; - - async validateInput(input: unknown): Promise { - return await fooV1.parseAsync(input); - } - - protected async migrate(input: FooV1): Promise { - return { + foo: "dog", + }), + } as unknown as ZodSchema; + const schema2 = { + parseAsync: jest.fn().mockResolvedValue({ version: 2, - age: 42, - name: input.name, - }; - } -} + foo: "dog", + bar: 24, + }), + } as unknown as ZodSchema; + + const fileHandler = await loadJsonFile( + "fake-file-path.json", + { + 1: schema1, + 2: schema2, + }, + { + version: 2, + foo: "cat", + bar: 42, + } + ); + + expect(fileHandler.content).toEqual({ + version: 2, + foo: "dog", + bar: 24, + }); + expect(schema1.parseAsync).toBeCalled(); + expect(schema2.parseAsync).toBeCalled(); + expect(fsp.writeFile).toBeCalledWith( + "fake-file-path.json", + `{"version":2,"foo":"dog","bar":24}`, + { encoding: "utf-8" } + ); +}); diff --git a/test/main/shortcuts.spec.ts b/test/main/shortcuts.spec.ts index d36b9954..3e18f83d 100644 --- a/test/main/shortcuts.spec.ts +++ b/test/main/shortcuts.spec.ts @@ -1,121 +1,121 @@ -import { createConfig } from "../__factories__/config"; -import { createIpcMainTS } from "../__factories__/ipc"; -import { createJsonFile } from "../__factories__/jsonFile"; -import { shortcutIpcs } from "../../src/main/shortcuts"; -import { loadJsonFile } from "../../src/main/json"; -import { Section } from "../../src/shared/ui/app"; -import { parseKeyCodes } from "../../src/shared/io/keyCode"; +// import { createConfig } from "../__factories__/config"; +// import { createIpcMainTS } from "../__factories__/ipc"; +// import { createJsonFile } from "../__factories__/jsonFile"; +// import { shortcutIpcs } from "../../src/main/shortcuts"; +// import { loadJsonFile } from "../../src/main/json"; +// import { Section } from "../../src/shared/ui/app"; +// import { parseKeyCodes } from "../../src/shared/io/keyCode"; -jest.mock("fs"); -jest.mock("fs/promises"); -jest.mock("../../src/main/json"); +// jest.mock("fs"); +// jest.mock("fs/promises"); +// jest.mock("../../src/main/json"); -test("shortcutIpcs init", async () => { - const ipc = createIpcMainTS(); - const config = createJsonFile(createConfig()); +// test("shortcutIpcs init", async () => { +// const ipc = createIpcMainTS(); +// const config = createJsonFile(createConfig()); - (loadJsonFile as jest.Mock).mockResolvedValueOnce({ - content: { - shortcuts: [ - // Disable existing shortcut - { - name: "sidebar.focus", - disabled: true, - }, - // Change keys existing shortcut - { - name: "app.toggleSidebar", - keys: "control+alt+s", - }, - // Change when existing shortcut - { - name: "app.quit", - when: Section.Sidebar, - }, - // Change event input existing shortcut - { - name: "notes.create", - eventInput: "foo", - }, - // Change repeat existing shortcut - { - name: "app.reload", - repeat: true, - }, - // Add a new shortcut - { - name: "openDataDirectory2", - event: "app.openDataDirectory", - keys: "control+o+d+f", - when: Section.Editor, - repeat: false, - }, - ], - }, - update: jest.fn(), - }); +// (loadJsonFile as jest.Mock).mockResolvedValueOnce({ +// content: { +// shortcuts: [ +// // Disable existing shortcut +// { +// name: "sidebar.focus", +// disabled: true, +// }, +// // Change keys existing shortcut +// { +// name: "app.toggleSidebar", +// keys: "control+alt+s", +// }, +// // Change when existing shortcut +// { +// name: "app.quit", +// when: Section.Sidebar, +// }, +// // Change event input existing shortcut +// { +// name: "notes.create", +// eventInput: "foo", +// }, +// // Change repeat existing shortcut +// { +// name: "app.reload", +// repeat: true, +// }, +// // Add a new shortcut +// { +// name: "openDataDirectory2", +// event: "app.openDataDirectory", +// keys: "control+o+d+f", +// when: Section.Editor, +// repeat: false, +// }, +// ], +// }, +// update: jest.fn(), +// }); - shortcutIpcs(ipc, config); - await ipc.trigger("init"); +// shortcutIpcs(ipc, config); +// await ipc.trigger("init"); - const shortcuts = await ipc.invoke("shortcuts.getAll"); +// const shortcuts = await ipc.invoke("shortcuts.getAll"); - const sidebarFocus = shortcuts.find((s) => s.name === "sidebar.focus"); - expect(sidebarFocus).toEqual( - expect.objectContaining({ - disabled: true, - event: "focus.push", - name: "sidebar.focus", - }) - ); +// const sidebarFocus = shortcuts.find((s) => s.name === "sidebar.focus"); +// expect(sidebarFocus).toEqual( +// expect.objectContaining({ +// disabled: true, +// event: "focus.push", +// name: "sidebar.focus", +// }) +// ); - const appToggleSidebar = shortcuts.find( - (s) => s.name === "app.toggleSidebar" - ); - expect(appToggleSidebar).toEqual( - expect.objectContaining({ - name: "app.toggleSidebar", - event: "app.toggleSidebar", - keys: parseKeyCodes("control+alt+s"), - }) - ); +// const appToggleSidebar = shortcuts.find( +// (s) => s.name === "app.toggleSidebar" +// ); +// expect(appToggleSidebar).toEqual( +// expect.objectContaining({ +// name: "app.toggleSidebar", +// event: "app.toggleSidebar", +// keys: parseKeyCodes("control+alt+s"), +// }) +// ); - const appQuit = shortcuts.find((s) => s.name === "app.quit"); - expect(appQuit).toEqual( - expect.objectContaining({ - name: "app.quit", - when: Section.Sidebar, - event: "app.quit", - }) - ); +// const appQuit = shortcuts.find((s) => s.name === "app.quit"); +// expect(appQuit).toEqual( +// expect.objectContaining({ +// name: "app.quit", +// when: Section.Sidebar, +// event: "app.quit", +// }) +// ); - const createNote = shortcuts.find((s) => s.name === "notes.create"); - expect(createNote).toEqual( - expect.objectContaining({ - name: "notes.create", - event: "sidebar.createNote", - eventInput: "foo", - }) - ); +// const createNote = shortcuts.find((s) => s.name === "notes.create"); +// expect(createNote).toEqual( +// expect.objectContaining({ +// name: "notes.create", +// event: "sidebar.createNote", +// eventInput: "foo", +// }) +// ); - const appReload = shortcuts.find((s) => s.name === "app.reload"); - expect(appReload).toEqual( - expect.objectContaining({ - name: "app.reload", - event: "app.reload", - repeat: true, - }) - ); +// const appReload = shortcuts.find((s) => s.name === "app.reload"); +// expect(appReload).toEqual( +// expect.objectContaining({ +// name: "app.reload", +// event: "app.reload", +// repeat: true, +// }) +// ); - const openDataDirectory = shortcuts.find( - (s) => s.name === "openDataDirectory2" - ); - expect(openDataDirectory).toEqual( - expect.objectContaining({ - name: "openDataDirectory2", - event: "app.openDataDirectory", - when: Section.Editor, - repeat: false, - }) - ); -}); +// const openDataDirectory = shortcuts.find( +// (s) => s.name === "openDataDirectory2" +// ); +// expect(openDataDirectory).toEqual( +// expect.objectContaining({ +// name: "openDataDirectory2", +// event: "app.openDataDirectory", +// when: Section.Editor, +// repeat: false, +// }) +// ); +// }); diff --git a/test/shared/domain/index.spec.ts b/test/shared/domain/index.spec.ts index 1fb52ea9..b4af845c 100644 --- a/test/shared/domain/index.spec.ts +++ b/test/shared/domain/index.spec.ts @@ -10,11 +10,11 @@ test("DATE_OR_STRING_SCHEMA", async () => { // In the past we've had some deserialize bugs pop up because JSON stores dates // as strings. This test is just to help prevent regressions. - const date = new Date("2020-01-01T20:37:21.765Z") + const date = new Date("2020-01-01T20:37:21.765Z"); const parsed = await DATE_OR_STRING_SCHEMA.parseAsync(date); expect(isEqual(date, parsed)).toBe(true); - + const serializedDate = "2022-09-09T20:37:21.765Z"; const parsedSerializedDate = parseJSON(serializedDate); expect(isEqual(parsedSerializedDate, parsedSerializedDate)).toBe(true); -} +}); diff --git a/tsconfig.json b/tsconfig.json index 6c372091..3876d98a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,6 +12,7 @@ "outDir": "dist", "moduleResolution": "node", "resolveJsonModule": true, + "strict": true, "strictNullChecks": true, "paths": { "*": [ diff --git a/tsconfig.test.json b/tsconfig.test.json index 34c25c71..f908854f 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -13,6 +13,7 @@ "moduleResolution": "node", "resolveJsonModule": true, "strictNullChecks": true, + "strict": true, "paths": { "*": [ "node_modules/*" From ed8f51a29b70b436531e1f1b0a65eb45c1627843 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sat, 24 Sep 2022 21:21:27 -0400 Subject: [PATCH 2/9] Fix all types to support strict mode --- src/main/app.ts | 77 ++---- src/main/index.ts | 12 +- src/main/json.ts | 9 +- .../schemas/appState/1_initialDefinition.ts | 81 +++---- src/main/schemas/appState/index.ts | 6 +- src/main/schemas/config/2_addLogDirectory.ts | 3 +- .../schemas/shortcuts/1_initialDefinition.ts | 41 ++-- src/main/schemas/shortcuts/index.ts | 8 +- src/main/shortcuts.ts | 12 +- src/renderer/components/Editor.tsx | 5 +- src/renderer/components/Markdown.tsx | 7 +- src/renderer/components/Sidebar.tsx | 4 +- src/renderer/io/mouse.ts | 4 +- src/renderer/menus/appMenu.ts | 5 +- src/renderer/menus/contextMenu.ts | 5 +- src/renderer/preload.ts | 8 + src/renderer/store.ts | 6 +- src/shared/domain/config.ts | 1 + src/shared/ui/app.ts | 1 + test/main/app.spec.ts | 36 +-- test/main/index.spec.ts | 99 ++++---- test/main/json.spec.ts | 4 +- test/main/shortcuts.spec.ts | 220 +++++++++--------- 23 files changed, 314 insertions(+), 340 deletions(-) diff --git a/src/main/app.ts b/src/main/app.ts index fc49182c..2b421b12 100644 --- a/src/main/app.ts +++ b/src/main/app.ts @@ -8,67 +8,17 @@ import { isRoleMenu, Menu as MenuType } from "../shared/ui/menu"; import { IpcChannel, IpcMainTS } from "../shared/ipc"; import { openInBrowser } from "./utils"; import { UIEventType, UIEventInput } from "../shared/ui/events"; -import { AppState, DEFAULT_SIDEBAR_WIDTH, Section } from "../shared/ui/app"; -import { z } from "zod"; -import { - NoteSort, - DEFAULT_NOTE_SORTING_ALGORITHM, -} from "../shared/domain/note"; +import { AppState, DEFAULT_SIDEBAR_WIDTH } from "../shared/ui/app"; + import { JsonFile, loadJsonFile } from "./json"; import { Config } from "../shared/domain/config"; import p from "path"; import { MissingDataDirectoryError } from "../shared/errors"; -import { DATE_OR_STRING_SCHEMA } from "../shared/domain"; -import { APP_STATE_MIGRATIONS } from "./schemas/appState"; +import { NoteSort } from "../shared/domain/note"; +import { APP_STATE_SCHEMAS } from "./schemas/appState"; export const APP_STATE_PATH = "ui.json"; -export const APP_STATE_SCHEMA = z - .object({ - version: z.literal(1).optional().default(1), - sidebar: z - .object({ - width: z - .string() - .regex(/^\d+px$/) - .optional() - .default(DEFAULT_SIDEBAR_WIDTH), - scroll: z.number().optional().default(0), - hidden: z.boolean().optional(), - selected: z.array(z.string()).optional(), - expanded: z.array(z.string()).optional(), - sort: z - .nativeEnum(NoteSort) - .optional() - .default(DEFAULT_NOTE_SORTING_ALGORITHM), - }) - .optional() - .default({}), - editor: z - .object({ - isEditing: z.boolean().optional().default(false), - scroll: z.number().optional().default(0), - tabs: z - .array( - z.object({ - noteId: z.string(), - // Intentionally omitted noteContent - lastActive: DATE_OR_STRING_SCHEMA.optional(), - }) - ) - .default([]), - tabsScroll: z.number().optional().default(0), - activeTabNoteId: z.string().optional(), - }) - .optional() - .default({}), - focused: z.array(z.nativeEnum(Section)).default([]), - }) - .optional() - .default({ - version: 1, - }); - export function appIpcs(ipc: IpcMainTS, config: JsonFile): void { let appStateFile: JsonFile; @@ -79,9 +29,22 @@ export function appIpcs(ipc: IpcMainTS, config: JsonFile): void { appStateFile = await loadJsonFile( p.join(config.content.dataDirectory, APP_STATE_PATH), - // APP_STATE_SCHEMA, - // [appStateSchemaV1] - APP_STATE_MIGRATIONS + APP_STATE_SCHEMAS, + { + version: 1, + sidebar: { + scroll: 0, + sort: NoteSort.Alphanumeric, + width: DEFAULT_SIDEBAR_WIDTH, + }, + editor: { + isEditing: false, + scroll: 0, + tabs: [], + tabsScroll: 0, + }, + focused: [], + } ); }); diff --git a/src/main/index.ts b/src/main/index.ts index 25bca7b4..5aa91348 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -8,7 +8,7 @@ import { openInBrowser } from "./utils"; import * as path from "path"; import * as fs from "fs"; import * as fsp from "fs/promises"; -import { loadJsonFile2 } from "./json"; +import { loadJsonFile } from "./json"; import { CONFIG_SCHEMAS } from "./schemas/config"; import { Config } from "../shared/domain/config"; import { shortcutIpcs } from "./shortcuts"; @@ -34,9 +34,15 @@ export const DEFAULT_WINDOW_WIDTH = 800; let mainWindow: BrowserWindow; export async function main(): Promise { - const configFile = await loadJsonFile2( + const configFile = await loadJsonFile( getConfigPath(), - CONFIG_SCHEMAS + CONFIG_SCHEMAS, + { + version: 2, + windowHeight: DEFAULT_WINDOW_HEIGHT, + windowWidth: DEFAULT_WINDOW_WIDTH, + logDirectory: app.getPath("logs"), + } ); if (isDevelopment() && configFile.content.dataDirectory == null) { diff --git a/src/main/json.ts b/src/main/json.ts index 4c258a5e..600d6e44 100644 --- a/src/main/json.ts +++ b/src/main/json.ts @@ -12,9 +12,7 @@ export interface JsonFile { export async function loadJsonFile( filePath: string, - schemas: { - [version: number]: ZodSchema; - }, + schemas: Record, defaultContent: Content ): Promise> { const schemaArray = Object.entries(schemas) @@ -47,6 +45,11 @@ export async function loadJsonFile( const relevantSchemas = schemaArray.filter( ([version]) => originalContent!.version <= version ); + if (relevantSchemas.length === 0) { + throw new Error( + `File ${filePath} has no schemas to run. Loaded content version was: ${originalContent.version}` + ); + } let content = cloneDeep(originalContent); for (const [, schema] of relevantSchemas) { diff --git a/src/main/schemas/appState/1_initialDefinition.ts b/src/main/schemas/appState/1_initialDefinition.ts index 372911b0..e93e580c 100644 --- a/src/main/schemas/appState/1_initialDefinition.ts +++ b/src/main/schemas/appState/1_initialDefinition.ts @@ -1,33 +1,51 @@ -import { parseJSON } from "date-fns"; import { z } from "zod"; import { DATE_OR_STRING_SCHEMA } from "../../../shared/domain"; -import { - DEFAULT_NOTE_SORTING_ALGORITHM, - NoteSort, -} from "../../../shared/domain/note"; -import { - AppState, - DEFAULT_SIDEBAR_WIDTH, - Section, -} from "../../../shared/ui/app"; -import { JsonMigration } from "../../json"; +import { NoteSort } from "../../../shared/domain/note"; +import { Section } from "../../../shared/ui/app"; -export const appStateSchemaV1 = z.object({ - version: z.literal(1).optional().default(1), +interface AppStateV1 { + sidebar: Sidebar; + editor: Editor; + focused: Section[]; +} + +export interface Sidebar { + searchString?: string; + hidden?: boolean; + width: string; + scroll: number; + selected?: string[]; + expanded?: string[]; + sort: NoteSort; +} + +export interface Editor { + isEditing: boolean; + scroll: number; + tabs: EditorTab[]; + tabsScroll: number; + activeTabNoteId?: string; +} + +export interface EditorTab { + noteId: string; + noteContent?: string; + lastActive?: Date | string; +} + +export const appStateV1: z.Schema = z.object({ + version: z.literal(1), sidebar: z.object({ - width: z - .string() - .regex(/^\d+px$/) - .default(DEFAULT_SIDEBAR_WIDTH), - scroll: z.number().default(0), + width: z.string().regex(/^\d+px$/), + scroll: z.number(), hidden: z.boolean().optional(), selected: z.array(z.string()).optional(), expanded: z.array(z.string()).optional(), - sort: z.nativeEnum(NoteSort).default(DEFAULT_NOTE_SORTING_ALGORITHM), + sort: z.nativeEnum(NoteSort), }), editor: z.object({ - isEditing: z.boolean().default(false), - scroll: z.number().default(0), + isEditing: z.boolean(), + scroll: z.number(), tabs: z.array( z.object({ noteId: z.string(), @@ -35,25 +53,8 @@ export const appStateSchemaV1 = z.object({ lastActive: DATE_OR_STRING_SCHEMA.optional(), }) ), - tabsScroll: z.number().default(0), + tabsScroll: z.number(), activeTabNoteId: z.string().optional(), }), - focused: z.array(z.nativeEnum(Section)).default([]), + focused: z.array(z.nativeEnum(Section)), }); - -type AppStateV1 = z.infer; - -export class AppStateInitialDefinition extends JsonMigration< - AppStateV1, - AppState -> { - version = 1; - - async validateInput(input: unknown): Promise { - return await appStateSchemaV1.parseAsync(input); - } - - protected async migrate(input: AppStateV1): Promise { - return input; - } -} diff --git a/src/main/schemas/appState/index.ts b/src/main/schemas/appState/index.ts index 19d5ebea..297e2e84 100644 --- a/src/main/schemas/appState/index.ts +++ b/src/main/schemas/appState/index.ts @@ -1,3 +1,5 @@ -import { AppStateInitialDefinition } from "./1_initialDefinition"; +import { appStateV1 } from "./1_initialDefinition"; -export const APP_STATE_MIGRATIONS = [new AppStateInitialDefinition()]; +export const APP_STATE_SCHEMAS = { + 1: appStateV1, +}; diff --git a/src/main/schemas/config/2_addLogDirectory.ts b/src/main/schemas/config/2_addLogDirectory.ts index bd22bcfe..6d6057f7 100644 --- a/src/main/schemas/config/2_addLogDirectory.ts +++ b/src/main/schemas/config/2_addLogDirectory.ts @@ -18,6 +18,7 @@ export const configSchemaV2 = z.preprocess( if (config.version === 1) { return { ...config, + version: 2, logDirectory: app.getPath("logs"), }; } @@ -25,7 +26,7 @@ export const configSchemaV2 = z.preprocess( return config; }, z.object({ - version: z.literal(1).transform(() => 2), + version: z.literal(2), windowHeight: z.number().min(1).default(DEFAULT_WINDOW_HEIGHT), windowWidth: z.number().min(1).default(DEFAULT_WINDOW_WIDTH), dataDirectory: z.string().optional(), diff --git a/src/main/schemas/shortcuts/1_initialDefinition.ts b/src/main/schemas/shortcuts/1_initialDefinition.ts index 47aec96b..6ac03cb5 100644 --- a/src/main/schemas/shortcuts/1_initialDefinition.ts +++ b/src/main/schemas/shortcuts/1_initialDefinition.ts @@ -1,10 +1,24 @@ import { z } from "zod"; import { Section } from "../../../shared/ui/app"; import { LIST_OF_EVENTS, UIEventType } from "../../../shared/ui/events"; -import { JsonMigration } from "../../json"; -import { Shortcuts } from "../../shortcuts"; -export const OVERRIDE_SCHEMA = z.object({ +interface ShortcutOverrideV1 { + name?: string | undefined; + event?: string | undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + eventInput?: any | undefined; + keys?: string | undefined; + when?: Section | undefined; + disabled?: boolean | undefined; + repeat?: boolean | undefined; +} + +interface ShortcutsV1 { + version: 1; + shortcuts?: ShortcutOverrideV1[]; +} + +export const OVERRIDE_SCHEMA: z.Schema = z.object({ // Name is how we determine whether it's an existing shortcut being updated, // or a new shortcut being added. name: z.string(), @@ -19,24 +33,7 @@ export const OVERRIDE_SCHEMA = z.object({ repeat: z.boolean().optional(), }); -export const SHORTCUTS_SCHEMA_V1 = z.object({ - version: z.literal(1).optional().default(1), +export const shortcutsV1: z.Schema = z.object({ + version: z.literal(1), shortcuts: z.array(OVERRIDE_SCHEMA).optional(), }); - -type ShortcutsV1 = z.infer; - -export class ShortcutsInitialDefinition extends JsonMigration< - ShortcutsV1, - Shortcuts -> { - version = 1; - - async validateInput(input: unknown): Promise { - return await SHORTCUTS_SCHEMA_V1.parseAsync(input); - } - - protected async migrate(input: ShortcutsV1): Promise { - return input as unknown as Shortcuts; - } -} diff --git a/src/main/schemas/shortcuts/index.ts b/src/main/schemas/shortcuts/index.ts index ca86d74a..de549904 100644 --- a/src/main/schemas/shortcuts/index.ts +++ b/src/main/schemas/shortcuts/index.ts @@ -1,7 +1,3 @@ -import { JsonMigration } from "../../json"; -import { Shortcuts } from "../../shortcuts"; -import { ShortcutsInitialDefinition } from "./1_initialDefinition"; +import { shortcutsV1 } from "./1_initialDefinition"; -export const SHORTCUT_FILE_MIGRATIONS: JsonMigration[] = [ - new ShortcutsInitialDefinition(), -]; +export const SHORTCUTS_SCHEMAS = { 1: shortcutsV1 }; diff --git a/src/main/shortcuts.ts b/src/main/shortcuts.ts index b75da4c6..3260673c 100644 --- a/src/main/shortcuts.ts +++ b/src/main/shortcuts.ts @@ -8,8 +8,7 @@ import { Section } from "../shared/ui/app"; import { UIEventInput, UIEventType } from "../shared/ui/events"; import { JsonFile, loadJsonFile } from "./json"; import p from "path"; -import { SHORTCUT_FILE_MIGRATIONS } from "./schemas/shortcuts"; -import { OVERRIDE_SCHEMA } from "./schemas/shortcuts/1_initialDefinition"; +import { SHORTCUTS_SCHEMAS } from "./schemas/shortcuts"; export interface Shortcuts { version: number; @@ -26,11 +25,6 @@ export interface Shortcuts { export const SHORTCUT_FILE_PATH = "shortcuts.json"; -export const SHORTCUT_FILE_SCHEMA = z.object({ - version: z.literal(1).optional().default(1), - shortcuts: z.array(OVERRIDE_SCHEMA).optional(), -}); - export interface ShortcutOverride { name: string; event?: UIEventType; @@ -47,8 +41,8 @@ export function shortcutIpcs(ipc: IpcMainTS, config: JsonFile): void { ipc.on("init", async () => { const shortcutFile = await loadJsonFile( p.join(config.content.dataDirectory!, SHORTCUT_FILE_PATH), - SHORTCUT_FILE_SCHEMA, - SHORTCUT_FILE_MIGRATIONS + SHORTCUTS_SCHEMAS, + { version: 1, shortcuts: [] } ); const overrides = shortcutFile.content.shortcuts ?? []; diff --git a/src/renderer/components/Editor.tsx b/src/renderer/components/Editor.tsx index 83e8bde1..81fa547f 100644 --- a/src/renderer/components/Editor.tsx +++ b/src/renderer/components/Editor.tsx @@ -79,7 +79,10 @@ const StyledContent = styled.div` overflow: hidden; `; -const debouncedInvoker = debounce(window.ipc, NOTE_SAVE_INTERVAL_MS) as Ipc; +const debouncedInvoker = debounce( + window.ipc, + NOTE_SAVE_INTERVAL_MS +) as unknown as Ipc; const setContent: Listener<"editor.setContent"> = async ({ value }, ctx) => { if (value == null) { diff --git a/src/renderer/components/Markdown.tsx b/src/renderer/components/Markdown.tsx index 9d1551d5..e5fabaa1 100644 --- a/src/renderer/components/Markdown.tsx +++ b/src/renderer/components/Markdown.tsx @@ -20,9 +20,6 @@ export interface MarkdownProps { onScroll: (newVal: number) => void; } -// TODO: Get better typing on this. -type Props = Record; - export function Markdown(props: MarkdownProps): JSX.Element { // Check for update so we can migrate to newer versions of remarkGFM // https://github.com/remarkjs/react-remark/issues/50 @@ -43,7 +40,7 @@ export function Markdown(props: MarkdownProps): JSX.Element { code: CodeSpan, span: Text, image: Image, - a: (p: Props) => ( + a: (p: any) => ( {p.children} @@ -55,7 +52,7 @@ export function Markdown(props: MarkdownProps): JSX.Element { em: Em, ul: UnorderedList, ol: OrderedList, - li: (p: Props) => { + li: (p: any) => { if (p.className === "task-list-item") { return
  • {p.children}
    • ; } else { diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index d858255c..5b6e9d30 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -457,7 +457,7 @@ export const createNote: Listener<"sidebar.createNote"> = async ( }, })); } catch (e) { - promptError(e.message); + promptError((e as Error).message); } } @@ -517,7 +517,7 @@ export const renameNote: Listener<"sidebar.renameNote"> = async ( } }); } catch (e) { - promptError(e.message); + promptError((e as Error).message); } } diff --git a/src/renderer/io/mouse.ts b/src/renderer/io/mouse.ts index 042d159d..24768026 100644 --- a/src/renderer/io/mouse.ts +++ b/src/renderer/io/mouse.ts @@ -181,12 +181,12 @@ export function useMouseDrag( } }; - el.addEventListener("mousedown", onMouseDown); + (el as HTMLElement).addEventListener("mousedown", onMouseDown); window.addEventListener("mousemove", onMouseMove); window.addEventListener("mouseup", onMouseUp); window.addEventListener("keyup", onKeyUp); return () => { - el.removeEventListener("mousedown", onMouseDown); + (el as HTMLElement).removeEventListener("mousedown", onMouseDown); window.removeEventListener("mousemove", onMouseMove); window.removeEventListener("mouseup", onMouseUp); window.removeEventListener("keyup", onKeyUp); diff --git a/src/renderer/menus/appMenu.ts b/src/renderer/menus/appMenu.ts index 6477a2d7..fdc2bb3f 100644 --- a/src/renderer/menus/appMenu.ts +++ b/src/renderer/menus/appMenu.ts @@ -3,6 +3,7 @@ import { Menu } from "../../shared/ui/menu"; import { isDevelopment } from "../../shared/env"; import { getShortcutLabels } from "../io/shortcuts"; import { Store } from "../store"; +import { IpcChannel } from "../../shared/ipc"; export function useApplicationMenu(store: Store): void { const { state } = store; @@ -141,9 +142,9 @@ export function useApplicationMenu(store: Store): void { store.dispatch(event, eventInput); }; - window.addEventListener("applicationmenuclick", onClick); + window.addEventListener(IpcChannel.ApplicationMenuClick, onClick); return () => { - window.removeEventListener("applicationmenuclick", onClick); + window.removeEventListener(IpcChannel.ApplicationMenuClick, onClick); }; }, [store]); } diff --git a/src/renderer/menus/contextMenu.ts b/src/renderer/menus/contextMenu.ts index 1e4b72c1..9fb58660 100644 --- a/src/renderer/menus/contextMenu.ts +++ b/src/renderer/menus/contextMenu.ts @@ -14,6 +14,7 @@ import { getShortcutLabels } from "../io/shortcuts"; import { Store } from "../store"; import { Section } from "../../shared/ui/app"; import { getEditorTabAttribute } from "../components/EditorTabs"; +import { IpcChannel } from "../../shared/ipc"; export function useContextMenu(store: Store): void { const { state } = store; @@ -177,9 +178,9 @@ export function useContextMenu(store: Store): void { store.dispatch(event, eventInput); }; - window.addEventListener("contextmenuclick", onClick); + window.addEventListener(IpcChannel.ContextMenuClick, onClick); return () => { - window.removeEventListener("contextmenuclick", onClick); + window.removeEventListener(IpcChannel.ContextMenuClick, onClick); }; }, [store]); } diff --git a/src/renderer/preload.ts b/src/renderer/preload.ts index 1a9638ea..87d7ca5e 100644 --- a/src/renderer/preload.ts +++ b/src/renderer/preload.ts @@ -23,6 +23,14 @@ contextBridge.exposeInMainWorld("ipc", (ipcType: IpcType, ...data: []) => { declare global { interface Window { ipc: Ipc; + addEventListener( + type: IpcChannel, + l: (this: Window, ev: CustomEvent) => void | Promise + ): void; + removeEventListener( + type: IpcChannel, + l: (this: Window, ev: CustomEvent) => void | Promise + ): void; } } diff --git a/src/renderer/store.ts b/src/renderer/store.ts index ce0736a1..ad44e9a4 100644 --- a/src/renderer/store.ts +++ b/src/renderer/store.ts @@ -79,7 +79,7 @@ export function useStore(initialState: State): Store { const setUI: SetUI = useCallback((transformer) => { setState((prevState) => { - const prevUI = pick(prevState, "editor", "sidebar", "focused"); + const prevUI = pick(prevState, "version", "editor", "sidebar", "focused"); const updates = typeof transformer === "function" ? transformer(prevUI) : transformer; @@ -92,7 +92,7 @@ export function useStore(initialState: State): Store { // We need to delete some values before sending them over to the main // thread otherwise electron will throw an error. - const newUI = pick(newState, "editor", "sidebar", "focused"); + const newUI = pick(newState, "version", "editor", "sidebar", "focused"); const clonedUI = cloneDeep(newUI); if (clonedUI?.sidebar != null) { delete clonedUI.sidebar.input; @@ -192,7 +192,7 @@ export function useStore(initialState: State): Store { } }, [ctx] - ); + ) as Dispatch; const on: On = ( event: ET | ET[], diff --git a/src/shared/domain/config.ts b/src/shared/domain/config.ts index 7d457d3b..fdca6316 100644 --- a/src/shared/domain/config.ts +++ b/src/shared/domain/config.ts @@ -1,4 +1,5 @@ export interface Config { + version: number; windowHeight: number; windowWidth: number; dataDirectory?: string; diff --git a/src/shared/ui/app.ts b/src/shared/ui/app.ts index c11d62d8..3de55056 100644 --- a/src/shared/ui/app.ts +++ b/src/shared/ui/app.ts @@ -5,6 +5,7 @@ import { Note, NoteSort } from "../domain/note"; export const DEFAULT_SIDEBAR_WIDTH = "300px"; export interface AppState { + version: number; sidebar: Sidebar; editor: Editor; focused: Section[]; diff --git a/test/main/app.spec.ts b/test/main/app.spec.ts index eec4523f..914efc6e 100644 --- a/test/main/app.spec.ts +++ b/test/main/app.spec.ts @@ -1,21 +1,21 @@ -// import { createConfig } from "../__factories__/config"; -// import { createIpcMainTS } from "../__factories__/ipc"; -// import { appIpcs } from "../../src/main/app"; -// import { createJsonFile } from "../__factories__/jsonFile"; -// import { BrowserWindow } from "../__mocks__/electron"; +import { createConfig } from "../__factories__/config"; +import { createIpcMainTS } from "../__factories__/ipc"; +import { appIpcs } from "../../src/main/app"; +import { createJsonFile } from "../__factories__/jsonFile"; +import { BrowserWindow } from "../__mocks__/electron"; -// test("app.inspectElement rounds floats", async () => { -// const ipc = createIpcMainTS(); -// const config = createJsonFile(createConfig()); -// appIpcs(ipc, config); +test("app.inspectElement rounds floats", async () => { + const ipc = createIpcMainTS(); + const config = createJsonFile(createConfig()); + appIpcs(ipc, config); -// const inspectElement = jest.fn(); -// BrowserWindow.getFocusedWindow.mockImplementationOnce(() => ({ -// webContents: { -// inspectElement, -// }, -// })); + const inspectElement = jest.fn(); + BrowserWindow.getFocusedWindow.mockImplementationOnce(() => ({ + webContents: { + inspectElement, + }, + })); -// await ipc.invoke("app.inspectElement", { x: 1.23, y: 2.67 }); -// expect(inspectElement).toBeCalledWith(1, 3); -// }); + await ipc.invoke("app.inspectElement", { x: 1.23, y: 2.67 }); + expect(inspectElement).toBeCalledWith(1, 3); +}); diff --git a/test/main/index.spec.ts b/test/main/index.spec.ts index caf6705b..3a8cd11b 100644 --- a/test/main/index.spec.ts +++ b/test/main/index.spec.ts @@ -1,50 +1,49 @@ -// import * as json from "../../src/main/json"; -// import { DEFAULT_DEV_DATA_DIRECTORY, main } from "../../src/main/index"; -// import fsp from "fs/promises"; -// import fs from "fs"; -// import * as env from "../../src/shared/env"; -// import { loadJsonFile } from "../../src/main/json"; - -// jest.mock("fs/promises"); -// jest.mock("fs"); -// jest.mock("../../src/main/json"); - -// test("main defaults data directory in development", async () => { -// const update = jest.fn(); - -// (loadJsonFile as jest.Mock).mockResolvedValueOnce({ -// content: { -// dataDirectory: null, -// windowHeight: 800, -// windowWidth: 600, -// }, -// update, -// }); - -// // We use spyOn instead of mocking entire module because we need getProcessType -// // to function normal. -// jest.spyOn(env, "isDevelopment").mockReturnValue(true); - -// await main(); -// expect(update).toHaveBeenCalledWith({ -// dataDirectory: DEFAULT_DEV_DATA_DIRECTORY, -// }); -// }); - -// test("main creates data directory if directory is missing.", async () => { -// jest.spyOn(json, "loadJsonFile").mockResolvedValueOnce({ -// content: { -// dataDirectory: "foo", -// windowHeight: 800, -// windowWidth: 600, -// }, -// update: jest.fn(), -// }); - -// const mkdir = jest.fn(); -// (fsp.mkdir as jest.Mock).mockImplementation(mkdir); -// (fs.existsSync as jest.Mock).mockReturnValue(false); - -// await main(); -// expect(mkdir).toHaveBeenCalledWith("foo"); -// }); +import { DEFAULT_DEV_DATA_DIRECTORY, main } from "../../src/main/index"; +import fsp from "fs/promises"; +import fs from "fs"; +import * as env from "../../src/shared/env"; +import { loadJsonFile } from "../../src/main/json"; + +jest.mock("fs/promises"); +jest.mock("fs"); +jest.mock("../../src/main/json"); + +test("main defaults data directory in development", async () => { + const update = jest.fn(); + + (loadJsonFile as jest.Mock).mockResolvedValueOnce({ + content: { + dataDirectory: null, + windowHeight: 800, + windowWidth: 600, + }, + update, + }); + + // We use spyOn instead of mocking entire module because we need getProcessType + // to function normal. + jest.spyOn(env, "isDevelopment").mockReturnValue(true); + + await main(); + expect(update).toHaveBeenCalledWith({ + dataDirectory: DEFAULT_DEV_DATA_DIRECTORY, + }); +}); + +test("main creates data directory if directory is missing.", async () => { + (loadJsonFile as jest.Mock).mockResolvedValueOnce({ + content: { + dataDirectory: "foo", + windowHeight: 800, + windowWidth: 600, + }, + update: jest.fn(), + }); + + const mkdir = jest.fn(); + (fsp.mkdir as jest.Mock).mockImplementation(mkdir); + (fs.existsSync as jest.Mock).mockReturnValue(false); + + await main(); + expect(mkdir).toHaveBeenCalledWith("foo"); +}); diff --git a/test/main/json.spec.ts b/test/main/json.spec.ts index ddacd6fe..e2697e73 100644 --- a/test/main/json.spec.ts +++ b/test/main/json.spec.ts @@ -18,13 +18,13 @@ interface FooV2 { } // eslint-disable-next-line @typescript-eslint/ban-ts-comment -//@ts-ignore foo! +//@ts-ignore ts-jest doesn't like running in strict mode for some reason? const fooV1: z.Schema = z.object({ version: z.literal(1), foo: z.string(), }); // eslint-disable-next-line @typescript-eslint/ban-ts-comment -//@ts-ignore foo! +//@ts-ignore ts-jest doesn't like running in strict mode for some reason? const fooV2: z.Schema = z.preprocess( (obj) => { const foo = obj as FooV1 | FooV2; diff --git a/test/main/shortcuts.spec.ts b/test/main/shortcuts.spec.ts index 3e18f83d..d36b9954 100644 --- a/test/main/shortcuts.spec.ts +++ b/test/main/shortcuts.spec.ts @@ -1,121 +1,121 @@ -// import { createConfig } from "../__factories__/config"; -// import { createIpcMainTS } from "../__factories__/ipc"; -// import { createJsonFile } from "../__factories__/jsonFile"; -// import { shortcutIpcs } from "../../src/main/shortcuts"; -// import { loadJsonFile } from "../../src/main/json"; -// import { Section } from "../../src/shared/ui/app"; -// import { parseKeyCodes } from "../../src/shared/io/keyCode"; +import { createConfig } from "../__factories__/config"; +import { createIpcMainTS } from "../__factories__/ipc"; +import { createJsonFile } from "../__factories__/jsonFile"; +import { shortcutIpcs } from "../../src/main/shortcuts"; +import { loadJsonFile } from "../../src/main/json"; +import { Section } from "../../src/shared/ui/app"; +import { parseKeyCodes } from "../../src/shared/io/keyCode"; -// jest.mock("fs"); -// jest.mock("fs/promises"); -// jest.mock("../../src/main/json"); +jest.mock("fs"); +jest.mock("fs/promises"); +jest.mock("../../src/main/json"); -// test("shortcutIpcs init", async () => { -// const ipc = createIpcMainTS(); -// const config = createJsonFile(createConfig()); +test("shortcutIpcs init", async () => { + const ipc = createIpcMainTS(); + const config = createJsonFile(createConfig()); -// (loadJsonFile as jest.Mock).mockResolvedValueOnce({ -// content: { -// shortcuts: [ -// // Disable existing shortcut -// { -// name: "sidebar.focus", -// disabled: true, -// }, -// // Change keys existing shortcut -// { -// name: "app.toggleSidebar", -// keys: "control+alt+s", -// }, -// // Change when existing shortcut -// { -// name: "app.quit", -// when: Section.Sidebar, -// }, -// // Change event input existing shortcut -// { -// name: "notes.create", -// eventInput: "foo", -// }, -// // Change repeat existing shortcut -// { -// name: "app.reload", -// repeat: true, -// }, -// // Add a new shortcut -// { -// name: "openDataDirectory2", -// event: "app.openDataDirectory", -// keys: "control+o+d+f", -// when: Section.Editor, -// repeat: false, -// }, -// ], -// }, -// update: jest.fn(), -// }); + (loadJsonFile as jest.Mock).mockResolvedValueOnce({ + content: { + shortcuts: [ + // Disable existing shortcut + { + name: "sidebar.focus", + disabled: true, + }, + // Change keys existing shortcut + { + name: "app.toggleSidebar", + keys: "control+alt+s", + }, + // Change when existing shortcut + { + name: "app.quit", + when: Section.Sidebar, + }, + // Change event input existing shortcut + { + name: "notes.create", + eventInput: "foo", + }, + // Change repeat existing shortcut + { + name: "app.reload", + repeat: true, + }, + // Add a new shortcut + { + name: "openDataDirectory2", + event: "app.openDataDirectory", + keys: "control+o+d+f", + when: Section.Editor, + repeat: false, + }, + ], + }, + update: jest.fn(), + }); -// shortcutIpcs(ipc, config); -// await ipc.trigger("init"); + shortcutIpcs(ipc, config); + await ipc.trigger("init"); -// const shortcuts = await ipc.invoke("shortcuts.getAll"); + const shortcuts = await ipc.invoke("shortcuts.getAll"); -// const sidebarFocus = shortcuts.find((s) => s.name === "sidebar.focus"); -// expect(sidebarFocus).toEqual( -// expect.objectContaining({ -// disabled: true, -// event: "focus.push", -// name: "sidebar.focus", -// }) -// ); + const sidebarFocus = shortcuts.find((s) => s.name === "sidebar.focus"); + expect(sidebarFocus).toEqual( + expect.objectContaining({ + disabled: true, + event: "focus.push", + name: "sidebar.focus", + }) + ); -// const appToggleSidebar = shortcuts.find( -// (s) => s.name === "app.toggleSidebar" -// ); -// expect(appToggleSidebar).toEqual( -// expect.objectContaining({ -// name: "app.toggleSidebar", -// event: "app.toggleSidebar", -// keys: parseKeyCodes("control+alt+s"), -// }) -// ); + const appToggleSidebar = shortcuts.find( + (s) => s.name === "app.toggleSidebar" + ); + expect(appToggleSidebar).toEqual( + expect.objectContaining({ + name: "app.toggleSidebar", + event: "app.toggleSidebar", + keys: parseKeyCodes("control+alt+s"), + }) + ); -// const appQuit = shortcuts.find((s) => s.name === "app.quit"); -// expect(appQuit).toEqual( -// expect.objectContaining({ -// name: "app.quit", -// when: Section.Sidebar, -// event: "app.quit", -// }) -// ); + const appQuit = shortcuts.find((s) => s.name === "app.quit"); + expect(appQuit).toEqual( + expect.objectContaining({ + name: "app.quit", + when: Section.Sidebar, + event: "app.quit", + }) + ); -// const createNote = shortcuts.find((s) => s.name === "notes.create"); -// expect(createNote).toEqual( -// expect.objectContaining({ -// name: "notes.create", -// event: "sidebar.createNote", -// eventInput: "foo", -// }) -// ); + const createNote = shortcuts.find((s) => s.name === "notes.create"); + expect(createNote).toEqual( + expect.objectContaining({ + name: "notes.create", + event: "sidebar.createNote", + eventInput: "foo", + }) + ); -// const appReload = shortcuts.find((s) => s.name === "app.reload"); -// expect(appReload).toEqual( -// expect.objectContaining({ -// name: "app.reload", -// event: "app.reload", -// repeat: true, -// }) -// ); + const appReload = shortcuts.find((s) => s.name === "app.reload"); + expect(appReload).toEqual( + expect.objectContaining({ + name: "app.reload", + event: "app.reload", + repeat: true, + }) + ); -// const openDataDirectory = shortcuts.find( -// (s) => s.name === "openDataDirectory2" -// ); -// expect(openDataDirectory).toEqual( -// expect.objectContaining({ -// name: "openDataDirectory2", -// event: "app.openDataDirectory", -// when: Section.Editor, -// repeat: false, -// }) -// ); -// }); + const openDataDirectory = shortcuts.find( + (s) => s.name === "openDataDirectory2" + ); + expect(openDataDirectory).toEqual( + expect.objectContaining({ + name: "openDataDirectory2", + event: "app.openDataDirectory", + when: Section.Editor, + repeat: false, + }) + ); +}); From e7e7d8f1ce61e2cf556f1e5e8d12790e51cf0134 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sat, 24 Sep 2022 21:50:00 -0400 Subject: [PATCH 3/9] Add test to check config migrations run correctly --- src/main/json.ts | 75 +++++++++++--------- src/main/schemas/config/2_addLogDirectory.ts | 7 +- src/main/shortcuts.ts | 2 +- test/main/schema/config.spec.ts | 19 +++++ 4 files changed, 66 insertions(+), 37 deletions(-) create mode 100644 test/main/schema/config.spec.ts diff --git a/src/main/json.ts b/src/main/json.ts index 600d6e44..3d7c0612 100644 --- a/src/main/json.ts +++ b/src/main/json.ts @@ -15,17 +15,6 @@ export async function loadJsonFile( schemas: Record, defaultContent: Content ): Promise> { - const schemaArray = Object.entries(schemas) - .map<[number, ZodSchema]>(([version, schema]) => [ - Number.parseInt(version, 10), - schema, - ]) - .sort(([a], [b]) => (a > b ? 1 : -1)); - - if (schemaArray.length === 0) { - throw new Error(`Expected at least 1 schema in order to validate content.`); - } - let originalContent: Content | undefined; if (fs.existsSync(filePath)) { const raw = await fsp.readFile(filePath, { encoding: "utf-8" }); @@ -41,34 +30,17 @@ export async function loadJsonFile( originalContent = defaultContent; } - // Always include current version schema so we can validate against it. - const relevantSchemas = schemaArray.filter( - ([version]) => originalContent!.version <= version + const { content, latestSchema, wasUpdated } = await runSchemas( + schemas, + originalContent ); - if (relevantSchemas.length === 0) { - throw new Error( - `File ${filePath} has no schemas to run. Loaded content version was: ${originalContent.version}` - ); - } - - let content = cloneDeep(originalContent); - for (const [, schema] of relevantSchemas) { - content = await schema.parseAsync(content); - } // Save changes to file if any were made while migrating to latest. - if (content.version !== originalContent.version) { + if (wasUpdated) { const jsonContent = JSON.stringify(content); await fsp.writeFile(filePath, jsonContent, { encoding: "utf-8" }); } - const [latestVersion, latestSchema] = last(relevantSchemas)!; - if (defaultContent.version !== latestVersion) { - throw new Error( - `Default content doesn't match latest version ${latestVersion}. Did you mean to update the default?` - ); - } - const update = async (partial: DeepPartial) => { const updated = deepUpdate(content, partial); @@ -86,3 +58,42 @@ export async function loadJsonFile( }; return fileHandler; } + +export async function runSchemas( + schemas: Record, + content: Content +): Promise<{ content: Content; latestSchema: ZodSchema; wasUpdated: boolean }> { + const schemaArray = Object.entries(schemas) + .map<[number, ZodSchema]>(([version, schema]) => [ + Number.parseInt(version, 10), + schema, + ]) + .sort(([a], [b]) => (a > b ? 1 : -1)); + + if (schemaArray.length === 0) { + throw new Error(`Expected at least 1 schema in order to validate content.`); + } + + // Always include current version schema so we can validate against it. + const relevantSchemas = schemaArray.filter( + ([version]) => content!.version <= version + ); + const [, latestSchema] = last(relevantSchemas)!; + + if (relevantSchemas.length === 0) { + throw new Error( + `No schema to run. Loaded content version was: ${content.version} but last schema had version: ${latestSchema}` + ); + } + + let validatedContent = cloneDeep(content); + for (const [, schema] of relevantSchemas) { + validatedContent = await schema.parseAsync(content); + } + + return { + content: validatedContent, + latestSchema, + wasUpdated: content.version !== validatedContent.version, + }; +} diff --git a/src/main/schemas/config/2_addLogDirectory.ts b/src/main/schemas/config/2_addLogDirectory.ts index 6d6057f7..7bf917aa 100644 --- a/src/main/schemas/config/2_addLogDirectory.ts +++ b/src/main/schemas/config/2_addLogDirectory.ts @@ -1,6 +1,5 @@ import { app } from "electron"; import { z } from "zod"; -import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "../.."; import { ConfigV1 } from "./1_initialDefinition"; export interface ConfigV2 { @@ -11,7 +10,7 @@ export interface ConfigV2 { logDirectory?: string; } -export const configSchemaV2 = z.preprocess( +export const configSchemaV2: z.Schema = z.preprocess( (obj) => { const config = obj as ConfigV1 | ConfigV2; @@ -27,8 +26,8 @@ export const configSchemaV2 = z.preprocess( }, z.object({ version: z.literal(2), - windowHeight: z.number().min(1).default(DEFAULT_WINDOW_HEIGHT), - windowWidth: z.number().min(1).default(DEFAULT_WINDOW_WIDTH), + windowHeight: z.number().min(1), + windowWidth: z.number().min(1), dataDirectory: z.string().optional(), logDirectory: z.string(), }) diff --git a/src/main/shortcuts.ts b/src/main/shortcuts.ts index 3260673c..1bf08d70 100644 --- a/src/main/shortcuts.ts +++ b/src/main/shortcuts.ts @@ -36,7 +36,7 @@ export interface ShortcutOverride { } export function shortcutIpcs(ipc: IpcMainTS, config: JsonFile): void { - const shortcuts: Shortcut[] = DEFAULT_SHORTCUTS; + const shortcuts = [...DEFAULT_SHORTCUTS]; ipc.on("init", async () => { const shortcutFile = await loadJsonFile( diff --git a/test/main/schema/config.spec.ts b/test/main/schema/config.spec.ts new file mode 100644 index 00000000..3c10001a --- /dev/null +++ b/test/main/schema/config.spec.ts @@ -0,0 +1,19 @@ +import { ConfigV1 } from "../../../src/main/schemas/config/1_initialDefinition"; +import { runSchemas } from "../../../src/main/json"; +import { CONFIG_SCHEMAS } from "../../../src/main/schemas/config"; +import { app } from "electron"; + +test("CONFIG_SCHEMAS migrate from 1 to latest", async () => { + (app.getPath as jest.Mock).mockReturnValue("foo"); + + const start: ConfigV1 = { + version: 1, + windowHeight: 200, + windowWidth: 400, + dataDirectory: "foo", + }; + + // TODO: Add better check than just if an error is thrown. + const res = await runSchemas(CONFIG_SCHEMAS, start); + expect(res.wasUpdated).toBe(true); +}); From 0d3c78c9970043dbdfb508fc0a45907b3ae6ad58 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sat, 24 Sep 2022 22:52:35 -0400 Subject: [PATCH 4/9] Pretty format json files --- src/main/json.ts | 23 ++++++++++++++++++----- test/main/json.spec.ts | 16 +++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/main/json.ts b/src/main/json.ts index 3d7c0612..3bac946c 100644 --- a/src/main/json.ts +++ b/src/main/json.ts @@ -13,7 +13,8 @@ export interface JsonFile { export async function loadJsonFile( filePath: string, schemas: Record, - defaultContent: Content + defaultContent: Content, + opts: { prettyPrint?: boolean } = { prettyPrint: true } ): Promise> { let originalContent: Content | undefined; if (fs.existsSync(filePath)) { @@ -37,7 +38,13 @@ export async function loadJsonFile( // Save changes to file if any were made while migrating to latest. if (wasUpdated) { - const jsonContent = JSON.stringify(content); + let jsonContent; + if (opts.prettyPrint) { + jsonContent = JSON.stringify(content, null, 2); + } else { + jsonContent = JSON.stringify(content); + } + await fsp.writeFile(filePath, jsonContent, { encoding: "utf-8" }); } @@ -47,7 +54,13 @@ export async function loadJsonFile( // Validate against latest schema when saving to ensure we have valid content. const validated = await latestSchema.parseAsync(updated); - const jsonString = JSON.stringify(updated); + let jsonString; + if (opts.prettyPrint) { + jsonString = JSON.stringify(updated, null, 2); + } else { + jsonString = JSON.stringify(updated); + } + fileHandler.content = validated; await fsp.writeFile(filePath, jsonString, { encoding: "utf-8" }); }; @@ -82,7 +95,7 @@ export async function runSchemas( if (relevantSchemas.length === 0) { throw new Error( - `No schema to run. Loaded content version was: ${content.version} but last schema had version: ${latestSchema}` + `No schema(s) to run. Loaded content version was: ${content.version} but last schema had version: ${latestSchema}` ); } @@ -94,6 +107,6 @@ export async function runSchemas( return { content: validatedContent, latestSchema, - wasUpdated: content.version !== validatedContent.version, + wasUpdated: validatedContent.version > content.version, }; } diff --git a/test/main/json.spec.ts b/test/main/json.spec.ts index e2697e73..5cec1746 100644 --- a/test/main/json.spec.ts +++ b/test/main/json.spec.ts @@ -47,7 +47,9 @@ const fooV2: z.Schema = z.preprocess( test("loadJsonFile throws if no migrations passed", async () => { await expect(async () => { - await loadJsonFile("fake-file-path.json", {}, null!); + await loadJsonFile("fake-file-path.json", {}, null!, { + prettyPrint: false, + }); }).rejects.toThrow(/Expected at least 1 schema/); }); @@ -64,7 +66,8 @@ test("loadJsonFile loads default content if no file found", async () => { version: 2, foo: "cat", bar: 42, - } + }, + { prettyPrint: false } ); expect(content).toEqual({ version: 2, @@ -104,7 +107,8 @@ test("loadJsonFile loads content and validates it", async () => { version: 2, foo: "cat", bar: 42, - } + }, + { prettyPrint: false } ); expect(content).toEqual({ @@ -147,7 +151,8 @@ test("loadJsonFile update validates content before saving to file.", async () => version: 2, foo: "cat", bar: 42, - } + }, + { prettyPrint: false } ); expect(fsp.writeFile).not.toBeCalled(); @@ -199,7 +204,8 @@ test("loadJsonFile migrates when loading older content", async () => { version: 2, foo: "cat", bar: 42, - } + }, + { prettyPrint: false } ); expect(fileHandler.content).toEqual({ From 8f53b0078b0418454cc252c0c7750dd3f5ee8036 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sun, 25 Sep 2022 12:52:38 -0400 Subject: [PATCH 5/9] Change main structure for easier testing of config changes --- src/main/app.ts | 6 +- src/main/config.ts | 64 ++++++++++++++++- src/main/index.ts | 64 +++-------------- src/main/log.ts | 72 +++++++++++++------ src/main/notes.ts | 6 +- src/main/shortcuts.ts | 6 +- .../components/SidebarNewNoteButton.tsx | 4 +- src/renderer/logger.ts | 9 ++- test/main/{index.spec.ts => config.spec.ts} | 15 ++-- 9 files changed, 157 insertions(+), 89 deletions(-) rename test/main/{index.spec.ts => config.spec.ts} (74%) diff --git a/src/main/app.ts b/src/main/app.ts index 2b421b12..50e8ddb8 100644 --- a/src/main/app.ts +++ b/src/main/app.ts @@ -19,7 +19,11 @@ import { APP_STATE_SCHEMAS } from "./schemas/appState"; export const APP_STATE_PATH = "ui.json"; -export function appIpcs(ipc: IpcMainTS, config: JsonFile): void { +export function appIpcs( + ipc: IpcMainTS, + config: JsonFile, + log: Logger +): void { let appStateFile: JsonFile; ipc.on("init", async () => { diff --git a/src/main/config.ts b/src/main/config.ts index 122ceada..81fd9b90 100644 --- a/src/main/config.ts +++ b/src/main/config.ts @@ -1,9 +1,25 @@ -import { BrowserWindow, dialog, shell } from "electron"; +import { app, BrowserWindow, dialog, shell } from "electron"; import { IpcMainTS } from "../shared/ipc"; import { Config } from "../shared/domain/config"; -import { JsonFile } from "./json"; +import { JsonFile, loadJsonFile } from "./json"; +import { Logger } from "../shared/logger"; +import { CONFIG_SCHEMAS } from "./schemas/config"; +import { isDevelopment, isTest } from "../shared/env"; +import * as path from "path"; +import * as fs from "fs"; +import * as fsp from "fs/promises"; -export function configIpcs(ipc: IpcMainTS, config: JsonFile): void { +export const CONFIG_FILE = "config.json"; +export const DEFAULT_DEV_DATA_DIRECTORY = "data"; +export const DEFAULT_DEV_LOG_DIRECTORY = "logs"; +export const DEFAULT_WINDOW_HEIGHT = 600; +export const DEFAULT_WINDOW_WIDTH = 800; + +export function configIpcs( + ipc: IpcMainTS, + config: JsonFile, + log: Logger +): void { ipc.handle( "config.hasDataDirectory", async () => config.content.dataDirectory != null @@ -34,3 +50,45 @@ export function configIpcs(ipc: IpcMainTS, config: JsonFile): void { focusedWindow.reload(); }); } + +export async function getConfig(): Promise> { + const configFile = await loadJsonFile( + getConfigPath(), + CONFIG_SCHEMAS, + { + version: 2, + windowHeight: DEFAULT_WINDOW_HEIGHT, + windowWidth: DEFAULT_WINDOW_WIDTH, + logDirectory: app.getPath("logs"), + } + ); + + // Override directories when running in development. + if (isDevelopment()) { + await configFile.update({ + dataDirectory: DEFAULT_DEV_DATA_DIRECTORY, + logDirectory: DEFAULT_DEV_LOG_DIRECTORY, + }); + } + + // Always check if we need to recreate the data directory on start. It may have + // been deleted to clear out notes. + if ( + configFile.content.dataDirectory != null && + !fs.existsSync(configFile.content.dataDirectory) + ) { + await fsp.mkdir(configFile.content.dataDirectory); + } + /* */ + return configFile; +} + +export function getConfigPath(): string { + if (isDevelopment()) { + return path.join(process.cwd(), CONFIG_FILE); + } else if (isTest()) { + return ""; + } else { + return path.join(app.getPath("userData"), CONFIG_FILE); + } +} diff --git a/src/main/index.ts b/src/main/index.ts index 5aa91348..30ce8ad3 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2,19 +2,14 @@ import { app, BrowserWindow, ipcMain, Menu, session } from "electron"; import { getProcessType, isDevelopment, isTest } from "../shared/env"; import { IpcMainTS } from "../shared/ipc"; import { appIpcs } from "./app"; -import { configIpcs } from "./config"; +import { configIpcs, getConfig } from "./config"; import { noteIpcs } from "./notes"; import { openInBrowser } from "./utils"; -import * as path from "path"; -import * as fs from "fs"; -import * as fsp from "fs/promises"; -import { loadJsonFile } from "./json"; -import { CONFIG_SCHEMAS } from "./schemas/config"; -import { Config } from "../shared/domain/config"; + import { shortcutIpcs } from "./shortcuts"; -import { logIpcs } from "./log"; +import { getLogger, logIpcs } from "./log"; -if (getProcessType() !== "main") { +if (!isTest() && getProcessType() !== "main") { throw Error( "ipcMain is null. Did you accidentally import main.ts in the renderer thread?" ); @@ -26,45 +21,18 @@ if (getProcessType() !== "main") { declare const MAIN_WINDOW_WEBPACK_ENTRY: string; declare const MAIN_WINDOW_PRELOAD_WEBPACK_ENTRY: string; -export const CONFIG_FILE = "config.json"; -export const DEFAULT_DEV_DATA_DIRECTORY = "data"; -export const DEFAULT_WINDOW_HEIGHT = 600; -export const DEFAULT_WINDOW_WIDTH = 800; - let mainWindow: BrowserWindow; export async function main(): Promise { - const configFile = await loadJsonFile( - getConfigPath(), - CONFIG_SCHEMAS, - { - version: 2, - windowHeight: DEFAULT_WINDOW_HEIGHT, - windowWidth: DEFAULT_WINDOW_WIDTH, - logDirectory: app.getPath("logs"), - } - ); - - if (isDevelopment() && configFile.content.dataDirectory == null) { - await configFile.update({ dataDirectory: DEFAULT_DEV_DATA_DIRECTORY }); - } - - // Always check if we need to recreate the data directory on start. It may have - // been deleted to clear out notes. - if ( - configFile.content.dataDirectory != null && - !fs.existsSync(configFile.content.dataDirectory) - ) { - await fsp.mkdir(configFile.content.dataDirectory); - } + const configFile = await getConfig(); + const log = await getLogger(configFile); const typeSafeIpc = ipcMain as IpcMainTS; - - configIpcs(typeSafeIpc, configFile); - logIpcs(typeSafeIpc, configFile); - appIpcs(typeSafeIpc, configFile); - shortcutIpcs(typeSafeIpc, configFile); - noteIpcs(typeSafeIpc, configFile); + logIpcs(typeSafeIpc, configFile, log); + configIpcs(typeSafeIpc, configFile, log); + appIpcs(typeSafeIpc, configFile, log); + shortcutIpcs(typeSafeIpc, configFile, log); + noteIpcs(typeSafeIpc, configFile, log); // Handle creating/removing shortcuts on Windows when installing/uninstalling. if (require("electron-squirrel-startup")) { @@ -173,13 +141,3 @@ export async function initPlugins(typeSafeIpc: IpcMainTS): Promise { const initListeners = typeSafeIpc.listeners("init"); return await Promise.all(initListeners.map((l) => l())); } - -export function getConfigPath(): string { - if (isDevelopment()) { - return path.join(process.cwd(), CONFIG_FILE); - } else if (isTest()) { - return ""; - } else { - return path.join(app.getPath("userData"), CONFIG_FILE); - } -} diff --git a/src/main/log.ts b/src/main/log.ts index 88775e2f..4cfefe80 100644 --- a/src/main/log.ts +++ b/src/main/log.ts @@ -1,40 +1,68 @@ +/* eslint-disable no-console */ import { format } from "date-fns"; import { Config } from "../shared/domain/config"; import { IpcMainTS } from "../shared/ipc"; +import { Logger } from "../shared/logger"; import { JsonFile } from "./json"; +import * as fsp from "fs/promises"; import * as fs from "fs"; // TODO: Add logging to file // Have it manage files and delete anything older than 2 weeks -export function logIpcs(ipc: IpcMainTS, config: JsonFile): void { - // let fileStream; +export function logIpcs( + ipc: IpcMainTS, + configFile: JsonFile, + log: Logger +): void { + ipc.handle("log.info", async (_, message) => + log.info(`[RENDERER] ${message}`) + ); - ipc.on("init", async () => { - // fileStream = fs.createWriteStream(); - }); + ipc.handle("log.debug", async (_, message) => + log.debug(`[RENDERER] ${message}`) + ); - ipc.handle("log.info", async (_, message) => { - const fullMessage = `${getTimeStamp()} [RENDERER] (info): ${message}`; - console.log(fullMessage); - }); + ipc.handle("log.warn", async (_, message) => + log.warn(`[RENDERER] ${message}`) + ); - ipc.handle("log.debug", async (_, message) => { - const fullMessage = `${getTimeStamp()} [RENDERER] (debug): ${message}`; - console.log(fullMessage); - }); + ipc.handle("log.error", async (_, message) => + log.error(`[RENDERER] ${message}`) + ); +} + +export async function getLogger(config: JsonFile): Promise { + const { logDirectory } = config.content; + if (logDirectory != null && !fs.existsSync(logDirectory)) { + await fsp.mkdir(logDirectory); + } + + + throw new Error("STOP"); - ipc.handle("log.warn", async (_, message) => { - const fullMessage = `${getTimeStamp()} [RENDERER] (warn): ${message}`; - console.warn(fullMessage); - }); + const log: Logger = { + async debug(message: string): Promise { + const fullMessage = `${getTimeStamp()} (debug): ${message}`; + console.log(fullMessage); + }, + async info(message: string): Promise { + const fullMessage = `${getTimeStamp()} (info): ${message}`; + console.log(fullMessage); + }, + async warn(message: string): Promise { + const fullMessage = `${getTimeStamp()} (warn): ${message}`; + console.warn(fullMessage); + }, + async error(message: string): Promise { + const fullMessage = `${getTimeStamp()} (ERROR): ${message}`; + console.error(fullMessage); + }, + }; - ipc.handle("log.error", async (_, message) => { - const fullMessage = `${getTimeStamp()} [RENDERER] (ERROR): ${message}`; - console.error(fullMessage); - }); + // let fileStream = fs.createWriteStream(); } -export function getTimeStamp() { +export function getTimeStamp(): string { return format(new Date(), "L/d H:mm"); } diff --git a/src/main/notes.ts b/src/main/notes.ts index 096107fd..691e6e54 100644 --- a/src/main/notes.ts +++ b/src/main/notes.ts @@ -19,7 +19,11 @@ export const NOTES_DIRECTORY = "notes"; export const METADATA_FILE_NAME = "metadata.json"; export const MARKDOWN_FILE_NAME = "index.md"; -export function noteIpcs(ipc: IpcMainTS, config: JsonFile): void { +export function noteIpcs( + ipc: IpcMainTS, + config: JsonFile, + log: Logger +): void { let initialized = false; let notes: Note[] = []; const dataDirectory = config.content.dataDirectory!; diff --git a/src/main/shortcuts.ts b/src/main/shortcuts.ts index 1bf08d70..cc6ed6a9 100644 --- a/src/main/shortcuts.ts +++ b/src/main/shortcuts.ts @@ -35,7 +35,11 @@ export interface ShortcutOverride { disabled?: boolean; } -export function shortcutIpcs(ipc: IpcMainTS, config: JsonFile): void { +export function shortcutIpcs( + ipc: IpcMainTS, + config: JsonFile, + log: Logger +): void { const shortcuts = [...DEFAULT_SHORTCUTS]; ipc.on("init", async () => { diff --git a/src/renderer/components/SidebarNewNoteButton.tsx b/src/renderer/components/SidebarNewNoteButton.tsx index 545393b6..b4b6b876 100644 --- a/src/renderer/components/SidebarNewNoteButton.tsx +++ b/src/renderer/components/SidebarNewNoteButton.tsx @@ -4,7 +4,7 @@ import React from "react"; import { PropsWithChildren } from "react"; import styled from "styled-components"; import { pb2 } from "../css"; -import { logger } from "../logger"; +import { log } from "../logger"; import { Store } from "../store"; import { Icon } from "./shared/Icon"; @@ -16,7 +16,7 @@ export function SidebarNewNoteButton( props: PropsWithChildren ): JSX.Element { const onClick = (ev: React.MouseEvent) => { - logger.info("HI"); + log.info("HI"); // Stop prop otherwise we'll mess up switching focus ev.stopPropagation(); props.store.dispatch("sidebar.createNote", null); diff --git a/src/renderer/logger.ts b/src/renderer/logger.ts index b2c634ee..b017db3a 100644 --- a/src/renderer/logger.ts +++ b/src/renderer/logger.ts @@ -1,6 +1,13 @@ +import { getProcessType, isTest } from "../shared/env"; import { Logger } from "../shared/logger"; -export const logger: Logger = { +if (!isTest() && getProcessType() !== "renderer") { + throw Error( + "window.ipc is null. Did you accidentally import logger.ts in the main thread?" + ); +} + +export const log: Logger = { info: async (message) => { console.log(message); window.ipc("log.info", message); diff --git a/test/main/index.spec.ts b/test/main/config.spec.ts similarity index 74% rename from test/main/index.spec.ts rename to test/main/config.spec.ts index 3a8cd11b..5605bf5f 100644 --- a/test/main/index.spec.ts +++ b/test/main/config.spec.ts @@ -1,4 +1,8 @@ -import { DEFAULT_DEV_DATA_DIRECTORY, main } from "../../src/main/index"; +import { + DEFAULT_DEV_DATA_DIRECTORY, + DEFAULT_DEV_LOG_DIRECTORY, + getConfig, +} from "../../src/main/config"; import fsp from "fs/promises"; import fs from "fs"; import * as env from "../../src/shared/env"; @@ -8,7 +12,7 @@ jest.mock("fs/promises"); jest.mock("fs"); jest.mock("../../src/main/json"); -test("main defaults data directory in development", async () => { +test("getConfig defaults data / log directory in development", async () => { const update = jest.fn(); (loadJsonFile as jest.Mock).mockResolvedValueOnce({ @@ -24,13 +28,14 @@ test("main defaults data directory in development", async () => { // to function normal. jest.spyOn(env, "isDevelopment").mockReturnValue(true); - await main(); + await getConfig(); expect(update).toHaveBeenCalledWith({ dataDirectory: DEFAULT_DEV_DATA_DIRECTORY, + logDirectory: DEFAULT_DEV_LOG_DIRECTORY, }); }); -test("main creates data directory if directory is missing.", async () => { +test("getConfig creates data directory if directory is missing.", async () => { (loadJsonFile as jest.Mock).mockResolvedValueOnce({ content: { dataDirectory: "foo", @@ -44,6 +49,6 @@ test("main creates data directory if directory is missing.", async () => { (fsp.mkdir as jest.Mock).mockImplementation(mkdir); (fs.existsSync as jest.Mock).mockReturnValue(false); - await main(); + await getConfig(); expect(mkdir).toHaveBeenCalledWith("foo"); }); From e5a9197484e03c4f773592a216625ff53e281659 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sun, 25 Sep 2022 15:00:34 -0400 Subject: [PATCH 6/9] Log to file and delete old logs --- .gitignore | 5 +- package-lock.json | 10 +- package.json | 1 + src/main/app.ts | 1 + src/main/index.ts | 2 +- src/main/log.ts | 63 ++++++++-- src/main/notes.ts | 1 + .../schemas/config/1_initialDefinition.ts | 2 +- src/main/shortcuts.ts | 1 + src/renderer/App.tsx | 1 + src/renderer/menus/contextMenu.ts | 2 +- src/shared/utils.ts | 4 + test/main/log.spec.ts | 108 ++++++++++++++++++ test/setup.ts | 2 +- 14 files changed, 181 insertions(+), 22 deletions(-) create mode 100644 test/main/log.spec.ts diff --git a/.gitignore b/.gitignore index fcab9fe1..3202e9ea 100644 --- a/.gitignore +++ b/.gitignore @@ -30,4 +30,7 @@ pnpm-debug.log* # Default dev data dir /data -config.json \ No newline at end of file +config.json + +# Default dev log dir +/logs \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 16cd675b..944ab912 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3118,7 +3118,6 @@ "version": "4.3.0", "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz", "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==", - "dev": true, "requires": { "color-convert": "^2.0.1" } @@ -3826,7 +3825,6 @@ "version": "4.1.2", "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.2.tgz", "integrity": "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==", - "dev": true, "requires": { "ansi-styles": "^4.1.0", "supports-color": "^7.1.0" @@ -3994,7 +3992,6 @@ "version": "2.0.1", "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-2.0.1.tgz", "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==", - "dev": true, "requires": { "color-name": "~1.1.4" } @@ -4002,8 +3999,7 @@ "color-name": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/color-name/-/color-name-1.1.4.tgz", - "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==", - "dev": true + "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==" }, "colorette": { "version": "2.0.16", @@ -7106,8 +7102,7 @@ "has-flag": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", - "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", - "dev": true + "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==" }, "has-symbols": { "version": "1.0.2", @@ -12220,7 +12215,6 @@ "version": "7.2.0", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", - "dev": true, "requires": { "has-flag": "^4.0.0" } diff --git a/package.json b/package.json index 95088ca5..69f036d4 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "webpack-dev-server": "^4.7.4" }, "dependencies": { + "chalk": "^4.1.2", "dart-sass": "^1.25.0", "date-fns": "^2.28.0", "electron-squirrel-startup": "^1.0.0", diff --git a/src/main/app.ts b/src/main/app.ts index 50e8ddb8..7d7db1ca 100644 --- a/src/main/app.ts +++ b/src/main/app.ts @@ -16,6 +16,7 @@ import p from "path"; import { MissingDataDirectoryError } from "../shared/errors"; import { NoteSort } from "../shared/domain/note"; import { APP_STATE_SCHEMAS } from "./schemas/appState"; +import { Logger } from "../shared/logger"; export const APP_STATE_PATH = "ui.json"; diff --git a/src/main/index.ts b/src/main/index.ts index 30ce8ad3..545c2137 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -25,7 +25,7 @@ let mainWindow: BrowserWindow; export async function main(): Promise { const configFile = await getConfig(); - const log = await getLogger(configFile); + const log = await getLogger(configFile, console); const typeSafeIpc = ipcMain as IpcMainTS; logIpcs(typeSafeIpc, configFile, log); diff --git a/src/main/log.ts b/src/main/log.ts index 4cfefe80..79ea4b07 100644 --- a/src/main/log.ts +++ b/src/main/log.ts @@ -1,11 +1,21 @@ /* eslint-disable no-console */ -import { format } from "date-fns"; +import { + differenceInCalendarDays, + format, + formatISO, + parseISO, +} from "date-fns"; import { Config } from "../shared/domain/config"; import { IpcMainTS } from "../shared/ipc"; import { Logger } from "../shared/logger"; import { JsonFile } from "./json"; import * as fsp from "fs/promises"; import * as fs from "fs"; +import * as p from "path"; +import { ISO_8601_REGEX } from "../shared/utils"; +import chalk from "chalk"; + +const DELETE_LOGS_OLDER_THAN_DAYS = 14; // TODO: Add logging to file // Have it manage files and delete anything older than 2 weeks @@ -32,37 +42,72 @@ export function logIpcs( ); } -export async function getLogger(config: JsonFile): Promise { +export async function getLogger( + config: JsonFile, + c: Console +): Promise { const { logDirectory } = config.content; if (logDirectory != null && !fs.existsSync(logDirectory)) { await fsp.mkdir(logDirectory); } - - throw new Error("STOP"); + // Delete any logs older than 2 weeks + const entries = await fsp.readdir(logDirectory, { withFileTypes: true }); + for (const entry of entries) { + if (p.extname(entry.name) !== "log" && !ISO_8601_REGEX.test(entry.name)) { + continue; + } + + const filePath = p.join(logDirectory, entry.name); + const fileStats = await fsp.stat(filePath); + + if ( + differenceInCalendarDays(new Date(), fileStats.birthtime) > + DELETE_LOGS_OLDER_THAN_DAYS + ) { + await fsp.unlink(filePath); + } + } + + const currLogFile = getLogFileName(new Date()); + const fileStream = fs.createWriteStream(p.join(logDirectory, currLogFile), { + flags: "a", + }); const log: Logger = { async debug(message: string): Promise { const fullMessage = `${getTimeStamp()} (debug): ${message}`; - console.log(fullMessage); + c.log(chalk.cyan(fullMessage)); + + await fileStream.write(fullMessage + "\n"); }, async info(message: string): Promise { const fullMessage = `${getTimeStamp()} (info): ${message}`; - console.log(fullMessage); + c.log(fullMessage); + + await fileStream.write(fullMessage + "\n"); }, async warn(message: string): Promise { const fullMessage = `${getTimeStamp()} (warn): ${message}`; - console.warn(fullMessage); + c.warn(chalk.yellow(fullMessage)); + + await fileStream.write(fullMessage + "\n"); }, async error(message: string): Promise { const fullMessage = `${getTimeStamp()} (ERROR): ${message}`; - console.error(fullMessage); + c.error(chalk.red(fullMessage)); + + await fileStream.write(fullMessage); }, }; - // let fileStream = fs.createWriteStream(); + return log; } export function getTimeStamp(): string { return format(new Date(), "L/d H:mm"); } + +export function getLogFileName(date: Date): string { + return `${formatISO(date)}.log`; +} diff --git a/src/main/notes.ts b/src/main/notes.ts index 691e6e54..0a0d73e7 100644 --- a/src/main/notes.ts +++ b/src/main/notes.ts @@ -14,6 +14,7 @@ import * as fs from "fs"; import * as fsp from "fs/promises"; import { JsonFile } from "./json"; import * as p from "path"; +import { Logger } from "../shared/logger"; export const NOTES_DIRECTORY = "notes"; export const METADATA_FILE_NAME = "metadata.json"; diff --git a/src/main/schemas/config/1_initialDefinition.ts b/src/main/schemas/config/1_initialDefinition.ts index c1efc748..45b8109f 100644 --- a/src/main/schemas/config/1_initialDefinition.ts +++ b/src/main/schemas/config/1_initialDefinition.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "../.."; +import { DEFAULT_WINDOW_HEIGHT, DEFAULT_WINDOW_WIDTH } from "../../config"; export interface ConfigV1 { version: number; diff --git a/src/main/shortcuts.ts b/src/main/shortcuts.ts index cc6ed6a9..098bbfbf 100644 --- a/src/main/shortcuts.ts +++ b/src/main/shortcuts.ts @@ -9,6 +9,7 @@ import { UIEventInput, UIEventType } from "../shared/ui/events"; import { JsonFile, loadJsonFile } from "./json"; import p from "path"; import { SHORTCUTS_SCHEMAS } from "./schemas/shortcuts"; +import { Logger } from "../shared/logger"; export interface Shortcuts { version: number; diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index f73ed9ab..0fbbb716 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -17,6 +17,7 @@ import { useContextMenu } from "./menus/contextMenu"; import { Editor } from "./components/Editor"; import { h100, HEADER_SIZES, mb2, w100 } from "./css"; import { AppState } from "../shared/ui/app"; +import { log } from "./logger"; const { ipc } = window; async function main() { diff --git a/src/renderer/menus/contextMenu.ts b/src/renderer/menus/contextMenu.ts index 9fb58660..312b99b6 100644 --- a/src/renderer/menus/contextMenu.ts +++ b/src/renderer/menus/contextMenu.ts @@ -19,7 +19,7 @@ import { IpcChannel } from "../../shared/ipc"; export function useContextMenu(store: Store): void { const { state } = store; - // useMemo prevents unneccessary renders + // useMemo prevents unnecessary renders const shortcutLabels = useMemo( () => getShortcutLabels(state.shortcuts), [state.shortcuts] diff --git a/src/shared/utils.ts b/src/shared/utils.ts index 39769fff..70ed61be 100644 --- a/src/shared/utils.ts +++ b/src/shared/utils.ts @@ -7,3 +7,7 @@ export function sleep(milliseconds: number): Promise { setTimeout(res, milliseconds); }); } + +// Source: https://stackoverflow.com/a/37563868 +export const ISO_8601_REGEX = + /^\d{4}(-\d\d(-\d\d(T\d\d:\d\d(:\d\d)?(\.\d+)?(([+-]\d\d:\d\d)|Z)?)?)?)?/i; diff --git a/test/main/log.spec.ts b/test/main/log.spec.ts new file mode 100644 index 00000000..e188472f --- /dev/null +++ b/test/main/log.spec.ts @@ -0,0 +1,108 @@ +import * as fs from "fs"; +import * as fsp from "fs/promises"; +import * as p from "path"; +import { subDays, subMonths, subWeeks } from "date-fns"; +import { getLogFileName, getLogger } from "../../src/main/log"; +import { createJsonFile } from "../__factories__/jsonFile"; +import { Config } from "../../src/shared/domain/config"; + +jest.mock("fs/promises"); +jest.mock("fs"); + +test("getLogger cleans up log directory", async () => { + const monthOldLog = getLogFileName(subMonths(new Date(), 1)); + const threeWeekOldLog = getLogFileName(subWeeks(new Date(), 3)); + const twoDayOldLog = getLogFileName(subDays(new Date(), 2)); + const yesterdayLog = getLogFileName(subDays(new Date(), 1)); + const randomFile = "foo.txt"; + + (fsp.readdir as jest.Mock).mockResolvedValueOnce([ + { name: monthOldLog } as fs.Dirent, + { name: threeWeekOldLog } as fs.Dirent, + { name: twoDayOldLog } as fs.Dirent, + { name: yesterdayLog } as fs.Dirent, + { name: randomFile } as fs.Dirent, + ]); + (fsp.stat as jest.Mock).mockResolvedValueOnce({ + birthtime: subMonths(new Date(), 1), + }); + (fsp.stat as jest.Mock).mockResolvedValueOnce({ + birthtime: subWeeks(new Date(), 3), + }); + (fsp.stat as jest.Mock).mockResolvedValueOnce({ + birthtime: subDays(new Date(), 2), + }); + (fsp.stat as jest.Mock).mockResolvedValueOnce({ + birthtime: subDays(new Date(), 1), + }); + + const configFile = createJsonFile({ + logDirectory: "logs", + } as Config); + await getLogger(configFile, { + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + } as unknown as Console); + + const { logDirectory } = configFile.content; + + expect(fsp.unlink).toHaveBeenNthCalledWith( + 1, + p.join(logDirectory, monthOldLog) + ); + expect(fsp.unlink).toHaveBeenNthCalledWith( + 2, + p.join(logDirectory, threeWeekOldLog) + ); + + // These files shouldn't be deleted. + expect(fsp.unlink).not.toHaveBeenCalledWith( + p.join(logDirectory, twoDayOldLog) + ); + expect(fsp.unlink).not.toHaveBeenCalledWith( + p.join(logDirectory, yesterdayLog) + ); + expect(fsp.unlink).not.toHaveBeenCalledWith(p.join(logDirectory, randomFile)); +}); + +test("getLogger logs to file", async () => { + const write = jest.fn(); + (fs.createWriteStream as jest.Mock).mockReturnValue({ + write, + }); + (fsp.readdir as jest.Mock).mockResolvedValueOnce([]); + + const configFile = createJsonFile({ + logDirectory: "logs", + } as Config); + const log = await getLogger(configFile, { + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + } as unknown as Console); + + log.info("foo"); + expect(write).toHaveBeenNthCalledWith( + 1, + expect.stringMatching(/.* \(info\): foo/) + ); + + log.debug("code reached here!"); + expect(write).toHaveBeenNthCalledWith( + 2, + expect.stringMatching(/.* \(debug\): code reached here!/) + ); + + log.warn("was missing file"); + expect(write).toHaveBeenNthCalledWith( + 3, + expect.stringMatching(/.* \(warn\): was missing file/) + ); + + log.error("something went wrong!"); + expect(write).toHaveBeenNthCalledWith( + 4, + expect.stringMatching(/.* \(ERROR\): something went wrong!/) + ); +}); diff --git a/test/setup.ts b/test/setup.ts index a4bfca2a..c3da8c1e 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -1 +1 @@ -window.ipc = jest.fn(); +(window as any).ipc = jest.fn(); From 168b2082c31312203c191453fcdaa542a41c2dc5 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sun, 25 Sep 2022 15:20:46 -0400 Subject: [PATCH 7/9] Add dev tools for opening logs faster --- src/main/app.ts | 5 +++++ src/main/index.ts | 6 ++++++ src/main/log.ts | 7 ++++--- src/renderer/App.tsx | 5 +++++ src/renderer/menus/appMenu.ts | 6 ++++++ src/renderer/store.ts | 2 ++ src/shared/ipc.ts | 2 ++ src/shared/ui/events.ts | 2 ++ 8 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/main/app.ts b/src/main/app.ts index 7d7db1ca..95474936 100644 --- a/src/main/app.ts +++ b/src/main/app.ts @@ -3,6 +3,7 @@ import { dialog, Menu, MenuItemConstructorOptions, + shell, } from "electron"; import { isRoleMenu, Menu as MenuType } from "../shared/ui/menu"; import { IpcChannel, IpcMainTS } from "../shared/ipc"; @@ -140,6 +141,10 @@ export function appIpcs( }); ipc.handle("app.openInWebBrowser", (_, url) => openInBrowser(url)); + + ipc.handle("app.openLogDirectory", async () => { + await shell.openPath(config.content.logDirectory); + }); } export function buildMenus( diff --git a/src/main/index.ts b/src/main/index.ts index 545c2137..04501885 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -122,6 +122,12 @@ export async function main(): Promise { } }); + app.on("quit", () => { + // Use console.log() over log.info to avoid appending this to the log file + // eslint-disable-next-line no-console + console.log(`Shutting down. Log saved to: ${log.filePath}`); + }); + // Ready event might fire before we finish loading our config file causing us // to miss it. // Source: https://github.com/electron/electron/issues/12557 diff --git a/src/main/log.ts b/src/main/log.ts index 79ea4b07..eaae2adf 100644 --- a/src/main/log.ts +++ b/src/main/log.ts @@ -45,7 +45,7 @@ export function logIpcs( export async function getLogger( config: JsonFile, c: Console -): Promise { +): Promise { const { logDirectory } = config.content; if (logDirectory != null && !fs.existsSync(logDirectory)) { await fsp.mkdir(logDirectory); @@ -70,7 +70,8 @@ export async function getLogger( } const currLogFile = getLogFileName(new Date()); - const fileStream = fs.createWriteStream(p.join(logDirectory, currLogFile), { + const currFilePath = p.join(logDirectory, currLogFile); + const fileStream = fs.createWriteStream(currFilePath, { flags: "a", }); @@ -101,7 +102,7 @@ export async function getLogger( }, }; - return log; + return { ...log, filePath: currFilePath }; } export function getTimeStamp(): string { diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 0fbbb716..ce53724c 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -67,6 +67,7 @@ export function App(props: AppProps): JSX.Element { store.on("app.toggleFullScreen", toggleFullScreen); store.on("app.openDataDirectory", openDataDirectory); store.on("app.selectDataDirectory", selectDataDirectory); + store.on("app.openLogDirectory", openLogs); store.on("sidebar.focusSearch", globalSearch); @@ -82,6 +83,7 @@ export function App(props: AppProps): JSX.Element { store.off("app.toggleFullScreen", toggleFullScreen); store.off("app.openDataDirectory", openDataDirectory); store.off("app.selectDataDirectory", selectDataDirectory); + store.off("app.openLogDirectory", openLogs); store.off("sidebar.focusSearch", globalSearch); @@ -234,3 +236,6 @@ export const globalSearch: Listener<"sidebar.focusSearch"> = (_, ctx) => { }, }); }; + +export const openLogs: Listener<"app.openLogDirectory"> = () => + ipc("app.openLogDirectory"); diff --git a/src/renderer/menus/appMenu.ts b/src/renderer/menus/appMenu.ts index fdc2bb3f..2d11ddce 100644 --- a/src/renderer/menus/appMenu.ts +++ b/src/renderer/menus/appMenu.ts @@ -30,6 +30,12 @@ export function useApplicationMenu(store: Store): void { shortcut: shortcutLabels["app.openDevTools"], event: "app.openDevTools", }, + { + label: "Open logs", + type: "normal", + shortcut: shortcutLabels["app.openLogDirectory"], + event: "app.openLogDirectory", + }, ], }); } diff --git a/src/renderer/store.ts b/src/renderer/store.ts index ad44e9a4..d53ea2bc 100644 --- a/src/renderer/store.ts +++ b/src/renderer/store.ts @@ -7,6 +7,7 @@ import { Note } from "../shared/domain/note"; import { Shortcut } from "../shared/domain/shortcut"; import { UIEventType, UIEventInput } from "../shared/ui/events"; import { Section, AppState } from "../shared/ui/app"; +import { log } from "./logger"; export interface Store { state: State; @@ -182,6 +183,7 @@ export function useStore(initialState: State): Store { async (event, value: any) => { const eventListeners: any = listeners.current[event]; if (eventListeners == null || eventListeners.length === 0) { + log.debug(`No store listener found for ${event}.`); return; } diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index 59495ae0..05532bc7 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -17,6 +17,7 @@ export const IPCS = [ "app.loadAppState", "app.saveAppState", "app.openInWebBrowser", + "app.openLogDirectory", "shortcuts.getAll", @@ -54,6 +55,7 @@ export interface IpcSchema extends Record any> { "app.loadAppState"(): Promise; "app.saveAppState"(ui: AppState): Promise; "app.openInWebBrowser"(url: string): Promise; + "app.openLogDirectory"(): Promise; // Shortcuts "shortcuts.getAll"(): Promise; diff --git a/src/shared/ui/events.ts b/src/shared/ui/events.ts index 68c65118..9f060358 100644 --- a/src/shared/ui/events.ts +++ b/src/shared/ui/events.ts @@ -19,6 +19,7 @@ export interface UIEvents { "app.toggleFullScreen": void; "app.inspectElement": Point; "app.toggleSidebar": void; + "app.openLogDirectory": void; // Sidebar "sidebar.updateScroll": number; @@ -86,6 +87,7 @@ export const LIST_OF_EVENTS: (keyof UIEvents)[] = [ "app.toggleFullScreen", "app.inspectElement", "app.toggleSidebar", + "app.openLogDirectory", // Sidebar "sidebar.updateScroll", From bd6f92cf5b00d98c1895ba0e670527492165e790 Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sun, 25 Sep 2022 15:29:10 -0400 Subject: [PATCH 8/9] Finish up feature --- src/renderer/logger.ts | 4 ++++ test/renderer/logger.spec.ts | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 test/renderer/logger.spec.ts diff --git a/src/renderer/logger.ts b/src/renderer/logger.ts index b017db3a..e7bcec6e 100644 --- a/src/renderer/logger.ts +++ b/src/renderer/logger.ts @@ -9,18 +9,22 @@ if (!isTest() && getProcessType() !== "renderer") { export const log: Logger = { info: async (message) => { + // eslint-disable-next-line no-console console.log(message); window.ipc("log.info", message); }, warn: async (message) => { + // eslint-disable-next-line no-console console.warn(message); window.ipc("log.warn", message); }, error: async (message) => { + // eslint-disable-next-line no-console console.error(message); window.ipc("log.error", message); }, debug: async (message) => { + // eslint-disable-next-line no-console console.log(message); window.ipc("log.debug", message); }, diff --git a/test/renderer/logger.spec.ts b/test/renderer/logger.spec.ts new file mode 100644 index 00000000..a9ac0136 --- /dev/null +++ b/test/renderer/logger.spec.ts @@ -0,0 +1,13 @@ +import { log } from "../../src/renderer/logger"; +import { getProcessType, isTest } from "../../src/shared/env"; + +jest.mock("../../src/shared/env"); + +test("throws if imported outside of renderer", async () => { + (isTest as jest.Mock).mockReturnValue(false); + (getProcessType as jest.Mock).mockReturnValue("main"); + + expect(() => require("../../src/renderer/logger")).toThrow( + /window.ipc is null. Did you accidentally import logger.ts in the main thread?/ + ); +}); From 4b8c6199443da5f3d92ea0f874e6b3d1e74963bb Mon Sep 17 00:00:00 2001 From: Ed Abbondanzio Date: Sun, 25 Sep 2022 15:42:43 -0400 Subject: [PATCH 9/9] Delete log files if they are empty --- src/main/index.ts | 2 ++ src/main/log.ts | 14 ++++++++++++-- test/main/log.spec.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 04501885..6f8b4de9 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -123,6 +123,8 @@ export async function main(): Promise { }); app.on("quit", () => { + log.close(); + // Use console.log() over log.info to avoid appending this to the log file // eslint-disable-next-line no-console console.log(`Shutting down. Log saved to: ${log.filePath}`); diff --git a/src/main/log.ts b/src/main/log.ts index eaae2adf..faf6fea8 100644 --- a/src/main/log.ts +++ b/src/main/log.ts @@ -45,7 +45,7 @@ export function logIpcs( export async function getLogger( config: JsonFile, c: Console -): Promise { +): Promise void }> { const { logDirectory } = config.content; if (logDirectory != null && !fs.existsSync(logDirectory)) { await fsp.mkdir(logDirectory); @@ -102,7 +102,17 @@ export async function getLogger( }, }; - return { ...log, filePath: currFilePath }; + // Cannot be async, otherwise app stops running before empty log is deleted. + const close = () => { + fileStream.close(); + + // Delete empty log files to avoid spamming file system. + if (fs.statSync(currFilePath).size === 0) { + fs.unlinkSync(currFilePath); + } + }; + + return { ...log, filePath: currFilePath, close }; } export function getTimeStamp(): string { diff --git a/test/main/log.spec.ts b/test/main/log.spec.ts index e188472f..71178437 100644 --- a/test/main/log.spec.ts +++ b/test/main/log.spec.ts @@ -106,3 +106,32 @@ test("getLogger logs to file", async () => { expect.stringMatching(/.* \(ERROR\): something went wrong!/) ); }); + +test("getLogger close deletes empty log files", async () => { + (fs.createWriteStream as jest.Mock).mockReturnValue({ + write: jest.fn(), + close: jest.fn(), + }); + (fsp.readdir as jest.Mock).mockResolvedValueOnce([]); + + const configFile = createJsonFile({ + logDirectory: "logs", + } as Config); + const log = await getLogger(configFile, { + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + } as unknown as Console); + + (fs.statSync as jest.Mock).mockReturnValueOnce({ + size: 10, + }); + log.close(); + expect(fs.unlinkSync).not.toBeCalledWith(log.filePath); + + (fs.statSync as jest.Mock).mockReturnValueOnce({ + size: 0, + }); + log.close(); + expect(fs.unlinkSync).toBeCalledWith(log.filePath); +});