From 3819b37a9f213100f1a8e0b673256e6238258d48 Mon Sep 17 00:00:00 2001 From: James Kebinger Date: Fri, 9 May 2025 13:58:46 -0500 Subject: [PATCH 1/2] Adds a way to register for notifications on config change --- package-lock.json | 178 ++++++++++++++++ package.json | 1 + src/__tests__/configChangeNotifier.test.ts | 209 +++++++++++++++++++ src/__tests__/integration.test.ts | 12 +- src/__tests__/prefab.test.ts | 232 ++++++++++++++++++++- src/configChangeNotifier.ts | 128 ++++++++++++ src/prefab.ts | 130 ++++++++---- src/resolver.ts | 59 +++++- 8 files changed, 889 insertions(+), 60 deletions(-) create mode 100644 src/__tests__/configChangeNotifier.test.ts create mode 100644 src/configChangeNotifier.ts diff --git a/package-lock.json b/package-lock.json index 16fd7db..59bb8d6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,6 +15,7 @@ "protobufjs": "^7.2.3" }, "devDependencies": { + "bun": "^1.2.12", "@types/jest": "^29.5.1", "@types/yaml": "^1.9.7", "@typescript-eslint/eslint-plugin": "^5.59.2", @@ -1252,6 +1253,149 @@ "node": ">= 8" } }, + "node_modules/@oven/bun-darwin-aarch64": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-darwin-aarch64/-/bun-darwin-aarch64-1.2.12.tgz", + "integrity": "sha512-4xH3AwitT21tOeUdxwz5vLei0FYP2rh65OHMTxgrZzH6+DiLAryQpOAJv41yOH8cWKmOclQWD7B8rCbKFiPekg==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/@oven/bun-darwin-x64": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-darwin-x64/-/bun-darwin-x64-1.2.12.tgz", + "integrity": "sha512-L3s33xv/e4DnF23oD2mNhmb7CpHA/uTSX4h1WbkhamTv1gJkUV2N+irZ18QB5iKOCt1q2u5ED1zpoPE2dthLzA==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/@oven/bun-darwin-x64-baseline": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-darwin-x64-baseline/-/bun-darwin-x64-baseline-1.2.12.tgz", + "integrity": "sha512-n3uCYO2MIWrRvczAYvPvtqmxzAPvBatP9E9z8e//CqgyTUr0DabNI07p+5S3433hC38hIgLtY2DsKTkfuM5eCQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/@oven/bun-linux-aarch64": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-linux-aarch64/-/bun-linux-aarch64-1.2.12.tgz", + "integrity": "sha512-uz6fBRrUkG7puT0snGeG476B9Cqrqc3pjFTAls0yMlFWSbCJpQfSO77jp3TklMOvEPreA0yG/8c3t9OKImAkxw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@oven/bun-linux-aarch64-musl": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-linux-aarch64-musl/-/bun-linux-aarch64-musl-1.2.12.tgz", + "integrity": "sha512-31ySDZXHgGxcMoxWSIupWhSug/Ddjwtfg8iTf9lEI2jymj1zaMPCt2jPLriw5/V5gDtuTWW1E29PlrLIiCxoCA==", + "cpu": [ + "aarch64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@oven/bun-linux-x64": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-linux-x64/-/bun-linux-x64-1.2.12.tgz", + "integrity": "sha512-gNjguT4fuekF1i0/OuDd+7FcjHkv2qE0xBTwZMmXOzFOADPRo5zLB4LIcwfKpiZT+jZlZNdVOao05ebdEadbIw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@oven/bun-linux-x64-baseline": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-linux-x64-baseline/-/bun-linux-x64-baseline-1.2.12.tgz", + "integrity": "sha512-JOX0GhMfoNADCxe2rlvXOsu05VmGis0vqghELN91vuWHAUt/8RQ1CvIJ47DlrQuKbxpYs1SjV4J+bwbte/RUAA==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@oven/bun-linux-x64-musl": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-linux-x64-musl/-/bun-linux-x64-musl-1.2.12.tgz", + "integrity": "sha512-8lSQ71mYx1WLzAvAHFaJwzWamdfcaWaqJemuIX0/5Njy36ieqo05gSc6k7c9gaZEb9aFK0bQ4S/f3enthO5HJQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@oven/bun-linux-x64-musl-baseline": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-linux-x64-musl-baseline/-/bun-linux-x64-musl-baseline-1.2.12.tgz", + "integrity": "sha512-6Gb8szm3mOLcHNUmuBbvDAqb7weqpsMJa/3cAkdWYa+svx5X1j3r7fXpzc0oSVKKqnlg9lIf69lza4iuJjVQfg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@oven/bun-windows-x64": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-windows-x64/-/bun-windows-x64-1.2.12.tgz", + "integrity": "sha512-tyfHFOxBoBgIJmx9rAcSykmLKcLJOTg5eAbyz0jbZHm85ClLLNynBi+rp1pmEMm8wPxmMzdjNhc6XFSUoA4/pg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, + "node_modules/@oven/bun-windows-x64-baseline": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/@oven/bun-windows-x64-baseline/-/bun-windows-x64-baseline-1.2.12.tgz", + "integrity": "sha512-8wwIRhQPibE9nGhZdy8MW8ueCCs/VLtJjDtV3bIZNl82SfXbVyObxQkYm6BjItKid2OTb5eLMcKc0kbd3fAPLw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, "node_modules/@protobufjs/aspromise": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/@protobufjs/aspromise/-/aspromise-1.1.2.tgz", @@ -2113,6 +2257,40 @@ "semver": "^7.0.0" } }, + "node_modules/bun": { + "version": "1.2.12", + "resolved": "https://registry.npmjs.org/bun/-/bun-1.2.12.tgz", + "integrity": "sha512-66kW1iPrYLEC3MEsmYJhcPdNwMrT6V+l0cwcI9wl1jt4a8lhBhomgM16jYnSDzBros2C/8oJ/aZY8Y9HUvLLTA==", + "cpu": [ + "arm64", + "x64", + "aarch64" + ], + "hasInstallScript": true, + "license": "MIT", + "os": [ + "darwin", + "linux", + "win32" + ], + "bin": { + "bun": "bin/bun.exe", + "bunx": "bin/bun.exe" + }, + "optionalDependencies": { + "@oven/bun-darwin-aarch64": "1.2.12", + "@oven/bun-darwin-x64": "1.2.12", + "@oven/bun-darwin-x64-baseline": "1.2.12", + "@oven/bun-linux-aarch64": "1.2.12", + "@oven/bun-linux-aarch64-musl": "1.2.12", + "@oven/bun-linux-x64": "1.2.12", + "@oven/bun-linux-x64-baseline": "1.2.12", + "@oven/bun-linux-x64-musl": "1.2.12", + "@oven/bun-linux-x64-musl-baseline": "1.2.12", + "@oven/bun-windows-x64": "1.2.12", + "@oven/bun-windows-x64-baseline": "1.2.12" + } + }, "node_modules/call-bind": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/call-bind/-/call-bind-1.0.2.tgz", diff --git a/package.json b/package.json index 4fe9f99..a902cde 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "author": "Jeffrey Chupp", "license": "ISC", "devDependencies": { + "bun": "^1.2.12", "@types/jest": "^29.5.1", "@types/yaml": "^1.9.7", "@typescript-eslint/eslint-plugin": "^5.59.2", diff --git a/src/__tests__/configChangeNotifier.test.ts b/src/__tests__/configChangeNotifier.test.ts new file mode 100644 index 0000000..42a0265 --- /dev/null +++ b/src/__tests__/configChangeNotifier.test.ts @@ -0,0 +1,209 @@ +import Long from "long"; +import { + ConfigChangeNotifier, + type ResolverInterface, +} from "../configChangeNotifier"; +import type { MinimumConfig } from "../resolver"; + +// Mock ResolverInterface +const mockResolver: jest.Mocked = { + keys: jest.fn(), + raw: jest.fn(), +}; + +describe("ConfigChangeNotifier", () => { + let notifier: ConfigChangeNotifier; + let listenerCallback: jest.Mock; + + beforeEach(() => { + notifier = new ConfigChangeNotifier(); + listenerCallback = jest.fn(); + // Reset mocks for each test + mockResolver.keys.mockReset(); + mockResolver.raw.mockReset(); + // Set a default return value for keys to prevent "not iterable" errors + mockResolver.keys.mockReturnValue([]); + }); + + describe("constructor and init", () => { + it("should initialize with ZERO lastTotalId before init", () => { + expect(notifier.getLastTotalId().equals(Long.ZERO)).toBe(true); + }); + + it("should calculate and set lastTotalId on init", () => { + mockResolver.keys.mockReturnValue(["key1"]); + mockResolver.raw.mockReturnValue({ id: Long.fromInt(10) }); + + notifier.init(mockResolver); + expect(notifier.getLastTotalId().toNumber()).toBe(10); + }); + + it("should set lastTotalId to ZERO if resolver has no configs on init", () => { + mockResolver.keys.mockReturnValue([]); + notifier.init(mockResolver); + expect(notifier.getLastTotalId().equals(Long.ZERO)).toBe(true); + }); + + it("should allow re-initialization with a new resolver and update lastTotalId", () => { + // First initialization + mockResolver.keys.mockReturnValue(["key1"]); + mockResolver.raw.mockReturnValue({ id: Long.fromInt(10) }); + notifier.init(mockResolver); + expect(notifier.getLastTotalId().toNumber()).toBe(10); + expect(mockResolver.keys).toHaveBeenCalledTimes(1); // Called during the first init + + // Create a new mock resolver for the second initialization + const newMockResolver: jest.Mocked = { + keys: jest.fn().mockReturnValue(["key2", "key3"]), + raw: jest.fn((key: string) => { + if (key === "key2") return { id: Long.fromInt(20) }; + if (key === "key3") return { id: Long.fromInt(30) }; // New max ID + return undefined; + }), + }; + + // Second initialization with the new resolver + notifier.init(newMockResolver); + expect(notifier.getLastTotalId().toNumber()).toBe(30); // Should reflect newMockResolver's state + expect(newMockResolver.keys).toHaveBeenCalledTimes(1); // Called during the second init + + // Ensure the original mockResolver's keys was not called again during the second init + expect(mockResolver.keys).toHaveBeenCalledTimes(1); + }); + }); + + describe("addListener and removeListener", () => { + it("should add a listener and return an unsubscribe function", () => { + const unsubscribe = notifier.addListener(listenerCallback); + + // Init with totalId = 0 (mockResolver.keys returns [] by default) + notifier.init(mockResolver); + expect(notifier.getLastTotalId().equals(Long.ZERO)).toBe(true); + + // Trigger with new totalId = 1 + mockResolver.keys.mockReturnValueOnce(["key1"]); // Override for this specific call inside handleResolverUpdate + mockResolver.raw.mockReturnValueOnce({ id: Long.fromInt(1) }); + notifier.handleResolverUpdate([]); + expect(listenerCallback).toHaveBeenCalledTimes(1); + + unsubscribe(); + // For the next handleResolverUpdate, keys will revert to default mockReturnValue([]) or need to be set again + // If we want to test it changes again and listener is NOT called: + mockResolver.keys.mockReturnValueOnce(["key2"]); // Setup for next call + mockResolver.raw.mockReturnValueOnce({ id: Long.fromInt(2) }); // Change ID again + notifier.handleResolverUpdate([]); + expect(listenerCallback).toHaveBeenCalledTimes(1); // Still 1, as it was unsubscribed + }); + + it("should remove a listener with removeListener", () => { + notifier.addListener(listenerCallback); + notifier.removeListener(listenerCallback); + + mockResolver.keys.mockReturnValueOnce([]).mockReturnValueOnce(["key1"]); + mockResolver.raw.mockReturnValueOnce({ id: Long.fromInt(1) }); + notifier.init(mockResolver); + notifier.handleResolverUpdate([]); + + expect(listenerCallback).not.toHaveBeenCalled(); + }); + }); + + describe("handleResolverUpdate", () => { + beforeEach(() => { + // Ensure notifier is initialized for these tests + mockResolver.keys.mockReturnValue([]); // Start with no configs, totalId = 0 + notifier.init(mockResolver); + notifier.addListener(listenerCallback); + }); + + it("should notify listeners when totalId changes", () => { + mockResolver.keys.mockReturnValue(["key1"]); + mockResolver.raw.mockReturnValue({ id: Long.fromInt(5) }); // New totalId = 5 + + notifier.handleResolverUpdate([]); + + expect(listenerCallback).toHaveBeenCalledTimes(1); + expect(notifier.getLastTotalId().toNumber()).toBe(5); + }); + + it("should NOT notify listeners when totalId does not change", () => { + mockResolver.keys.mockReturnValue(["key1"]); + mockResolver.raw.mockReturnValue({ id: Long.fromInt(0) }); // Same as initial Long.ZERO + + notifier.handleResolverUpdate([]); + + expect(listenerCallback).not.toHaveBeenCalled(); + expect(notifier.getLastTotalId().equals(Long.ZERO)).toBe(true); + }); + + it("should handle multiple listeners", () => { + const listener2 = jest.fn(); + notifier.addListener(listener2); + + mockResolver.keys.mockReturnValue(["key1"]); + mockResolver.raw.mockReturnValue({ id: Long.fromInt(10) }); + + notifier.handleResolverUpdate([]); + + expect(listenerCallback).toHaveBeenCalledTimes(1); + expect(listener2).toHaveBeenCalledTimes(1); + }); + + it("should log a warning if handleResolverUpdate is called before init", () => { + const freshNotifier = new ConfigChangeNotifier(); // Not initialized + const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(); + + freshNotifier.handleResolverUpdate([]); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + "ConfigChangeNotifier.handleResolverUpdate called before init. Skipping." + ); + consoleWarnSpy.mockRestore(); + }); + }); + + describe("calculateCurrentTotalId", () => { + it("should return Long.ZERO if resolver is not initialized", () => { + const freshNotifier = new ConfigChangeNotifier(); + // Access private method for testing - might need @ts-ignore or a different approach for strictness + expect( + (freshNotifier as any).calculateCurrentTotalId().equals(Long.ZERO) + ).toBe(true); + }); + + it("should return Long.ZERO for no keys", () => { + mockResolver.keys.mockReturnValue([]); + notifier.init(mockResolver); + expect( + (notifier as any).calculateCurrentTotalId().equals(Long.ZERO) + ).toBe(true); + }); + + it("should correctly calculate maxId from resolver data", () => { + mockResolver.keys.mockReturnValue(["key1", "key2", "key3"]); + mockResolver.raw.mockImplementation((key: string) => { + if (key === "key1") return { id: Long.fromInt(1) }; + if (key === "key2") return { id: Long.fromInt(100) }; // Max + if (key === "key3") return { id: Long.fromInt(50) }; + return undefined; + }); + notifier.init(mockResolver); // Recalculates based on this new mock setup for the test + expect((notifier as any).calculateCurrentTotalId().toNumber()).toBe(100); + }); + + it("should ignore configs with no id or non-Long id", () => { + mockResolver.keys.mockReturnValue(["key1", "key2", "key3", "key4"]); + mockResolver.raw.mockImplementation( + (key: string): Pick | undefined => { + if (key === "key1") return { id: Long.fromInt(5) }; + if (key === "key2") return { id: undefined }; // id is present but undefined + if (key === "key3") return { id: "not-a-long" as any }; // id is present but not a Long + if (key === "key4") return { id: undefined }; // Represents a config object that might exist but its id is not set or not relevant + return undefined; + } + ); + notifier.init(mockResolver); + expect((notifier as any).calculateCurrentTotalId().toNumber()).toBe(5); + }); + }); +}); diff --git a/src/__tests__/integration.test.ts b/src/__tests__/integration.test.ts index 5c58fa9..8393aa9 100644 --- a/src/__tests__/integration.test.ts +++ b/src/__tests__/integration.test.ts @@ -1,17 +1,21 @@ import { Prefab } from "../prefab"; import type { PrefabInterface } from "../prefab"; +import type { ResolverAPI } from "../resolver"; import type { GetValue } from "../unwrap"; import { tests } from "./integrationHelper"; import type { InputOutputTest } from "./integrationHelper"; -const func = (prefab: PrefabInterface, test: InputOutputTest): any => { +const func = ( + client: PrefabInterface | ResolverAPI, + test: InputOutputTest +): any => { switch (test.function) { case "get": - return prefab.get.bind(prefab); + return client.get.bind(client); case "get_or_raise": - return prefab.get.bind(prefab); + return client.get.bind(client); case "enabled": - return prefab.isFeatureEnabled.bind(prefab); + return client.isFeatureEnabled.bind(client); default: throw new Error(`Unknown function: ${test.function}`); } diff --git a/src/__tests__/prefab.test.ts b/src/__tests__/prefab.test.ts index baaf442..090891c 100644 --- a/src/__tests__/prefab.test.ts +++ b/src/__tests__/prefab.test.ts @@ -12,9 +12,15 @@ import envConfig from "./fixtures/envConfig"; import propIsOneOf from "./fixtures/propIsOneOf"; import propIsOneOfAndEndsWith from "./fixtures/propIsOneOfAndEndsWith"; import { Prefab, MULTIPLE_INIT_WARNING } from "../prefab"; -import type { Contexts } from "../types"; -import { LogLevel, Criterion_CriterionOperator, ConfigType } from "../proto"; -import type { Config } from "../proto"; +import type { Contexts, ProjectEnvId } from "../types"; +import type { GetValue } from "../unwrap"; +import { + LogLevel, + Criterion_CriterionOperator, + ConfigType, + Config_ValueType, +} from "../proto"; +import type { Config, ConfigValue } from "../proto"; import { encrypt, generateNewHexKey } from "../../src/encryption"; import secretConfig from "./fixtures/secretConfig"; import decryptionKeyConfig from "./fixtures/decryptionKeyConfig"; @@ -82,13 +88,17 @@ describe("prefab", () => { it("throws a 401 if you have an invalid API key", async () => { const prefab = new Prefab({ apiKey: invalidApiKey }); - jest.spyOn(console, "warn").mockImplementation(); + const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(); + const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(); await expect(prefab.init()).rejects.toThrow( "Unauthorized. Check your Prefab SDK API key for https://suspenders.prefab.cloud/api/v1/configs/0" ); - expect(console.warn).toHaveBeenCalled(); + expect(consoleWarnSpy).toHaveBeenCalled(); + + consoleErrorSpy.mockRestore(); + consoleWarnSpy.mockRestore(); }); [mostlyDeletedConfig, deletedConfig].forEach((deletionConfig) => { @@ -213,8 +223,12 @@ describe("prefab", () => { expect(mock).not.toHaveBeenCalled(); await prefab.init(); - expect(mock).toHaveBeenCalled(); - expect(mock.mock.calls).toStrictEqual([[MULTIPLE_INIT_WARNING]]); + expect(mock).toHaveBeenCalledTimes(2); + expect(mock.mock.calls).toStrictEqual([ + [MULTIPLE_INIT_WARNING], + ["Prefab already initialized."], + ]); + mock.mockRestore(); }); it("warns when called multiple times if enableSSE is set", async () => { @@ -230,8 +244,12 @@ describe("prefab", () => { expect(mock).not.toHaveBeenCalled(); await prefab.init(); - expect(mock).toHaveBeenCalled(); - expect(mock.mock.calls).toStrictEqual([[MULTIPLE_INIT_WARNING]]); + expect(mock).toHaveBeenCalledTimes(2); + expect(mock.mock.calls).toStrictEqual([ + [MULTIPLE_INIT_WARNING], + ["Prefab already initialized."], + ]); + mock.mockRestore(); }); it("does not warn when init is called multiple times if enableSSE and enablePolling are false", async () => { @@ -247,7 +265,9 @@ describe("prefab", () => { expect(mock).not.toHaveBeenCalled(); await prefab.init(); - expect(mock).not.toHaveBeenCalled(); + expect(mock).toHaveBeenCalledTimes(1); + expect(mock.mock.calls).toStrictEqual([["Prefab already initialized."]]); + mock.mockRestore(); }); it("loads remote config so Prefab can provided value", async () => { @@ -1216,4 +1236,196 @@ import { Prefab } from "../prefab"; expect(duration).toBeLessThan(3000); }); }); + + describe("ConfigChangeNotifier integration", () => { + let prefab: Prefab; + // const apiKey = process.env["PREFAB_TEST_API_KEY"] ?? "fallback-api-key-if-not-set"; // apiKey might not be needed if init is fully mocked + + beforeEach(() => { + // No longer async + const apiKeyForTests = + process.env["PREFAB_INTEGRATION_TEST_API_KEY"] ?? + "fallback-api-key-if-not-set"; + prefab = new Prefab({ + ...defaultOptions, + apiKey: apiKeyForTests, + enablePolling: false, + enableSSE: false, + }); + + const configData = { + id: new Long(1), + key: "initial.key.for.notifier.test", + projectId: irrelevantLong, + configType: ConfigType.CONFIG, + rows: [ + { + properties: {}, + values: [{ criteria: [], value: { string: "initial_value" } }], + }, + ], + valueType: Config_ValueType.STRING, + sendToClientSdk: false, + allowableValues: [] as ConfigValue[], + changedBy: undefined, + }; + const minimalConfigForInit: Config = configData; + + const projectEnvIdForTest: ProjectEnvId = irrelevantLong; // projectEnvId is now a Long + + prefab.setConfig([minimalConfigForInit], projectEnvIdForTest, new Map()); + }); + + it("should call a listener when total config ID changes", () => { + const listenerCallback = jest.fn(); + prefab.addConfigChangeListener(listenerCallback); + + const initialResolver = (prefab as any).resolver; + expect(initialResolver).toBeDefined(); + + const newConfig: Config = { + id: new Long(1001), + key: "new.config.key", + projectId: irrelevantLong, + configType: ConfigType.CONFIG, + rows: [ + { + properties: {}, + values: [{ criteria: [], value: { string: "new_value" } }], + }, + ], + valueType: Config_ValueType.STRING, + sendToClientSdk: true, + allowableValues: [], + changedBy: undefined, + }; + + initialResolver.update([newConfig]); + + expect(listenerCallback).toHaveBeenCalledTimes(1); + }); + + it("should NOT call a listener if total config ID does not change", () => { + const listenerCallback = jest.fn(); + const existingConfig: Config = { ...basicConfig, id: new Long(500) }; + (prefab as any)._createOrReconfigureResolver( + [existingConfig], + projectEnvIdUnderTest, + new Map() + ); + prefab.addConfigChangeListener(listenerCallback); + const resolver = (prefab as any).resolver; + + const nonChangingConfig: Config = { + ...basicConfig, + id: new Long(100), + key: "another.key", + rows: basicConfig.rows.map((r) => ({ + ...r, + properties: r.properties ?? {}, + values: r.values.map((v) => ({ ...v, criteria: v.criteria ?? [] })), + })), + valueType: basicConfig.valueType, + }; + resolver.update([nonChangingConfig]); + expect(listenerCallback).not.toHaveBeenCalled(); + + const nonLongIdConfig: Config = { + ...basicConfig, + id: "not-a-long" as any, + key: "yet.another.key", + rows: basicConfig.rows.map((r) => ({ + ...r, + properties: r.properties ?? {}, + values: r.values.map((v) => ({ ...v, criteria: v.criteria ?? [] })), + })), + valueType: basicConfig.valueType, + }; + resolver.update([nonLongIdConfig]); + expect(listenerCallback).not.toHaveBeenCalled(); + }); + + it("unsubscribe should prevent listener from being called", () => { + const listenerCallback = jest.fn(); + const unsubscribe = prefab.addConfigChangeListener(listenerCallback); + + unsubscribe(); + + const resolver = (prefab as any).resolver; + const newConfig: Config = { ...basicConfig, id: new Long(2000) }; + resolver.update([newConfig]); + + expect(listenerCallback).not.toHaveBeenCalled(); + }); + + it("should notify multiple listeners", () => { + const listener1 = jest.fn(); + const listener2 = jest.fn(); + + prefab.addConfigChangeListener(listener1); + prefab.addConfigChangeListener(listener2); + + const resolver = (prefab as any).resolver; + const newConfig: Config = { ...basicConfig, id: new Long(3000) }; + resolver.update([newConfig]); + + expect(listener1).toHaveBeenCalledTimes(1); + expect(listener2).toHaveBeenCalledTimes(1); + }); + + it("listener can safely get updated value from Prefab", () => { + const targetKey = basicConfig.key; + const initialValue = "initial_test_value"; + const updatedValueString = "updated_test_value"; + + const initialConfig: Config = { + ...basicConfig, + id: new Long(100), + rows: [ + { + properties: {}, + values: [{ criteria: [], value: { string: initialValue } }], + }, + ], + valueType: Config_ValueType.STRING, + }; + (prefab as any)._createOrReconfigureResolver( + [initialConfig], + projectEnvIdUnderTest, + new Map() + ); + + expect(prefab.get(targetKey)).toBe(initialValue); + + let valueInListener: GetValue | undefined; + + const listenerCallback = jest.fn(() => { + valueInListener = prefab.get(targetKey); + }); + + prefab.addConfigChangeListener(listenerCallback); + + const updatedConfig: Config = { + ...basicConfig, + id: new Long(200), + rows: [ + { + properties: {}, + values: [{ criteria: [], value: { string: updatedValueString } }], + }, + ], + valueType: Config_ValueType.STRING, + }; + + const resolver = (prefab as any).resolver; + resolver.update([updatedConfig]); + + expect(listenerCallback).toHaveBeenCalledTimes(1); + expect(valueInListener).toBe(updatedValueString); + }); + }); + + describe("init with datafile", () => { + // ... existing code ... + }); }); diff --git a/src/configChangeNotifier.ts b/src/configChangeNotifier.ts new file mode 100644 index 0000000..c5da99d --- /dev/null +++ b/src/configChangeNotifier.ts @@ -0,0 +1,128 @@ +import Long from "long"; +import type { Config } from "./proto"; // Adjust path if necessary +import type { MinimumConfig } from "./resolver"; // Corrected import for MinimumConfig +import { maxLong } from "./maxLong"; // Adjust path if necessary + +// Define an interface for the parts of Resolver that ConfigChangeNotifier will use. +// This helps in decoupling and testing. +export interface ResolverInterface { + keys: () => string[]; + // Assuming MinimumConfig has an optional `id` of type `Long`. + // If MinimumConfig is not directly usable, this could be: raw: (key: string) => { id?: Long } | undefined; + raw: (key: string) => Pick | undefined; +} + +export type GlobalListenerCallback = () => void; + +export class ConfigChangeNotifier { + private readonly listeners: Set = + new Set(); + + private lastTotalId: Long = Long.ZERO; + private resolver: ResolverInterface | undefined; + + /** + * Initializes the notifier with a resolver instance. + * This should be called after the Resolver has been instantiated, + * typically passing this notifier's `handleResolverUpdate` to the Resolver's constructor. + * @param resolver The resolver instance. + */ + public init(resolver: ResolverInterface): void { + // Allow re-initialization: always bind to the provided resolver + // and recalculate the baseline based on its current state. + this.resolver = resolver; + this.lastTotalId = this.calculateCurrentTotalId(); + } + + /** + * This method is designed to be used as the onUpdate callback for the Resolver. + * It's called when the Resolver's configuration data has changed. + * + * It recalculates the total ID of all configurations and notifies listeners + * if this total ID has changed. + * + * The `updatedConfigs` argument is not directly used to calculate the totalId, + * as the notifier will query the resolver for the complete current state + * to ensure accuracy. + */ + public handleResolverUpdate = ( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _updatedConfigs: Array + ): void => { + if (this.resolver == null) { + // This might happen if Resolver calls onUpdate before init is called, + // though ideally init is called right after Resolver construction. + console.warn( + "ConfigChangeNotifier.handleResolverUpdate called before init. Skipping." + ); + return; + } + + const newTotalId = this.calculateCurrentTotalId(); + + if (!newTotalId.equals(this.lastTotalId)) { + this.lastTotalId = newTotalId; + this.notifyGlobalListeners(); + } + }; + + private readonly calculateCurrentTotalId = (): Long => { + if (this.resolver == null) { + // Should not happen if init is called properly. + return Long.ZERO; + } + + const keys = this.resolver.keys(); + const ids: Long[] = []; + + for (const key of keys) { + const config = this.resolver.raw(key); + // Ensure config exists, config.id exists, and config.id is a Long. + // `MinimumConfig` might have `id` as optional or `Long | undefined`. + if (config?.id != null && Long.isLong(config.id)) { + ids.push(config.id); + } + } + + return maxLong(ids); // maxLong correctly returns Long.ZERO for an empty array. + }; + + private readonly notifyGlobalListeners = (): void => { + // Iterate over a copy in case listeners modify the array during iteration + [...this.listeners].forEach((callback) => { + try { + callback(); + } catch (error) { + console.error("Error executing global config change listener:", error); + } + }); + }; + + /** + * Adds a global listener that will be called when the total configuration ID changes. + * @param callback The function to call. + * @returns An unsubscribe function. Call this function to remove the listener. + */ + public addListener = (callback: GlobalListenerCallback): (() => void) => { + this.listeners.add(callback); + return () => { + this.removeListener(callback); + }; + }; + + /** + * Removes a previously added global listener. + * @param callback The exact callback function instance to remove. + */ + public removeListener = (callback: GlobalListenerCallback): void => { + this.listeners.delete(callback); + }; + + /** + * Gets the last known total ID. Useful for debugging or specific checks. + * @returns The last calculated total ID. + */ + public getLastTotalId = (): Long => { + return this.lastTotalId; + }; +} diff --git a/src/prefab.ts b/src/prefab.ts index 3a22bbd..f0d6ffc 100644 --- a/src/prefab.ts +++ b/src/prefab.ts @@ -2,7 +2,7 @@ import crypto from "crypto"; import Long from "long"; import { apiClient, type ApiClient } from "./apiClient"; import { loadConfig } from "./loadConfig"; -import { Resolver, type MinimumConfig } from "./resolver"; +import { Resolver, type MinimumConfig, type ResolverAPI } from "./resolver"; import { Sources } from "./sources"; import type { ContextObj, @@ -36,6 +36,10 @@ import { contextShapes } from "./telemetry/contextShapes"; import { exampleContexts } from "./telemetry/exampleContexts"; import { evaluationSummaries } from "./telemetry/evaluationSummaries"; import { encrypt, generateNewHexKey } from "./encryption"; +import { + ConfigChangeNotifier, + type GlobalListenerCallback, +} from "./configChangeNotifier"; const DEFAULT_POLL_INTERVAL = 60 * 1000; export const PREFAB_DEFAULT_LOG_LEVEL = LogLevel.WARN; @@ -74,7 +78,8 @@ export interface PrefabInterface { }) => boolean; telemetry?: Telemetry; updateIfStalerThan: (durationInMs: number) => Promise | undefined; - withContext: (contexts: Contexts | ContextObj) => Resolver; + withContext: (contexts: Contexts | ContextObj) => ResolverAPI; + addConfigChangeListener: (callback: GlobalListenerCallback) => () => void; } export interface Telemetry { @@ -111,11 +116,11 @@ class Prefab implements PrefabInterface { private readonly namespace?: string; private readonly onNoDefault: "error" | "warn" | "ignore"; private readonly pollInterval: number; - private resolver?: Resolver; + private resolver: Resolver | undefined; private readonly apiClient: ApiClient; private readonly defaultLogLevel: ValidLogLevel; private readonly instanceHash: string; - private readonly onUpdate: ConstructorProps["onUpdate"]; + private readonly onUpdate: (configs: Array) => void; private initCount: number = 0; private lastUpdatedAt: number = 0; private loading: boolean = false; @@ -125,6 +130,7 @@ class Prefab implements PrefabInterface { private running = true; private pollTimeout?: NodeJS.Timeout; private startAtId = Long.fromInt(0); + private readonly configChangeNotifier: ConfigChangeNotifier; constructor({ apiKey, @@ -141,7 +147,7 @@ class Prefab implements PrefabInterface { collectLoggerCounts = true, contextUploadMode = "periodicExample", collectEvaluationSummaries = true, - onUpdate = () => {}, + onUpdate, }: ConstructorProps) { this.apiKey = apiKey; @@ -161,7 +167,7 @@ class Prefab implements PrefabInterface { this.onNoDefault = onNoDefault ?? "error"; this.pollInterval = pollInterval ?? DEFAULT_POLL_INTERVAL; this.instanceHash = crypto.randomUUID(); - this.onUpdate = onUpdate; + this.onUpdate = onUpdate ?? (() => {}); this.globalContext = globalContext; const parsedDefaultLogLevel = parseLevel(defaultLogLevel); @@ -206,6 +212,46 @@ class Prefab implements PrefabInterface { collectEvaluationSummaries ), }; + + this.configChangeNotifier = new ConfigChangeNotifier(); + } + + private _createOrReconfigureResolver( + configs: Config[], + projectEnvId: ProjectEnvId, + defaultContext: Contexts + ): void { + // Create resolver with a temporary no-op onUpdate for its construction phase + const tempResolver = new Resolver( + configs, + projectEnvId, + this.namespace, + this.onNoDefault, + this.updateIfStalerThan.bind(this), + this.telemetry, + undefined, // context object for resolver itself + () => {}, // Temporary onInitialUpdate, so Resolver's constructor update doesn't trigger full handler yet + defaultContext, + this.globalContext + ); + + // Now that tempResolver is constructed and has processed its initial configs, + // initialize the configChangeNotifier with it. This sets the baseline. + this.configChangeNotifier.init(tempResolver); + + // Define the actual combined onUpdate callback + const actualCombinedOnUpdate = ( + updatedConfigs: Array + ): void => { + this.onUpdate(updatedConfigs); // User's onUpdate callback (from Prefab constructor) + this.configChangeNotifier.handleResolverUpdate(updatedConfigs); // Notifier's handler + }; + + // Set the actual onUpdate callback on the resolver + tempResolver.setOnUpdate(actualCombinedOnUpdate); + + // Assign the fully configured resolver to the Prefab instance + this.resolver = tempResolver; } async init({ @@ -220,34 +266,44 @@ class Prefab implements PrefabInterface { console.warn(MULTIPLE_INIT_WARNING); } - const { configs, projectEnvId, startAtId, defaultContext } = - await loadConfig({ - sources: this.sources.configSources, - apiClient: this.apiClient, - datafile: this.datafile, - }); + if (this.resolver !== undefined) { + console.warn("Prefab already initialized."); + return; + } - this.setConfig(configs, projectEnvId, defaultContext); + try { + const { configs, projectEnvId, startAtId, defaultContext } = + await loadConfig({ + sources: this.sources.configSources, + apiClient: this.apiClient, + datafile: this.datafile, + }); - this.loadingComplete(); + this._createOrReconfigureResolver(configs, projectEnvId, defaultContext); - runtimeConfig.forEach(([key, value]) => { - this.set(key, value); - }); + this.loadingComplete(); - if (this.enableSSE) { - this.startSSE(startAtId); - } + runtimeConfig.forEach(([key, value]) => { + this.set(key, value); + }); + + if (this.enableSSE) { + this.startSSE(startAtId); + } + + if (this.enablePolling) { + setTimeout(() => { + this.startPolling(); + }, this.pollInterval); + } - if (this.enablePolling) { setTimeout(() => { - this.startPolling(); - }, this.pollInterval); + TelemetryReporter.start(Object.values(this.telemetry)); + }); + } catch (error) { + console.error("Error during Prefab initialization:", error); + throw error; } - - setTimeout(() => { - TelemetryReporter.start(Object.values(this.telemetry)); - }); } logger( @@ -297,18 +353,7 @@ class Prefab implements PrefabInterface { projectEnvId: ProjectEnvId, defaultContext: Contexts ): void { - this.resolver = new Resolver( - config, - projectEnvId, - this.namespace, - this.onNoDefault, - this.updateIfStalerThan.bind(this), - this.telemetry, - undefined, - this.onUpdate, - defaultContext, - this.globalContext - ); + this._createOrReconfigureResolver(config, projectEnvId, defaultContext); } startSSE(startAtId: Long): void { @@ -358,7 +403,7 @@ class Prefab implements PrefabInterface { return func(this.resolver.cloneWithContext(contexts)); } - withContext(contexts: Contexts | ContextObj): Resolver { + withContext(contexts: Contexts | ContextObj): ResolverAPI { requireResolver(this.resolver); return this.resolver.cloneWithContext(contexts); @@ -441,7 +486,6 @@ class Prefab implements PrefabInterface { set(key: string, value: ConfigValue): void { requireResolver(this.resolver); - this.resolver.set(key, value); } @@ -472,6 +516,10 @@ class Prefab implements PrefabInterface { this.running = false; } + + addConfigChangeListener(callback: GlobalListenerCallback): () => void { + return this.configChangeNotifier.addListener(callback); + } } const encryption = { diff --git a/src/resolver.ts b/src/resolver.ts index 05b9d50..2150ec2 100644 --- a/src/resolver.ts +++ b/src/resolver.ts @@ -9,7 +9,7 @@ import type { OnNoDefault, ProjectEnvId, } from "./types"; -import type { PrefabInterface, Telemetry } from "./prefab"; +import type { Telemetry } from "./prefab"; import { PREFAB_DEFAULT_LOG_LEVEL } from "./prefab"; import { mergeContexts, contextObjToMap } from "./mergeContexts"; @@ -22,6 +22,48 @@ const emptyContexts: Contexts = new Map(); export const NOT_PROVIDED = Symbol("NOT_PROVIDED"); +// Interface for Resolver's public API +export interface ResolverAPI { + id: number; + contexts?: Contexts; + readonly telemetry: Telemetry | undefined; + readonly defaultContext?: Contexts; + updateIfStalerThan: + | ((durationInMs: number) => Promise | undefined) + | undefined; + + cloneWithContext: (contexts: Contexts | ContextObj) => ResolverAPI; + withContext: (contexts: Contexts | ContextObj) => ResolverAPI; + update: ( + configs: Array, + defaultContext?: Contexts + ) => void; + raw: (key: string) => MinimumConfig | undefined; + set: (key: string, value: ConfigValue) => void; + get: ( + key: string, + localContexts?: Contexts | ContextObj, + defaultValue?: GetValue | symbol, + onNoDefault?: OnNoDefault + ) => GetValue; + isFeatureEnabled: (key: string, contexts?: Contexts | ContextObj) => boolean; + keys: () => string[]; + logger: ( + loggerName: string, + defaultLevel: ValidLogLevelName | ValidLogLevel, + contexts?: Contexts | ContextObj + ) => ReturnType; + shouldLog: (args: { + loggerName: string; + desiredLevel: ValidLogLevel | ValidLogLevelName; + defaultLevel?: ValidLogLevel | ValidLogLevelName; + contexts?: Contexts | ContextObj; + }) => boolean; + setOnUpdate: ( + onUpdate: (configs: Array) => void + ) => void; +} + type OptionalKeys = "id" | "projectId" | "changedBy" | "allowableValues"; export type MinimumConfig = { @@ -56,14 +98,15 @@ const mergeDefaultContexts = ( let id = 0; -class Resolver implements PrefabInterface { +class Resolver implements ResolverAPI { + // Implement the new interface private readonly config = new Map(); private readonly projectEnvId: ProjectEnvId; private readonly namespace: string | undefined; private readonly onNoDefault: OnNoDefault; public contexts?: Contexts; readonly telemetry: Telemetry | undefined; - private readonly onUpdate: (configs: Array) => void; + private onUpdate: (configs: Array) => void; private readonly globalContext?: Contexts; public id: number; public readonly defaultContext?: Contexts; @@ -79,7 +122,7 @@ class Resolver implements PrefabInterface { updateIfStalerThan: (durationInMs: number) => Promise | undefined, telemetry?: Telemetry, contexts?: Contexts | ContextObj, - onUpdate?: (configs: Array) => void, + onInitialUpdate?: (configs: Array) => void, defaultContext?: Contexts, globalContext?: Contexts ) { @@ -88,7 +131,7 @@ class Resolver implements PrefabInterface { this.projectEnvId = projectEnvId; this.namespace = namespace; this.onNoDefault = onNoDefault; - this.onUpdate = onUpdate ?? (() => {}); + this.onUpdate = onInitialUpdate ?? (() => {}); this.defaultContext = defaultContext ?? new Map(); this.globalContext = globalContext ?? new Map(); this.contexts = mergeDefaultContexts( @@ -294,6 +337,12 @@ class Resolver implements PrefabInterface { resolver: this, }); } + + public setOnUpdate( + onUpdate: (configs: Array) => void + ): void { + this.onUpdate = onUpdate; + } } export { Resolver }; From 4075003d2e3f8d02a8f9198ece7ff90c4fe9fec1 Mon Sep 17 00:00:00 2001 From: James Kebinger Date: Mon, 12 May 2025 10:17:56 -0500 Subject: [PATCH 2/2] remove extra comments, unused argument --- src/__tests__/configChangeNotifier.test.ts | 14 +++++++------- src/__tests__/prefab.test.ts | 6 ------ src/configChangeNotifier.ts | 22 ++++------------------ src/prefab.ts | 11 ++++------- 4 files changed, 15 insertions(+), 38 deletions(-) diff --git a/src/__tests__/configChangeNotifier.test.ts b/src/__tests__/configChangeNotifier.test.ts index 42a0265..97af77c 100644 --- a/src/__tests__/configChangeNotifier.test.ts +++ b/src/__tests__/configChangeNotifier.test.ts @@ -83,7 +83,7 @@ describe("ConfigChangeNotifier", () => { // Trigger with new totalId = 1 mockResolver.keys.mockReturnValueOnce(["key1"]); // Override for this specific call inside handleResolverUpdate mockResolver.raw.mockReturnValueOnce({ id: Long.fromInt(1) }); - notifier.handleResolverUpdate([]); + notifier.handleResolverUpdate(); expect(listenerCallback).toHaveBeenCalledTimes(1); unsubscribe(); @@ -91,7 +91,7 @@ describe("ConfigChangeNotifier", () => { // If we want to test it changes again and listener is NOT called: mockResolver.keys.mockReturnValueOnce(["key2"]); // Setup for next call mockResolver.raw.mockReturnValueOnce({ id: Long.fromInt(2) }); // Change ID again - notifier.handleResolverUpdate([]); + notifier.handleResolverUpdate(); expect(listenerCallback).toHaveBeenCalledTimes(1); // Still 1, as it was unsubscribed }); @@ -102,7 +102,7 @@ describe("ConfigChangeNotifier", () => { mockResolver.keys.mockReturnValueOnce([]).mockReturnValueOnce(["key1"]); mockResolver.raw.mockReturnValueOnce({ id: Long.fromInt(1) }); notifier.init(mockResolver); - notifier.handleResolverUpdate([]); + notifier.handleResolverUpdate(); expect(listenerCallback).not.toHaveBeenCalled(); }); @@ -120,7 +120,7 @@ describe("ConfigChangeNotifier", () => { mockResolver.keys.mockReturnValue(["key1"]); mockResolver.raw.mockReturnValue({ id: Long.fromInt(5) }); // New totalId = 5 - notifier.handleResolverUpdate([]); + notifier.handleResolverUpdate(); expect(listenerCallback).toHaveBeenCalledTimes(1); expect(notifier.getLastTotalId().toNumber()).toBe(5); @@ -130,7 +130,7 @@ describe("ConfigChangeNotifier", () => { mockResolver.keys.mockReturnValue(["key1"]); mockResolver.raw.mockReturnValue({ id: Long.fromInt(0) }); // Same as initial Long.ZERO - notifier.handleResolverUpdate([]); + notifier.handleResolverUpdate(); expect(listenerCallback).not.toHaveBeenCalled(); expect(notifier.getLastTotalId().equals(Long.ZERO)).toBe(true); @@ -143,7 +143,7 @@ describe("ConfigChangeNotifier", () => { mockResolver.keys.mockReturnValue(["key1"]); mockResolver.raw.mockReturnValue({ id: Long.fromInt(10) }); - notifier.handleResolverUpdate([]); + notifier.handleResolverUpdate(); expect(listenerCallback).toHaveBeenCalledTimes(1); expect(listener2).toHaveBeenCalledTimes(1); @@ -153,7 +153,7 @@ describe("ConfigChangeNotifier", () => { const freshNotifier = new ConfigChangeNotifier(); // Not initialized const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(); - freshNotifier.handleResolverUpdate([]); + freshNotifier.handleResolverUpdate(); expect(consoleWarnSpy).toHaveBeenCalledWith( "ConfigChangeNotifier.handleResolverUpdate called before init. Skipping." diff --git a/src/__tests__/prefab.test.ts b/src/__tests__/prefab.test.ts index 090891c..77ed726 100644 --- a/src/__tests__/prefab.test.ts +++ b/src/__tests__/prefab.test.ts @@ -1239,8 +1239,6 @@ import { Prefab } from "../prefab"; describe("ConfigChangeNotifier integration", () => { let prefab: Prefab; - // const apiKey = process.env["PREFAB_TEST_API_KEY"] ?? "fallback-api-key-if-not-set"; // apiKey might not be needed if init is fully mocked - beforeEach(() => { // No longer async const apiKeyForTests = @@ -1424,8 +1422,4 @@ import { Prefab } from "../prefab"; expect(valueInListener).toBe(updatedValueString); }); }); - - describe("init with datafile", () => { - // ... existing code ... - }); }); diff --git a/src/configChangeNotifier.ts b/src/configChangeNotifier.ts index c5da99d..523dda9 100644 --- a/src/configChangeNotifier.ts +++ b/src/configChangeNotifier.ts @@ -1,14 +1,9 @@ import Long from "long"; -import type { Config } from "./proto"; // Adjust path if necessary -import type { MinimumConfig } from "./resolver"; // Corrected import for MinimumConfig -import { maxLong } from "./maxLong"; // Adjust path if necessary +import type { MinimumConfig } from "./resolver"; +import { maxLong } from "./maxLong"; -// Define an interface for the parts of Resolver that ConfigChangeNotifier will use. -// This helps in decoupling and testing. export interface ResolverInterface { keys: () => string[]; - // Assuming MinimumConfig has an optional `id` of type `Long`. - // If MinimumConfig is not directly usable, this could be: raw: (key: string) => { id?: Long } | undefined; raw: (key: string) => Pick | undefined; } @@ -45,13 +40,8 @@ export class ConfigChangeNotifier { * as the notifier will query the resolver for the complete current state * to ensure accuracy. */ - public handleResolverUpdate = ( - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _updatedConfigs: Array - ): void => { + public handleResolverUpdate = (): void => { if (this.resolver == null) { - // This might happen if Resolver calls onUpdate before init is called, - // though ideally init is called right after Resolver construction. console.warn( "ConfigChangeNotifier.handleResolverUpdate called before init. Skipping." ); @@ -68,7 +58,6 @@ export class ConfigChangeNotifier { private readonly calculateCurrentTotalId = (): Long => { if (this.resolver == null) { - // Should not happen if init is called properly. return Long.ZERO; } @@ -77,18 +66,15 @@ export class ConfigChangeNotifier { for (const key of keys) { const config = this.resolver.raw(key); - // Ensure config exists, config.id exists, and config.id is a Long. - // `MinimumConfig` might have `id` as optional or `Long | undefined`. if (config?.id != null && Long.isLong(config.id)) { ids.push(config.id); } } - return maxLong(ids); // maxLong correctly returns Long.ZERO for an empty array. + return maxLong(ids); }; private readonly notifyGlobalListeners = (): void => { - // Iterate over a copy in case listeners modify the array during iteration [...this.listeners].forEach((callback) => { try { callback(); diff --git a/src/prefab.ts b/src/prefab.ts index f0d6ffc..f546b4b 100644 --- a/src/prefab.ts +++ b/src/prefab.ts @@ -229,25 +229,22 @@ class Prefab implements PrefabInterface { this.onNoDefault, this.updateIfStalerThan.bind(this), this.telemetry, - undefined, // context object for resolver itself - () => {}, // Temporary onInitialUpdate, so Resolver's constructor update doesn't trigger full handler yet + undefined, + () => {}, defaultContext, this.globalContext ); - // Now that tempResolver is constructed and has processed its initial configs, - // initialize the configChangeNotifier with it. This sets the baseline. this.configChangeNotifier.init(tempResolver); // Define the actual combined onUpdate callback const actualCombinedOnUpdate = ( updatedConfigs: Array ): void => { - this.onUpdate(updatedConfigs); // User's onUpdate callback (from Prefab constructor) - this.configChangeNotifier.handleResolverUpdate(updatedConfigs); // Notifier's handler + this.onUpdate(updatedConfigs); + this.configChangeNotifier.handleResolverUpdate(); }; - // Set the actual onUpdate callback on the resolver tempResolver.setOnUpdate(actualCombinedOnUpdate); // Assign the fully configured resolver to the Prefab instance