From 32d940f801515bd21c8ee614ae6412e837051c02 Mon Sep 17 00:00:00 2001 From: Abdulrahman Alfawal <61620817+alfawal@users.noreply.github.com> Date: Sat, 17 Oct 2026 21:19:36 +0300 Subject: [PATCH] fix: warn about unknown and misplaced config keys Zod strips keys it doesn't know, so an option set at the top level of the config (like `hideIcon` in #544) or a misspelled key was silently ignored. Log a warning for those when the config loads, and point misplaced options to `options`. --- packages/config-api/src/config.test.ts | 46 +++++++++++++++++++++++-- packages/config-api/src/configSchema.ts | 2 +- packages/config-api/src/index.ts | 35 +++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/packages/config-api/src/config.test.ts b/packages/config-api/src/config.test.ts index 57b03be..1b25262 100644 --- a/packages/config-api/src/config.test.ts +++ b/packages/config-api/src/config.test.ts @@ -1,7 +1,49 @@ -import { describe, it, expect, vi } from "vitest"; -import { openUrl } from "./index"; +import { describe, it, expect, vi, afterEach } from "vitest"; +import { getConfiguration, openUrl } from "./index"; import { Config, ProcessInfo } from "./configSchema"; +describe("getConfiguration", () => { + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + const loadConfig = (config: object) => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.stubGlobal("self", { finickyConfig: { default: config } }); + getConfiguration("finickyConfig"); + return warn.mock.calls.map(([message]) => message); + }; + + it("warns about options set at the top level", () => { + const warnings = loadConfig({ defaultBrowser: "Safari", hideIcon: true }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('Ignoring "hideIcon" at the top level'); + }); + + it("warns about unknown properties and options", () => { + const warnings = loadConfig({ + defaultBrowser: "Safari", + handler: [], + options: { checkForUpdate: false }, + }); + expect(warnings).toEqual([ + 'Ignoring unknown configuration property "handler".', + 'Ignoring unknown option "options.checkForUpdate".', + ]); + }); + + it("doesn't warn about a valid configuration", () => { + const warnings = loadConfig({ + defaultBrowser: "Safari", + options: { hideIcon: true, keepRunning: true }, + rewrite: [], + handlers: [], + }); + expect(warnings).toEqual([]); + }); +}); + describe("openUrl", () => { const mockProcessInfo: ProcessInfo = { name: "TestApp", diff --git a/packages/config-api/src/configSchema.ts b/packages/config-api/src/configSchema.ts index cea27ae..c74706f 100644 --- a/packages/config-api/src/configSchema.ts +++ b/packages/config-api/src/configSchema.ts @@ -129,7 +129,7 @@ const HandlerRuleSchema = z .identifier("BrowserHandler"); // ===== Configuration Schemas ===== -const ConfigOptionsSchema = z +export const ConfigOptionsSchema = z .object({ urlShorteners: z.array(z.string()).optional(), logRequests: z.boolean().optional().describe("Log to file on disk"), diff --git a/packages/config-api/src/index.ts b/packages/config-api/src/index.ts index 4723550..652b0c2 100644 --- a/packages/config-api/src/index.ts +++ b/packages/config-api/src/index.ts @@ -2,6 +2,7 @@ import "core-js/features/url"; import "core-js/features/url-search-params"; import { ConfigSchema, + ConfigOptionsSchema, Config, OpenUrlOptions, ProcessInfo, @@ -44,11 +45,13 @@ export function getConfiguration(namespace: string): Config { const namespaceObj = (self as any)[namespace]; if (namespaceObj) { if (namespaceObj.default) { + warnAboutUnknownKeys(namespaceObj.default); return namespaceObj.default; } else { console.warn( "No default export found for configuration namespace, using legacy configuration. Please update your configuration to use `export default { ... }` instead of `module.exports = { ... }` syntax. https://github.com/johnste/finicky/wiki/Use-Modern-ECMAScript-Module-Syntax" ); + warnAboutUnknownKeys(namespaceObj); return namespaceObj; } } @@ -58,6 +61,38 @@ export function getConfiguration(namespace: string): Config { ); } +// Zod strips unknown keys silently, so a misplaced option (like a top-level +// `hideIcon`) would otherwise just be ignored. +function warnAboutUnknownKeys(config: unknown) { + if (!config || typeof config !== "object") { + return; + } + + const configKeys = new Set(Object.keys(ConfigSchema.shape)); + // `.identifier()` types the options schema as a plain ZodType, so read its + // keys from the definition instead of `.shape` + const optionKeys = new Set(Object.keys(ConfigOptionsSchema._def.shape())); + + for (const key of Object.keys(config)) { + if (optionKeys.has(key)) { + console.warn( + `Ignoring "${key}" at the top level of the configuration, it needs to be inside \`options\`. https://github.com/johnste/finicky/wiki/Configuration-(v4)#general-options` + ); + } else if (!configKeys.has(key)) { + console.warn(`Ignoring unknown configuration property "${key}".`); + } + } + + const options = "options" in config ? config.options : undefined; + if (options && typeof options === "object") { + for (const key of Object.keys(options)) { + if (!optionKeys.has(key)) { + console.warn(`Ignoring unknown option "options.${key}".`); + } + } + } +} + export function getOption( option: keyof Config["options"], config: Config,