From fbf8b6631a530ea348c5214d6c6b118746e8b50d Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 Nov 2024 13:39:56 +0000 Subject: [PATCH 1/6] fix: #15 --- src/lib/adapters/utils.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/lib/adapters/utils.ts b/src/lib/adapters/utils.ts index 5c524e8..06fd992 100644 --- a/src/lib/adapters/utils.ts +++ b/src/lib/adapters/utils.ts @@ -46,6 +46,12 @@ export function deepMerge(target: any, ...sources: any[]) { return deepMerge(target, ...sources); } -export function isMergeableObject(item: any) { - return item && typeof item === "object" && !Array.isArray(item); +export function isMergeableObject(item: unknown): item is Record { + if (!item) return false + if (typeof item !== "object") return false + // ES6 class instances, Maps, Sets, Arrays, etc. are not considered records + if (Object.getPrototypeOf(item) === Object.prototype) return true + // Some library/Node.js functions return records with null prototype + if (Object.getPrototypeOf(item) === null) return true + return false } From 3d125a6aa0e64e9e3352442eb1473a51ae05b155 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 Nov 2024 13:50:49 +0000 Subject: [PATCH 2/6] improve type-safety using `unknown` --- src/lib/adapters/utils.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/adapters/utils.ts b/src/lib/adapters/utils.ts index 06fd992..4b94852 100644 --- a/src/lib/adapters/utils.ts +++ b/src/lib/adapters/utils.ts @@ -1,5 +1,5 @@ export const filterByPrefixKey = ( - data: { [key: string]: any } | undefined | null, + data: Partial> | undefined | null, prefixKey: string, ) => { if (!data) { @@ -8,14 +8,14 @@ export const filterByPrefixKey = ( return Object.keys(data) .filter((key) => key.startsWith(prefixKey)) - .reduce<{ [key: string]: any }>((acc, key) => { + .reduce>>((acc, key) => { acc[key] = data[key]; return acc; }, {}); }; -export function deepMerge(target: any, ...sources: any[]) { +export function deepMerge(target: unknown, ...sources: unknown[]) { if (!sources.length) { return target; } @@ -46,7 +46,7 @@ export function deepMerge(target: any, ...sources: any[]) { return deepMerge(target, ...sources); } -export function isMergeableObject(item: unknown): item is Record { +export function isMergeableObject(item: unknown): item is Partial> { if (!item) return false if (typeof item !== "object") return false // ES6 class instances, Maps, Sets, Arrays, etc. are not considered records From bbffd3de6cfb47916aa8fdb7a75c090f14d0eb20 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 Nov 2024 13:53:47 +0000 Subject: [PATCH 3/6] accept any value for `filterByPrefixKey` and `throw` for invalid values --- src/lib/adapters/utils.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/adapters/utils.ts b/src/lib/adapters/utils.ts index 4b94852..1dc8c13 100644 --- a/src/lib/adapters/utils.ts +++ b/src/lib/adapters/utils.ts @@ -1,10 +1,9 @@ export const filterByPrefixKey = ( - data: Partial> | undefined | null, + data: unknown, prefixKey: string, ) => { - if (!data) { - return {}; - } + if (data == null) return {} + if (!isMergeableObject(data)) throw new TypeError(`Cannot filter ${data} by prefix key as it is not a record-like object`) return Object.keys(data) .filter((key) => key.startsWith(prefixKey)) From 678456f9180866c633478f36cc942789148fc996 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:06:50 +0000 Subject: [PATCH 4/6] fix: typescript cannot remember inference on indexed types --- src/lib/adapters/utils.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/lib/adapters/utils.ts b/src/lib/adapters/utils.ts index 1dc8c13..12fa003 100644 --- a/src/lib/adapters/utils.ts +++ b/src/lib/adapters/utils.ts @@ -14,7 +14,10 @@ export const filterByPrefixKey = ( }, {}); }; -export function deepMerge(target: unknown, ...sources: unknown[]) { +export function deepMerge( + target: Partial>, + ...sources: unknown[] +): Partial> { if (!sources.length) { return target; } @@ -34,11 +37,13 @@ export function deepMerge(target: unknown, ...sources: unknown[]) { return; } - if (!target[key]) { - target[key] = {}; + let subTarget = target[key] + if (!isMergeableObject(subTarget)) { + target[key] = subTarget = {} + return } - deepMerge(target[key], source[key]); + deepMerge(subTarget, source[key]); }); } From c15080969ba11f11cc3c53a58fbbab61550b4d2b Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:26:49 +0000 Subject: [PATCH 5/6] fix: deep-merge before early return --- src/lib/adapters/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/adapters/utils.ts b/src/lib/adapters/utils.ts index 12fa003..a92526a 100644 --- a/src/lib/adapters/utils.ts +++ b/src/lib/adapters/utils.ts @@ -37,9 +37,9 @@ export function deepMerge( return; } - let subTarget = target[key] + const subTarget = target[key] if (!isMergeableObject(subTarget)) { - target[key] = subTarget = {} + target[key] = deepMerge({}, source[key]) return } From 24c3e66431bcc8db8f7167b98cda4904e91f73d6 Mon Sep 17 00:00:00 2001 From: Alexandre Marques Date: Sat, 2 Nov 2024 11:39:50 +0000 Subject: [PATCH 6/6] chore: add some unit tests --- tests/script-adapter.test.ts | 63 ++++++++++++++++++++++++++++++++++++ tests/utils.test.ts | 25 +++++++++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/tests/script-adapter.test.ts b/tests/script-adapter.test.ts index 2bc1526..13249a3 100644 --- a/tests/script-adapter.test.ts +++ b/tests/script-adapter.test.ts @@ -52,3 +52,66 @@ describe.each([ expect(config.PORT).toBe(expected.PORT); }); }); + +describe("combining multiple script adapters", () => { + const testFilePath1 = path.join(__dirname, "test-multiple-script-adapter1.ts"); + const testFilePath2 = path.join(__dirname, "test-multiple-script-adapter2.ts"); + + beforeAll(async () => { + const fileContent1 = `export default { + HOST: "app name", + PORT: "1111", + TEST_MAP: new Map([["key", "value"], ["key2", "value2"]]), + TEST_RECORD: { key: "key", value: "value1" } + }`; + const fileContent2 = `export default { + HOST: "app name2", + PORT: "1234", + TEST_MAP: new Map([["key", "value2"]]), + TEST_RECORD: { key: "key2" } + }`; + + await Promise.all([ + writeFile(testFilePath1, fileContent1), + writeFile(testFilePath2, fileContent2), + ]); + }); + + afterAll(async () => { + await Promise.all([ + unlink(testFilePath1), + unlink(testFilePath2), + ]); + }); + + it("should successfully parse, merge and return type-safe data", async () => { + // given + const schema = z.object({ + HOST: z.string(), + PORT: z.string().regex(/^\d+$/), + TEST_RECORD: z.object({ + key: z.string(), + value: z.string() + }), + TEST_MAP: z.map(z.string(), z.string()).optional(), + }); + + // when + const config = await loadConfig({ + schema, + adapters: [ + scriptAdapter({ path: testFilePath1 }), + scriptAdapter({ path: testFilePath2 }), + ], + }); + + // then + expect(config.HOST).toBe("app name2"); // second adapter overrides the first one + expect(config.PORT).toBe("1234"); // second adapter overrides the first one + expect(config.TEST_MAP).toEqual(new Map([["key", "value2"]])); // MAP is not mergeable so only the last one is loaded + expect(config.TEST_RECORD).toEqual({ + key: "key2", // from second adapter + value: "value1" // preserved from first adapter + }); // records are merged between adapters + }); +}); diff --git a/tests/utils.test.ts b/tests/utils.test.ts index 1ac8e1a..a3f5171 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -1,4 +1,4 @@ -import { deepMerge, filterByPrefixKey } from "../src/lib/adapters/utils"; +import { deepMerge, filterByPrefixKey, isMergeableObject } from "../src/lib/adapters/utils"; import { describe, it, expect } from "vitest"; describe("filterByPrefixKey", () => { @@ -93,3 +93,26 @@ describe("deepMerge", () => { expect(result).toEqual({ a: null, b: 2, c: [] }); }); }); + +describe("isMergeableObject", () => { + it("should return true for plain objects", () => { + expect(isMergeableObject({})).toBe(true); + expect(isMergeableObject({ a: 1 })).toBe(true); + }); + + it("should return false for non-mergeable values", () => { + // null and undefined + expect(isMergeableObject(null)).toBe(false); + expect(isMergeableObject(undefined)).toBe(false); + + // primitives + expect(isMergeableObject(10)).toBe(false); + expect(isMergeableObject("string")).toBe(false); + expect(isMergeableObject(true)).toBe(false); + + // built-in objects and collections + expect(isMergeableObject([])).toBe(false); + expect(isMergeableObject(new Map())).toBe(false); + expect(isMergeableObject(new Set())).toBe(false); + }); +});