From 4f6ccb61132e80045c1d998c448742462d7e316e Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Fri, 10 Jan 2025 15:43:54 +0000 Subject: [PATCH 01/34] Add support for feature-specific config payloads This update introduces a new `config` property for features, allowing optional, user-defined configuration payloads. The change includes implementation within SDKs, unit tests, and updates to version numbers. It maintains backward compatibility while enabling new configuration flexibility. --- packages/browser-sdk/package.json | 2 +- packages/browser-sdk/src/client.ts | 49 +++--- .../browser-sdk/src/feature/featureCache.ts | 11 +- packages/browser-sdk/src/feature/features.ts | 8 + packages/browser-sdk/test/features.test.ts | 9 +- packages/browser-sdk/test/mocks/handlers.ts | 18 +- packages/browser-sdk/test/usage.test.ts | 155 +++++++++++------- packages/node-sdk/src/types.ts | 5 + packages/react-sdk/package.json | 4 +- packages/react-sdk/src/index.tsx | 26 ++- packages/react-sdk/test/usage.test.tsx | 23 +++ yarn.lock | 4 +- 12 files changed, 206 insertions(+), 108 deletions(-) diff --git a/packages/browser-sdk/package.json b/packages/browser-sdk/package.json index bf9bcc07..1159884b 100644 --- a/packages/browser-sdk/package.json +++ b/packages/browser-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/browser-sdk", - "version": "2.5.0", + "version": "2.6.0", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 5048a912..a5fc8391 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -94,6 +94,7 @@ const defaultConfig: Config = { export interface Feature { isEnabled: boolean; + config: any; track: () => Promise; requestFeedback: ( options: Omit, @@ -232,7 +233,7 @@ export class BucketClient { * Performs a shallow merge with the existing company context. * Attempting to update the company ID will log a warning and be ignored. * - * @param company + * @param company The company details. */ async updateCompany(company: { [key: string]: string | number | undefined }) { if (company.id && company.id !== this.context.company?.id) { @@ -255,7 +256,7 @@ export class BucketClient { * Performs a shallow merge with the existing company context. * Updates to the company ID will be ignored. * - * @param company + * @param otherContext Additional context. */ async updateOtherContext(otherContext: { [key: string]: string | number | undefined; @@ -273,8 +274,7 @@ export class BucketClient { * * Calling `client.stop()` will remove all listeners added here. * - * @param callback this will be called when the features are updated. - * @param options passed as-is to addEventListener + * @param cb The callback to call when the update completes. */ onFeaturesUpdated(cb: () => void) { return this.featuresClient.onUpdated(cb); @@ -283,8 +283,8 @@ export class BucketClient { /** * Track an event in Bucket. * - * @param eventName The name of the event - * @param attributes Any attributes you want to attach to the event + * @param eventName The name of the event. + * @param attributes Any attributes you want to attach to the event. */ async track(eventName: string, attributes?: Record | null) { if (!this.context.user) { @@ -312,8 +312,8 @@ export class BucketClient { /** * Submit user feedback to Bucket. Must include either `score` or `comment`, or both. * - * @returns - * @param payload + * @param payload The feedback details to submit. + * @returns The server response. */ async feedback(payload: Feedback) { const userId = @@ -407,35 +407,44 @@ export class BucketClient { * Returns a map of enabled features. * Accessing a feature will *not* send a check event * - * @returns Map of features + * @returns Map of features. */ getFeatures(): RawFeatures { return this.featuresClient.getFeatures(); } /** - * Return a feature. Accessing `isEnabled` will automatically send a `check` event. - * @returns A feature + * Return a feature. Accessing `isEnabled` or `config` will automatically send a `check` event. + * @returns A feature. */ getFeature(key: string): Feature { const f = this.getFeatures()[key]; const fClient = this.featuresClient; const value = f?.isEnabled ?? false; + const config = f?.config?.payload; + + function sendCheckEvent() { + fClient + .sendCheckEvent({ + key: key, + version: f?.targetingVersion, + value, + }) + .catch(() => { + // ignore + }); + } return { get isEnabled() { - fClient - .sendCheckEvent({ - key: key, - version: f?.targetingVersion, - value, - }) - .catch(() => { - // ignore - }); + sendCheckEvent(); return value; }, + get config() { + sendCheckEvent(); + return config; + }, track: () => this.track(key), requestFeedback: ( options: Omit, diff --git a/packages/browser-sdk/src/feature/featureCache.ts b/packages/browser-sdk/src/feature/featureCache.ts index 1a66c441..7be35611 100644 --- a/packages/browser-sdk/src/feature/featureCache.ts +++ b/packages/browser-sdk/src/feature/featureCache.ts @@ -22,19 +22,24 @@ export function parseAPIFeaturesResponse( const features: RawFeatures = {}; for (const key in featuresInput) { const feature = featuresInput[key]; + if ( typeof feature.isEnabled !== "boolean" || feature.key !== key || - typeof feature.targetingVersion !== "number" + typeof feature.targetingVersion !== "number" || + (feature.config && typeof feature.config !== "object") ) { return; } + features[key] = { isEnabled: feature.isEnabled, targetingVersion: feature.targetingVersion, key, + config: feature.config, }; } + return features; } @@ -45,8 +50,8 @@ export interface CacheResult { export class FeatureCache { private storage: StorageItem; - private staleTimeMs: number; - private expireTimeMs: number; + private readonly staleTimeMs: number; + private readonly expireTimeMs: number; constructor({ storage, diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index b2d0c2fc..4d4477b5 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -13,6 +13,11 @@ export type RawFeature = { key: string; isEnabled: boolean; targetingVersion?: number; + config?: { + name: string | null; + version: number; + payload: any; + }; }; const FEATURES_UPDATED_EVENT = "features-updated"; @@ -53,7 +58,9 @@ export function validateFeaturesResponse(response: any) { if (typeof response.success !== "boolean" || !isObject(response.features)) { return; } + const features = parseAPIFeaturesResponse(response.features); + if (!features) { return; } @@ -198,6 +205,7 @@ export class FeaturesClient { JSON.stringify(errorBody), ); } + const typeRes = validateFeaturesResponse(await res.json()); if (!typeRes || !typeRes.success) { throw new Error("unable to validate response"); diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index 6a8ae823..19be9812 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -27,8 +27,10 @@ function featuresClientFactory() { const httpClient = new HttpClient("pk", { baseUrl: "https://front.bucket.co", }); + vi.spyOn(httpClient, "get"); vi.spyOn(httpClient, "post"); + return { cache, httpClient, @@ -54,7 +56,7 @@ function featuresClientFactory() { }; } -describe("FeaturesClient unit tests", () => { +describe("FeaturesClient", () => { test("fetches features", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); const featuresClient = newFeaturesClient(); @@ -69,8 +71,9 @@ describe("FeaturesClient unit tests", () => { expect(updated).toBe(true); expect(httpClient.get).toBeCalledTimes(1); - const calls = vi.mocked(httpClient.get).mock.calls.at(0); - const { params, path, timeoutMs } = calls![0]; + + const calls = vi.mocked(httpClient.get).mock.calls.at(0)!; + const { params, path, timeoutMs } = calls[0]; const paramsObj = Object.fromEntries(new URLSearchParams(params)); expect(paramsObj).toEqual({ diff --git a/packages/browser-sdk/test/mocks/handlers.ts b/packages/browser-sdk/test/mocks/handlers.ts index 0c575745..1e34e8c5 100644 --- a/packages/browser-sdk/test/mocks/handlers.ts +++ b/packages/browser-sdk/test/mocks/handlers.ts @@ -9,16 +9,20 @@ export const featureResponse: FeaturesResponse = { success: true, features: { featureA: { isEnabled: true, key: "featureA", targetingVersion: 1 }, + featureB: { + isEnabled: true, + targetingVersion: 11, + key: "featureB", + config: { + version: 12, + name: "gpt3", + payload: { model: "gpt-something", temperature: 0.5 }, + }, + }, }, }; -export const featuresResult: Features = { - featureA: { - isEnabled: true, - key: "featureA", - targetingVersion: 1, - }, -}; +export const featuresResult: Features = featureResponse.features; function checkRequest(request: StrictRequest) { const url = new URL(request.url); diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 905d27f8..64986ce1 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -79,6 +79,7 @@ describe("usage", () => { isEnabled: false, track: expect.any(Function), requestFeedback: expect.any(Function), + config: undefined, }); }); @@ -393,82 +394,114 @@ describe(`sends "check" events `, () => { ).toHaveBeenCalledTimes(0); }); - it(`getFeature() sends check event when accessing "isEnabled"`, async () => { - vi.spyOn(FeaturesClient.prototype, "sendCheckEvent"); - vi.spyOn(HttpClient.prototype, "post"); - - const client = new BucketClient({ - publishableKey: KEY, - user: { id: "uid" }, - company: { id: "cid" }, + describe("getFeature", async () => { + afterEach(() => { + vi.clearAllMocks(); }); - await client.initialize(); - const featureA = client.getFeature("featureA"); + it(`sends check event when accessing "isEnabled"`, async () => { + const sendCheckEventSpy = vi.spyOn( + FeaturesClient.prototype, + "sendCheckEvent", + ); + const postSpy = vi.spyOn(HttpClient.prototype, "post"); + + const client = new BucketClient({ + publishableKey: KEY, + user: { id: "uid" }, + company: { id: "cid" }, + }); + await client.initialize(); - expect( - vi.mocked(FeaturesClient.prototype.sendCheckEvent), - ).toHaveBeenCalledTimes(0); - expect(featureA.isEnabled).toBe(true); + const featureA = client.getFeature("featureA"); - expect( - vi.mocked(FeaturesClient.prototype.sendCheckEvent), - ).toHaveBeenCalledTimes(1); - expect( - vi.mocked(FeaturesClient.prototype.sendCheckEvent), - ).toHaveBeenCalledWith({ - key: "featureA", - value: true, - version: 1, - }); + expect(sendCheckEventSpy).toHaveBeenCalledTimes(0); + expect(featureA.isEnabled).toBe(true); + expect(sendCheckEventSpy).toHaveBeenCalledTimes(1); + expect(sendCheckEventSpy).toHaveBeenCalledWith({ + key: "featureA", + value: true, + version: 1, + }); - expect(vi.mocked(HttpClient.prototype.post)).toHaveBeenCalledWith({ - body: { - action: "check", - evalContext: { - company: { - id: "cid", - }, - other: undefined, - user: { - id: "uid", + expect(postSpy).toHaveBeenCalledWith({ + body: { + action: "check", + evalContext: { + company: { + id: "cid", + }, + other: undefined, + user: { + id: "uid", + }, }, + evalResult: true, + key: "featureA", + targetingVersion: 1, }, - evalResult: true, - key: "featureA", - targetingVersion: 1, - }, - path: "features/events", + path: "features/events", + }); }); - }); - it("sends check event for not-enabled features", async () => { - // disabled features don't appear in the API response - vi.spyOn(FeaturesClient.prototype, "sendCheckEvent"); + it(`sends check event when accessing "config"`, async () => { + const postSpy = vi.spyOn(HttpClient.prototype, "post"); - const client = new BucketClient({ publishableKey: KEY }); - await client.initialize(); + const client = new BucketClient({ + publishableKey: KEY, + user: { id: "uid" }, + }); - const nonExistentFeature = client.getFeature("non-existent"); + await client.initialize(); + const featureB = client.getFeature("featureB"); + expect(featureB.config).toEqual({ + model: "gpt-something", + temperature: 0.5, + }); - expect( - vi.mocked(FeaturesClient.prototype.sendCheckEvent), - ).toHaveBeenCalledTimes(0); - expect(nonExistentFeature.isEnabled).toBe(false); + expect(postSpy).toHaveBeenCalledWith({ + body: { + action: "check", + evalContext: { + other: undefined, + user: { + id: "uid", + }, + }, + evalResult: true, + key: "featureB", + targetingVersion: 11, + }, + path: "features/events", + }); + }); - expect( - vi.mocked(FeaturesClient.prototype.sendCheckEvent), - ).toHaveBeenCalledTimes(1); - expect( - vi.mocked(FeaturesClient.prototype.sendCheckEvent), - ).toHaveBeenCalledWith({ - value: false, - key: "non-existent", - version: undefined, + it("sends check event for not-enabled features", async () => { + // disabled features don't appear in the API response + vi.spyOn(FeaturesClient.prototype, "sendCheckEvent"); + + const client = new BucketClient({ publishableKey: KEY }); + await client.initialize(); + + const nonExistentFeature = client.getFeature("non-existent"); + + expect( + vi.mocked(FeaturesClient.prototype.sendCheckEvent), + ).toHaveBeenCalledTimes(0); + expect(nonExistentFeature.isEnabled).toBe(false); + + expect( + vi.mocked(FeaturesClient.prototype.sendCheckEvent), + ).toHaveBeenCalledTimes(1); + expect( + vi.mocked(FeaturesClient.prototype.sendCheckEvent), + ).toHaveBeenCalledWith({ + value: false, + key: "non-existent", + version: undefined, + }); }); - }); - describe("getFeature", async () => { it("calls client.track with the featureId", async () => { const client = new BucketClient({ publishableKey: KEY }); await client.initialize(); diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index e31879e8..7a6eb586 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -94,6 +94,11 @@ export interface Feature { */ isEnabled: boolean; + /** + * Optional user-defined configuration if the feature is enabled. + */ + config: any; + /** * Track feature usage in Bucket. */ diff --git a/packages/react-sdk/package.json b/packages/react-sdk/package.json index 0d5a0133..20af4cab 100644 --- a/packages/react-sdk/package.json +++ b/packages/react-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/react-sdk", - "version": "2.5.1", + "version": "2.6.0", "license": "MIT", "repository": { "type": "git", @@ -34,7 +34,7 @@ } }, "dependencies": { - "@bucketco/browser-sdk": "2.5.0", + "@bucketco/browser-sdk": "2.6.0", "canonical-json": "^0.0.4" }, "peerDependencies": { diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index 0d7e5adc..33193802 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -198,22 +198,30 @@ export function useFeature(key: FeatureKey) { const feature = features[key]; const enabled = feature?.isEnabled ?? false; + function sendCheckEvent() { + client + ?.sendCheckEvent({ + key, + value: enabled, + version: feature?.targetingVersion, + }) + .catch(() => { + // ignore + }); + } + return { isLoading, track, requestFeedback, get isEnabled() { - client - ?.sendCheckEvent({ - key, - value: enabled, - version: feature?.targetingVersion, - }) - .catch(() => { - // ignore - }); + sendCheckEvent(); return enabled; }, + get config() { + sendCheckEvent(); + return feature?.config?.payload; + }, }; } diff --git a/packages/react-sdk/test/usage.test.tsx b/packages/react-sdk/test/usage.test.tsx index 317a0bcb..c78cb639 100644 --- a/packages/react-sdk/test/usage.test.tsx +++ b/packages/react-sdk/test/usage.test.tsx @@ -81,6 +81,11 @@ const server = setupServer( key: "abc", isEnabled: true, targetingVersion: 1, + config: { + name: "gpt3", + payload: { model: "gpt-something", temperature: 0.5 }, + version: 2, + }, }, def: { key: "def", @@ -242,6 +247,24 @@ describe("useFeature", () => { unmount(); }); + + test("provides the expected values if feature is enabled", async () => { + const { result, unmount } = renderHook(() => useFeature("abc"), { + wrapper: ({ children }) => getProvider({ children }), + }); + + await waitFor(() => { + expect(result.current).toStrictEqual({ + isEnabled: true, + isLoading: false, + config: { model: "gpt-something", temperature: 0.5 }, + track: expect.any(Function), + requestFeedback: expect.any(Function), + }); + }); + + unmount(); + }); }); describe("useTrack", () => { diff --git a/yarn.lock b/yarn.lock index 4d2264ef..3c9764b3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -894,7 +894,7 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:2.5.0, @bucketco/browser-sdk@workspace:packages/browser-sdk": +"@bucketco/browser-sdk@npm:2.6.0, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" dependencies: @@ -1030,7 +1030,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/react-sdk@workspace:packages/react-sdk" dependencies: - "@bucketco/browser-sdk": "npm:2.5.0" + "@bucketco/browser-sdk": "npm:2.6.0" "@bucketco/eslint-config": "workspace:^" "@bucketco/tsconfig": "workspace:^" "@testing-library/react": "npm:^15.0.7" From 7b3a4af7f261266f91b2f2af8ffc8f8f4f9046d0 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 13 Jan 2025 15:21:51 +0000 Subject: [PATCH 02/34] Refactor flag evaluation logic and update SDK version. Simplified flag evaluation by introducing a generic `resolveFeature` method for type consistency and streamlined logic. Added extensive test cases for different flag types and scenarios. Updated `@bucketco/browser-sdk` dependency to version 2.6.0 for compatibility. --- .../openfeature-browser-provider/package.json | 2 +- .../src/index.test.ts | 160 +++++++++++++++++- .../openfeature-browser-provider/src/index.ts | 93 ++++++---- packages/react-sdk/test/usage.test.tsx | 1 + yarn.lock | 14 +- 5 files changed, 217 insertions(+), 53 deletions(-) diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index 6338dd25..0707a706 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -35,7 +35,7 @@ } }, "dependencies": { - "@bucketco/browser-sdk": "2.4.0" + "@bucketco/browser-sdk": "2.6.0" }, "devDependencies": { "@bucketco/eslint-config": "0.0.2", diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index 54b4e700..7a2a6e79 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -90,22 +90,174 @@ describe("BucketBrowserSDKProvider", () => { }); describe("resolveBooleanEvaluation", () => { - it("calls the client correctly for boolean evaluation", async () => { + function mockFeature(enabled: boolean, config: any) { bucketClientMock.getFeature = vi.fn().mockReturnValue({ - isEnabled: true, + isEnabled: enabled, + config: config, }); + bucketClientMock.getFeatures = vi.fn().mockReturnValue({ [testFlagKey]: { - isEnabled: true, + isEnabled: enabled, + config: { + name: "test", + version: 1, + payload: config, + }, targetingVersion: 1, }, }); + } + + it("calls the client correctly when evaluating", async () => { + mockFeature(true, true); await provider.initialize(); - ofClient.getBooleanDetails(testFlagKey, false); + const val = ofClient.getBooleanDetails(testFlagKey, false); + + expect(val).toBeDefined(); + expect(bucketClientMock.getFeatures).toHaveBeenCalled(); expect(bucketClientMock.getFeature).toHaveBeenCalledWith(testFlagKey); }); + + it.each([ + [true, true, false, true, "TARGETING_MATCH"], + [true, false, false, true, "TARGETING_MATCH"], + [true, null, false, true, "TARGETING_MATCH"], + [true, { obj: true }, false, true, "TARGETING_MATCH"], + [true, 15, false, true, "TARGETING_MATCH"], + [false, true, false, false, "DISABLED"], + [false, true, true, true, "DISABLED"], + ])( + "should return the correct result when evaluating boolean %s, %s, %s, %s, %s`", + async (enabled, config, def, expected, reason) => { + mockFeature(enabled, config); + expect(ofClient.getBooleanDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([ + [true, 1, -1, 1, "TARGETING_MATCH"], + [true, null, -2, -2, "DEFAULT"], + [false, 3, -3, -3, "DISABLED"], + [false, 4, -4, -4, "DISABLED"], + ])( + "should return the correct result when evaluating number %s, %s, %s, %s, %s`", + async (enabled, config, def, expected, reason) => { + mockFeature(enabled, config); + expect(ofClient.getNumberDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([["string"], [true], [{}]])( + "should handle type mismatch when evaluating number as %s`", + async (config) => { + mockFeature(true, config); + expect(ofClient.getNumberDetails(testFlagKey, -1)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "TYPE_MISMATCH", + errorMessage: "", + value: -1, + }); + }, + ); + + it.each([ + [true, "1", "-1", "1", "TARGETING_MATCH"], + [true, null, "-2", "-2", "DEFAULT"], + [false, "2", "-3", "-3", "DISABLED"], + [false, "3", "-4", "-4", "DISABLED"], + ])( + "should return the correct result when evaluating string %s, %s, %s, %s, %s`", + async (enabled, config, def, expected, reason) => { + mockFeature(enabled, config); + expect(ofClient.getStringDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([[15], [true], [{}]])( + "should handle type mismatch when evaluating string as %s`", + async (config) => { + mockFeature(true, config); + expect(ofClient.getStringDetails(testFlagKey, "hello")).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "TYPE_MISMATCH", + errorMessage: "", + value: "hello", + }); + }, + ); + + it.each([ + [true, [], [1], [], "TARGETING_MATCH"], + [true, null, [2], [2], "DEFAULT"], + [false, [3], [4], [4], "DISABLED"], + [false, [5], [6], [6], "DISABLED"], + ])( + "should return the correct result when evaluating array %s, %s, %s, %s, %s`", + async (enabled, config, def, expected, reason) => { + mockFeature(enabled, config); + expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([ + [true, {}, { a: 1 }, {}, "TARGETING_MATCH"], + [true, null, { a: 2 }, { a: 2 }, "DEFAULT"], + [false, { a: 3 }, { a: 4 }, { a: 4 }, "DISABLED"], + [false, { a: 5 }, { a: 6 }, { a: 6 }, "DISABLED"], + ])( + "should return the correct result when evaluating object %s, %s, %s, %s, %s`", + async (enabled, config, def, expected, reason) => { + mockFeature(enabled, config); + expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([["string"], [15], [true]])( + "should handle type mismatch when evaluating object as %s`", + async (config) => { + mockFeature(true, config); + expect(ofClient.getObjectDetails(testFlagKey, { obj: true })).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "TYPE_MISMATCH", + errorMessage: "", + value: { obj: true }, + }); + }, + ); }); describe("track", () => { diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index 48894ecb..995ab7ef 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -44,8 +44,8 @@ export class BucketBrowserSDKProvider implements Provider { private _client?: BucketClient; - private _clientOptions: InitOptions; - private _contextTranslator: ContextTranslationFn; + private readonly _clientOptions: InitOptions; + private readonly _contextTranslator: ContextTranslationFn; public events = new OpenFeatureEventEmitter(); @@ -100,66 +100,89 @@ export class BucketBrowserSDKProvider implements Provider { await this.initialize(newContext); } - resolveBooleanEvaluation( + private resolveFeature( flagKey: string, - defaultValue: boolean, - ): ResolutionDetails { - if (!this._client) + defaultValue: T, + ): ResolutionDetails { + const expType = typeof defaultValue; + + if (!this._client) { return { value: defaultValue, reason: StandardResolutionReasons.DEFAULT, errorCode: ErrorCode.PROVIDER_NOT_READY, - } satisfies ResolutionDetails; + errorMessage: "Bucket client not initialized", + } satisfies ResolutionDetails; + } const features = this._client.getFeatures(); if (flagKey in features) { const feature = this._client.getFeature(flagKey); + + if (!feature.isEnabled) { + return { + value: defaultValue, + reason: StandardResolutionReasons.DISABLED, + }; + } + + if (expType === "boolean") { + return { + value: true as T, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + } + + if (feature.config === null || feature.config === undefined) { + return { + value: defaultValue, + reason: StandardResolutionReasons.DEFAULT, + }; + } + + if (typeof feature.config !== expType) { + return { + value: defaultValue, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.TYPE_MISMATCH, + errorMessage: `Expected ${expType} but got ${typeof feature.config}`, + }; + } + return { - value: feature.isEnabled, + value: feature.config as T, reason: StandardResolutionReasons.TARGETING_MATCH, - } satisfies ResolutionDetails; + }; } return { value: defaultValue, reason: StandardResolutionReasons.DEFAULT, - } satisfies ResolutionDetails; + errorCode: ErrorCode.FLAG_NOT_FOUND, + errorMessage: `Flag ${flagKey} not found`, + }; } - resolveNumberEvaluation( - _flagKey: string, - defaultValue: number, - ): ResolutionDetails { - return { - value: defaultValue, - errorCode: ErrorCode.TYPE_MISMATCH, - reason: StandardResolutionReasons.ERROR, - errorMessage: "Bucket doesn't support number flags", - }; + resolveBooleanEvaluation(flagKey: string, defaultValue: boolean) { + return this.resolveFeature(flagKey, defaultValue); + } + + resolveNumberEvaluation(flagKey: string, defaultValue: number) { + return this.resolveFeature(flagKey, defaultValue); } resolveObjectEvaluation( - _flagKey: string, + flagKey: string, defaultValue: T, - ): ResolutionDetails { - return { - value: defaultValue, - errorCode: ErrorCode.TYPE_MISMATCH, - reason: StandardResolutionReasons.ERROR, - errorMessage: "Bucket doesn't support object flags", - }; + ) { + return this.resolveFeature(flagKey, defaultValue); } resolveStringEvaluation( - _flagKey: string, + flagKey: string, defaultValue: string, ): ResolutionDetails { - return { - value: defaultValue, - errorCode: ErrorCode.TYPE_MISMATCH, - reason: StandardResolutionReasons.ERROR, - errorMessage: "Bucket doesn't support string flags", - }; + return this.resolveFeature(flagKey, defaultValue); } track( diff --git a/packages/react-sdk/test/usage.test.tsx b/packages/react-sdk/test/usage.test.tsx index c78cb639..535f0075 100644 --- a/packages/react-sdk/test/usage.test.tsx +++ b/packages/react-sdk/test/usage.test.tsx @@ -238,6 +238,7 @@ describe("useFeature", () => { await waitFor(() => { expect(result.current).toStrictEqual({ + config: undefined, isEnabled: false, isLoading: false, track: expect.any(Function), diff --git a/yarn.lock b/yarn.lock index 3c9764b3..ee25e38d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,18 +882,6 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:2.4.0": - version: 2.4.0 - resolution: "@bucketco/browser-sdk@npm:2.4.0" - dependencies: - "@floating-ui/dom": "npm:^1.6.8" - canonical-json: "npm:^0.0.4" - js-cookie: "npm:^3.0.5" - preact: "npm:^10.22.1" - checksum: 10c0/b33a9fdafa4a857ac4f815fe69b602b37527a37d54270abd479b754da998d030f5a70b738c662ab57fa4f6374b8e1fbd052feb8bdbd8b78367086dcedc5a5432 - languageName: node - linkType: hard - "@bucketco/browser-sdk@npm:2.6.0, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" @@ -984,7 +972,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/openfeature-browser-provider@workspace:packages/openfeature-browser-provider" dependencies: - "@bucketco/browser-sdk": "npm:2.4.0" + "@bucketco/browser-sdk": "npm:2.6.0" "@bucketco/eslint-config": "npm:0.0.2" "@bucketco/tsconfig": "npm:0.0.2" "@openfeature/core": "npm:1.5.0" From 70f23932709b0959af08bd10ab24d5c6201a9717 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 13 Jan 2025 16:34:23 +0000 Subject: [PATCH 03/34] Add feature configuration support and enhance fallback features Introduced support for feature-specific configurations, enabling dynamic payloads for features based on user-defined rules. Enhanced fallback features to accept both string arrays and object records, providing more flexibility for feature initialization. Updated documentation and tests to reflect the new configuration capabilities and backwards-compatible fallback behavior. --- packages/browser-sdk/README.md | 47 +++++++++++++++++-- packages/browser-sdk/src/client.ts | 12 ++--- packages/browser-sdk/src/feature/features.ts | 38 +++++++++++---- packages/browser-sdk/test/features.test.ts | 26 +++++++++- packages/node-sdk/src/client.ts | 3 ++ packages/node-sdk/test/client.test.ts | 4 ++ .../openfeature-browser-provider/README.md | 5 +- packages/react-sdk/README.md | 10 ++-- packages/react-sdk/src/index.tsx | 23 +++++---- 9 files changed, 133 insertions(+), 35 deletions(-) diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index 5b908263..d7489140 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -27,19 +27,24 @@ const bucketClient = new BucketClient({ publishableKey, user, company }); await bucketClient.initialize(); -const { isEnabled, track, requestFeedback } = bucketClient.getFeature("huddle"); +const { isEnabled, config, track, requestFeedback } = bucketClient.getFeature("huddle"); if (isEnabled) { - // show feature. When retrieving `isEnabled` the client automatically + // Show feature. When retrieving `isEnabled` the client automatically // sends a "check" event for the "huddle" feature which is shown in the // Bucket UI. // On usage, call `track` to let Bucket know that a user interacted with the feature track(); + // The `config` is a user-supplied value in Bucket that can be dynamically evaluated + // with respect to the current context. Here, it is assumed that one could either get + // a config value that maches the context or not. + const question = config?.question ?? "Tell us what you think of Huddles" + // Use `requestFeedback` to create "Send feedback" buttons easily for specific // features. This is not related to `track` and you can call them individually. - requestFeedback({ title: "Tell us what you think of Huddles" }); + requestFeedback({ title: question }); } // `track` just calls `bucketClient.track()` to send an event using the same feature key @@ -127,6 +132,7 @@ To retrieve features along with their targeting information, use `getFeature(key const huddle = bucketClient.getFeature("huddle"); // { // isEnabled: true, +// config: any, // track: () => Promise // requestFeedback: (options: RequestFeedbackData) => void // } @@ -140,6 +146,7 @@ const features = bucketClient.getFeatures(); // huddle: { // isEnabled: true, // targetingVersion: 42, +// config: ... // } // } ``` @@ -148,7 +155,39 @@ const features = bucketClient.getFeatures(); by down-stream clients, like the React SDK. Note that accessing `isEnabled` on the object returned by `getFeatures` does not automatically -generate a `check` event, contrary to the `isEnabled` property on the object return from `getFeature`. +generate a `check` event, contrary to the `isEnabled` property on the object returned by `getFeature`. + +### Feature toggles + +Similar to `isEnabled`, each feature has a `config` property. This configuration is set by the user within Bucket. It is +similar to the way access is controlled, using matching rules. Each config-bound rule is given a configuration payload +(a JSON value) that is returned to the SDKs if the requested context matches that specific rule. It is possible to have +multiple rules with different configuration payloads. Whichever rule matches the context, provides the configuration +payload. + + +The config is accessible through the same methods as the `isEnabled` property: + +```ts +const features = bucketClient.getFeatures(); +// { +// huddle: { +// isEnabled: true, +// targetingVersion: 42, +// config?: { +// name: "gpt-3.5", +// version: 2, +// payload: { maxTokens: 10000, model: "gpt-3.5-beta1" } +// } +// } +// } +``` +The `name` is given by the user in Bucket for each configuration variant, and `version` is maintained by Bucket similar +to `targetingVersion`. The `payload` is the actual JSON value supplied by the user and serves as context-based +configuration. + +Just as `isEnabled`, accessing `config` on the object returned by `getFeatures` does not automatically +generate a `check` event, contrary to the `config` property on the object returned by `getFeature`. ### Tracking feature usage diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index a5fc8391..148727f4 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -102,16 +102,16 @@ export interface Feature { } export class BucketClient { - private publishableKey: string; - private context: BucketContext; + private readonly publishableKey: string; + private readonly context: BucketContext; private config: Config; private requestFeedbackOptions: Partial; - private logger: Logger; - private httpClient: HttpClient; + private readonly logger: Logger; + private readonly httpClient: HttpClient; - private autoFeedback: AutoFeedback | undefined; + private readonly autoFeedback: AutoFeedback | undefined; private autoFeedbackInit: Promise | undefined; - private featuresClient: FeaturesClient; + private readonly featuresClient: FeaturesClient; constructor(opts: InitOptions) { this.publishableKey = opts.publishableKey; diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 4d4477b5..f3dbf0cb 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -25,7 +25,7 @@ const FEATURES_UPDATED_EVENT = "features-updated"; export type RawFeatures = Record; export type FeaturesOptions = { - fallbackFeatures?: string[]; + fallbackFeatures?: string[] | Record; timeoutMs?: number; staleWhileRevalidate?: boolean; staleTimeMs?: number; @@ -33,13 +33,13 @@ export type FeaturesOptions = { }; type Config = { - fallbackFeatures: string[]; + fallbackFeatures: Record; timeoutMs: number; staleWhileRevalidate: boolean; }; export const DEFAULT_FEATURES_CONFIG: Config = { - fallbackFeatures: [], + fallbackFeatures: {}, timeoutMs: 5000, staleWhileRevalidate: false, }; @@ -133,6 +133,20 @@ export class FeaturesClient { staleTimeMs: options?.staleTimeMs ?? 0, expireTimeMs: options?.expireTimeMs ?? FEATURES_EXPIRE_MS, }); + + if (Array.isArray(options?.fallbackFeatures)) { + options = { + ...options, + fallbackFeatures: options.fallbackFeatures.reduce( + (acc, key) => { + acc[key] = null; + return acc; + }, + {} as Record, + ), + }; + } + this.config = { ...DEFAULT_FEATURES_CONFIG, ...options }; this.rateLimiter = options?.rateLimiter ?? @@ -312,12 +326,16 @@ export class FeaturesClient { } // fetch failed, nothing cached => return fallbacks - return this.config.fallbackFeatures.reduce((acc, key) => { - acc[key] = { - key, - isEnabled: true, - }; - return acc; - }, {} as RawFeatures); + return Object.entries(this.config.fallbackFeatures).reduce( + (acc, [key, config]) => { + acc[key] = { + key, + isEnabled: true, + config, + }; + return acc; + }, + {} as RawFeatures, + ); } } diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index 19be9812..2b99fa99 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -115,7 +115,7 @@ describe("FeaturesClient", () => { expect(timeoutMs).toEqual(5000); }); - test("return fallback features on failure", async () => { + test("return fallback features on failure (string list)", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); vi.mocked(httpClient.get).mockRejectedValue( @@ -124,10 +124,32 @@ describe("FeaturesClient", () => { const featuresClient = newFeaturesClient({ fallbackFeatures: ["huddle"], }); + + await featuresClient.initialize(); + expect(featuresClient.getFeatures()).toStrictEqual({ + huddle: { + isEnabled: true, + config: null, + key: "huddle", + }, + }); + }); + + test("return fallback features on failure (record)", async () => { + const { newFeaturesClient, httpClient } = featuresClientFactory(); + + vi.mocked(httpClient.get).mockRejectedValue( + new Error("Failed to fetch features"), + ); + const featuresClient = newFeaturesClient({ + fallbackFeatures: { huddle: { name: "john" } }, + }); + await featuresClient.initialize(); - expect(featuresClient.getFeatures()).toEqual({ + expect(featuresClient.getFeatures()).toStrictEqual({ huddle: { isEnabled: true, + config: { name: "john" }, key: "huddle", }, }); diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index ec1393e7..8aef9270 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -925,6 +925,9 @@ export class BucketClient { return isEnabled; }, + get config() { + return undefined; + }, key, track: async () => { if (typeof options.user?.id === "undefined") { diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index ed6fac69..201d4879 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1698,11 +1698,13 @@ describe("BucketClient", () => { key: "feature1", isEnabled: true, track: expect.any(Function), + config: undefined, }, feature2: { key: "feature2", isEnabled: false, track: expect.any(Function), + config: undefined, }, }); @@ -1720,11 +1722,13 @@ describe("BucketClient", () => { key: "feature1", isEnabled: false, track: expect.any(Function), + config: undefined, }, feature2: { key: "feature2", isEnabled: true, track: expect.any(Function), + config: undefined, }, }); }); diff --git a/packages/openfeature-browser-provider/README.md b/packages/openfeature-browser-provider/README.md index 4a15758b..3725534a 100644 --- a/packages/openfeature-browser-provider/README.md +++ b/packages/openfeature-browser-provider/README.md @@ -36,9 +36,10 @@ const client = OpenFeature.getClient(); // use client const boolValue = client.getBooleanValue("huddles", false); -``` -Bucket only supports boolean values. +// use more complex, config-enabled functionality. +const feedbackConfig = client.getObjectValue("ask-feedback", { question: "How are you enjoying this feature?" }); +``` Initializing the Bucket Browser Provider will also initialize [automatic feedback surveys](https://github.com/bucketco/bucket-javascript-sdk/tree/main/packages/browser-sdk#qualitative-feedback). diff --git a/packages/react-sdk/README.md b/packages/react-sdk/README.md index 30e5d203..b48f82bc 100644 --- a/packages/react-sdk/README.md +++ b/packages/react-sdk/README.md @@ -29,6 +29,10 @@ declare module "@bucketco/react-sdk" { interface Features { huddle: boolean; recordVideo: boolean; + questionnaire?: { + showAll: boolean, + time: 600000, + } } } ``` @@ -92,7 +96,7 @@ Returns the state of a given features for the current context. import { useFeature } from "@bucketco/react-sdk"; function StartHuddleButton() { - const { isLoading, isEnabled, track, requestFeedback } = useFeature("huddle"); + const { isLoading, isEnabled, config, track, requestFeedback } = useFeature("huddle"); if (isLoading) { return ; @@ -108,7 +112,7 @@ function StartHuddleButton() { ; + * return ; * } * } * ``` */ -export function useFeature(key: FeatureKey) { +export function useFeature(key: TKey) { const { features: { features, isLoading }, client, @@ -220,7 +227,7 @@ export function useFeature(key: FeatureKey) { }, get config() { sendCheckEvent(); - return feature?.config?.payload; + return feature?.config?.payload as FeatureConfig; }, }; } From d2e9de6e6cdab1b8452e88ab3e842e7be5b2d9ed Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 13 Jan 2025 16:46:30 +0000 Subject: [PATCH 04/34] Bump version to 0.4.0 Update package version to 0.4.0 to reflect changes and improvements made in the OpenFeature browser provider. This helps signify a new iteration with potential new features or fixes. --- packages/openfeature-browser-provider/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index 0707a706..69f0f53f 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/openfeature-browser-provider", - "version": "0.3.1", + "version": "0.4.0", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { From 340fc9c57faa485caa497ddd9df4a276f07a7a3b Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 22 Jan 2025 12:38:20 +0000 Subject: [PATCH 05/34] feat(browser-sdk): add format script, improve README formatting, and enhance feature handling - Added a new "format" script to package.json for consistent code formatting. - Improved formatting in README.md files across browser-sdk, react-sdk, and openfeature-browser-provider for better readability. - Updated feature handling in features.ts and mocks/handlers.ts to ensure proper type usage and maintainability. - Adjusted test cases to reflect changes in feature access patterns. These changes enhance the developer experience and maintain code quality. --- package.json | 1 + packages/browser-sdk/README.md | 13 ++++++----- packages/browser-sdk/src/feature/features.ts | 22 ++++++++++-------- packages/browser-sdk/test/client.test.ts | 2 +- packages/browser-sdk/test/mocks/handlers.ts | 23 ++++++++++--------- .../openfeature-browser-provider/README.md | 4 +++- packages/react-sdk/README.md | 9 ++++---- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/package.json b/package.json index 2becdc8f..ff3107ee 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "build": "lerna run build --stream", "test:ci": "lerna run test:ci --stream", "test": "lerna run test --stream", + "format": "lerna run format --stream", "prettier": "lerna run prettier --stream", "lint": "lerna run lint --stream", "lint:ci": "lerna run lint:ci --stream", diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index d8ecdf36..65932a9e 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -27,7 +27,8 @@ const bucketClient = new BucketClient({ publishableKey, user, company }); await bucketClient.initialize(); -const { isEnabled, config, track, requestFeedback } = bucketClient.getFeature("huddle"); +const { isEnabled, config, track, requestFeedback } = + bucketClient.getFeature("huddle"); if (isEnabled) { // Show feature. When retrieving `isEnabled` the client automatically @@ -39,9 +40,9 @@ if (isEnabled) { // The `config` is a user-supplied value in Bucket that can be dynamically evaluated // with respect to the current context. Here, it is assumed that one could either get - // a config value that maches the context or not. - const question = config?.question ?? "Tell us what you think of Huddles" - + // a config value that matches the context or not. + const question = config?.question ?? "Tell us what you think of Huddles"; + // Use `requestFeedback` to create "Send feedback" buttons easily for specific // features. This is not related to `track` and you can call them individually. requestFeedback({ title: question }); @@ -165,7 +166,6 @@ similar to the way access is controlled, using matching rules. Each config-bound multiple rules with different configuration payloads. Whichever rule matches the context, provides the configuration payload. - The config is accessible through the same methods as the `isEnabled` property: ```ts @@ -176,12 +176,13 @@ const features = bucketClient.getFeatures(); // targetingVersion: 42, // config?: { // name: "gpt-3.5", -// version: 2, +// targetingVersion: 2, // payload: { maxTokens: 10000, model: "gpt-3.5-beta1" } // } // } // } ``` + The `name` is given by the user in Bucket for each configuration variant, and `version` is maintained by Bucket similar to `targetingVersion`. The `payload` is the actual JSON value supplied by the user and serves as context-based configuration. diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 240d0271..d567364c 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -48,6 +48,7 @@ export type FetchedFeature = { const FEATURES_UPDATED_EVENT = "features-updated"; export type FetchedFeatures = Record; + // todo: on next major, come up with a better name for this type. Maybe `LocalFeature`. export type RawFeature = FetchedFeature & { /** @@ -55,6 +56,7 @@ export type RawFeature = FetchedFeature & { */ isEnabledOverride: boolean | null; }; + export type RawFeatures = Record; export type FeaturesOptions = { @@ -456,15 +458,17 @@ export class FeaturesClient { } // fetch failed, nothing cached => return fallbacks - - return this.config.fallbackFeatures.reduce((acc, [key, config]) => { - acc[key] = { - key, - isEnabled: true, - config, - }; - return acc; - }, {} as FetchedFeatures); + return Object.entries(this.config.fallbackFeatures).reduce( + (acc, [key, config]) => { + acc[key] = { + key, + isEnabled: true, + config, + }; + return acc; + }, + {} as FetchedFeatures, + ); } setFeatureOverride(key: string, isEnabled: boolean | null) { diff --git a/packages/browser-sdk/test/client.test.ts b/packages/browser-sdk/test/client.test.ts index b6bd728a..76cd1c24 100644 --- a/packages/browser-sdk/test/client.test.ts +++ b/packages/browser-sdk/test/client.test.ts @@ -68,7 +68,7 @@ describe("BucketClient", () => { describe("getFeature", () => { it("takes overrides into account", async () => { await client.initialize(); - expect(featuresResult.featureA.isEnabled).toBe(true); + expect(featuresResult["featureA"].isEnabled).toBe(true); expect(client.getFeature("featureA").isEnabled).toBe(true); client.setFeatureOverride("featureA", false); expect(client.getFeature("featureA").isEnabled).toBe(false); diff --git a/packages/browser-sdk/test/mocks/handlers.ts b/packages/browser-sdk/test/mocks/handlers.ts index 9b5d9686..06d03cc3 100644 --- a/packages/browser-sdk/test/mocks/handlers.ts +++ b/packages/browser-sdk/test/mocks/handlers.ts @@ -1,7 +1,6 @@ import { DefaultBodyType, http, HttpResponse, StrictRequest } from "msw"; -import { Features } from "../../../node-sdk/src/types"; -import { FeaturesResponse } from "../../src/feature/features"; +import { FeaturesResponse, RawFeatures } from "../../src/feature/features"; export const testChannel = "testChannel"; @@ -22,15 +21,17 @@ export const featureResponse: FeaturesResponse = { }, }; -export const featuresResult: Features = Object.entries( - featureResponse.features, -).reduce((acc, [key, feature]) => { - acc[key] = { - ...feature, - isEnabledOverride: null, - }; - return acc; -}, {} as Features); +export const featuresResult = Object.entries(featureResponse.features).reduce( + (acc, [key, feature]) => { + acc[key] = { + ...feature!, + key: key, + isEnabledOverride: null, + }; + return acc; + }, + {} as RawFeatures, +); function checkRequest(request: StrictRequest) { const url = new URL(request.url); diff --git a/packages/openfeature-browser-provider/README.md b/packages/openfeature-browser-provider/README.md index 3725534a..cdb60e1f 100644 --- a/packages/openfeature-browser-provider/README.md +++ b/packages/openfeature-browser-provider/README.md @@ -38,7 +38,9 @@ const client = OpenFeature.getClient(); const boolValue = client.getBooleanValue("huddles", false); // use more complex, config-enabled functionality. -const feedbackConfig = client.getObjectValue("ask-feedback", { question: "How are you enjoying this feature?" }); +const feedbackConfig = client.getObjectValue("ask-feedback", { + question: "How are you enjoying this feature?", +}); ``` Initializing the Bucket Browser Provider will diff --git a/packages/react-sdk/README.md b/packages/react-sdk/README.md index b48f82bc..006004c4 100644 --- a/packages/react-sdk/README.md +++ b/packages/react-sdk/README.md @@ -30,9 +30,9 @@ declare module "@bucketco/react-sdk" { huddle: boolean; recordVideo: boolean; questionnaire?: { - showAll: boolean, - time: 600000, - } + showAll: boolean; + time: 600000; + }; } } ``` @@ -96,7 +96,8 @@ Returns the state of a given features for the current context. import { useFeature } from "@bucketco/react-sdk"; function StartHuddleButton() { - const { isLoading, isEnabled, config, track, requestFeedback } = useFeature("huddle"); + const { isLoading, isEnabled, config, track, requestFeedback } = + useFeature("huddle"); if (isLoading) { return ; From a6c66782b09de4a02037c5281ae893febf11c112 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 22 Jan 2025 14:54:51 +0000 Subject: [PATCH 06/34] feat(browser-sdk): update feature configuration handling and improve documentation - Renamed "Feature toggles" section to "Remote config" in README.md for clarity. - Introduced a new type, `FeatureDynamicConfig`, to better represent dynamic feature configurations in client.ts. - Updated the `Feature` interface to use `FeatureDynamicConfig` instead of a generic `any` type for the `config` property. - Adjusted the handling of feature configurations in the `BucketClient` class to utilize a default `missingConfig`. - Enhanced the `FetchedFeature` type in features.ts to reflect the new configuration structure. - Updated test cases to align with the new configuration model, ensuring accurate feature representation. These changes enhance type safety and improve the overall developer experience when working with feature configurations. --- packages/browser-sdk/README.md | 2 +- packages/browser-sdk/src/client.ts | 45 +++++++++++++++++--- packages/browser-sdk/src/feature/features.ts | 11 +++-- packages/browser-sdk/test/features.test.ts | 15 ++++--- packages/browser-sdk/test/mocks/handlers.ts | 29 ++++++++++++- packages/browser-sdk/test/usage.test.ts | 12 ++++-- 6 files changed, 94 insertions(+), 20 deletions(-) diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index 65932a9e..3804ca97 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -158,7 +158,7 @@ by down-stream clients, like the React SDK. Note that accessing `isEnabled` on the object returned by `getFeatures` does not automatically generate a `check` event, contrary to the `isEnabled` property on the object returned by `getFeature`. -### Feature toggles +### Remote config Similar to `isEnabled`, each feature has a `config` property. This configuration is set by the user within Bucket. It is similar to the way access is controlled, using matching rules. Each config-bound rule is given a configuration payload diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 07d7ce96..8ade560e 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -174,25 +174,54 @@ const defaultConfig: Config = { enableTracking: true, }; +/** + * A dynamic configuration value for a feature. + */ +export type FeatureDynamicConfig = + | { + /** + * The key of the matched configuration value. + */ + key: string; + /** + * The version of the matched configuration value. + */ + version: number; + + /** + * The user-supplied data. + */ + payload: any; + } + | { key: undefined; version: undefined; payload: undefined }; + +/** + * A feature. + */ export interface Feature { /** - * Result of feature flag evaluation + * Result of feature flag evaluation. */ isEnabled: boolean; /* - * Optional user-defined configuration + * Optional user-defined configuration. */ - config: any; + config: FeatureDynamicConfig; /** - * Function to send analytics events for this feature + * Function to send analytics events for this feature. */ track: () => Promise; + + /** + * Function to request feedback for this feature. + */ requestFeedback: ( options: Omit, ) => void; } + /** * BucketClient lets you interact with the Bucket API. * @@ -209,6 +238,7 @@ export class BucketClient { private readonly featuresClient: FeaturesClient; public readonly logger: Logger; + /** * Create a new BucketClient instance. */ @@ -514,6 +544,11 @@ export class BucketClient { return this.featuresClient.getFeatures(); } + private missingConfig: FeatureDynamicConfig = { + key: undefined, + version: undefined, + payload: undefined, + }; /** * Return a feature. Accessing `isEnabled` or `config` will automatically send a `check` event. * @returns A feature. @@ -523,7 +558,7 @@ export class BucketClient { const fClient = this.featuresClient; const value = f?.isEnabledOverride ?? f?.isEnabled ?? false; - const config = f?.config?.payload; + const config = f?.config ?? this.missingConfig; function sendCheckEvent() { fClient diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index d567364c..45952b40 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -9,6 +9,9 @@ import { parseAPIFeaturesResponse, } from "./featureCache"; +/** + * A feature fetched from the server. + */ export type FetchedFeature = { /** * Feature key @@ -26,15 +29,15 @@ export type FetchedFeature = { targetingVersion?: number; /** - * Optional user-defined configuration. + * Optional user-defined dynamic configuration. */ config?: { /** - * The name of the matched configuration variant. + * The key of the matched configuration value. */ - name: string | null; + key: string; /** - * The version of the matched configuration variant. + * The version of the matched configuration value. */ version: number; diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index d757240a..cf2dd7ce 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -144,14 +144,17 @@ describe("FeaturesClient", () => { new Error("Failed to fetch features"), ); const featuresClient = newFeaturesClient({ - fallbackFeatures: { huddle: { name: "john" } }, + fallbackFeatures: { + huddle: { key: "john", version: 1, payload: { something: "else" } }, + }, }); await featuresClient.initialize(); expect(featuresClient.getFeatures()).toStrictEqual({ huddle: { isEnabled: true, - config: { name: "john" }, + config: { key: "john", version: 1, payload: { something: "else" } }, + key: "huddle", isEnabledOverride: null, }, }); @@ -335,12 +338,12 @@ describe("FeaturesClient", () => { updated = true; }); - expect(client.getFeatures().featureB).toBeUndefined(); + expect(client.getFeatures().featureC).toBeUndefined(); - client.setFeatureOverride("featureB", true); + client.setFeatureOverride("featureC", true); expect(updated).toBe(true); - expect(client.getFeatures().featureB.isEnabled).toBe(false); - expect(client.getFeatures().featureB.isEnabledOverride).toBe(true); + expect(client.getFeatures().featureC.isEnabled).toBe(false); + expect(client.getFeatures().featureC.isEnabledOverride).toBe(true); }); }); diff --git a/packages/browser-sdk/test/mocks/handlers.ts b/packages/browser-sdk/test/mocks/handlers.ts index 06d03cc3..b05879e7 100644 --- a/packages/browser-sdk/test/mocks/handlers.ts +++ b/packages/browser-sdk/test/mocks/handlers.ts @@ -14,7 +14,7 @@ export const featureResponse: FeaturesResponse = { key: "featureB", config: { version: 12, - name: "gpt3", + key: "gpt3", payload: { model: "gpt-something", temperature: 0.5 }, }, }, @@ -116,6 +116,18 @@ export const handlers = [ success: true, }); }), + http.post("https://front.bucket.co/features/events", async ({ request }) => { + if (!checkRequest(request)) return invalidReqResponse; + const data = await request.json(); + + if (typeof data !== "object" || !data || !data["userId"]) { + return new HttpResponse(null, { status: 400 }); + } + + return HttpResponse.json({ + success: true, + }); + }), http.post("https://front.bucket.co/feedback", async ({ request }) => { if (!checkRequest(request)) return invalidReqResponse; const data = await request.json(); @@ -146,4 +158,19 @@ export const handlers = [ if (!checkRequest(request)) return invalidReqResponse; return HttpResponse.json({ success: true, keyName: "keyName" }); }), + http.post( + "https://livemessaging.bucket.co/keys/keyName/requestToken", + async ({ request }) => { + const data = await request.json(); + if (typeof data !== "object") { + return new HttpResponse(null, { status: 400 }); + } + + return HttpResponse.json({ + success: true, + token: "token", + expires: 1234567890, + }); + }, + ), ]; diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 64986ce1..5c712cd9 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -79,7 +79,7 @@ describe("usage", () => { isEnabled: false, track: expect.any(Function), requestFeedback: expect.any(Function), - config: undefined, + config: { key: undefined, version: undefined, payload: undefined }, }); }); @@ -455,8 +455,12 @@ describe(`sends "check" events `, () => { await client.initialize(); const featureB = client.getFeature("featureB"); expect(featureB.config).toEqual({ - model: "gpt-something", - temperature: 0.5, + key: "gpt3", + version: 12, + payload: { + model: "gpt-something", + temperature: 0.5, + }, }); expect(postSpy).toHaveBeenCalledWith({ @@ -511,6 +515,7 @@ describe(`sends "check" events `, () => { isEnabled: false, track: expect.any(Function), requestFeedback: expect.any(Function), + config: { key: undefined, version: undefined, payload: undefined }, }); vi.spyOn(client, "track"); @@ -529,6 +534,7 @@ describe(`sends "check" events `, () => { isEnabled: false, track: expect.any(Function), requestFeedback: expect.any(Function), + config: { key: undefined, version: undefined, payload: undefined }, }); vi.spyOn(client, "requestFeedback"); From f5f250ed44e2c96540376865aaf1170835fa0983 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 22 Jan 2025 15:07:58 +0000 Subject: [PATCH 07/34] feat(react-sdk): enhance feature handling in useFeature hook - Introduced a new EMPTY_FEATURE_CONFIG constant to provide a default configuration for features. - Updated the useFeature hook to return a more structured Feature type, including isEnabled, isLoading, and config properties. - Modified requestFeedback function to use a more specific RequestFeedbackOptions type. - Adjusted test cases to validate the new feature configuration structure and ensure correct default values are returned. These changes improve type safety and enhance the developer experience when working with feature flags. --- packages/react-sdk/src/index.tsx | 35 ++++++++++++++++++++++---- packages/react-sdk/test/usage.test.tsx | 11 +++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index 8821d1f0..b7a1378d 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -170,6 +170,31 @@ export function BucketProvider({ ); } +const EMPTY_FEATURE_CONFIG = { + key: undefined, + version: undefined, + payload: undefined, +}; + +type RequestFeedbackOptions = Omit< + RequestFeedbackData, + "featureKey" | "featureId" +>; + +type Feature = { + isEnabled: boolean; + isLoading: boolean; + config: + | { + key: string; + version: number; + payload: FeatureConfig; + } + | typeof EMPTY_FEATURE_CONFIG; + track: () => void; + requestFeedback: (opts: RequestFeedbackOptions) => void; +}; + /** * Returns the state of a given feature for the current context, e.g. * @@ -182,21 +207,21 @@ export function BucketProvider({ * } * ``` */ -export function useFeature(key: TKey) { +export function useFeature(key: TKey): Feature { const { features: { features, isLoading }, client, } = useContext(ProviderContext); const track = () => client?.track(key); - const requestFeedback = ( - opts: Omit, - ) => client?.requestFeedback({ ...opts, featureKey: key }); + const requestFeedback = (opts: RequestFeedbackOptions) => + client?.requestFeedback({ ...opts, featureKey: key }); if (isLoading) { return { isLoading, isEnabled: false, + config: EMPTY_FEATURE_CONFIG, track, requestFeedback, }; @@ -227,7 +252,7 @@ export function useFeature(key: TKey) { }, get config() { sendCheckEvent(); - return feature?.config?.payload as FeatureConfig; + return feature?.config ?? EMPTY_FEATURE_CONFIG; }, }; } diff --git a/packages/react-sdk/test/usage.test.tsx b/packages/react-sdk/test/usage.test.tsx index 535f0075..0c456f16 100644 --- a/packages/react-sdk/test/usage.test.tsx +++ b/packages/react-sdk/test/usage.test.tsx @@ -82,7 +82,7 @@ const server = setupServer( isEnabled: true, targetingVersion: 1, config: { - name: "gpt3", + key: "gpt3", payload: { model: "gpt-something", temperature: 0.5 }, version: 2, }, @@ -224,6 +224,7 @@ describe("useFeature", () => { expect(result.current).toStrictEqual({ isEnabled: false, isLoading: true, + config: { key: undefined, version: undefined, payload: undefined }, track: expect.any(Function), requestFeedback: expect.any(Function), }); @@ -238,7 +239,7 @@ describe("useFeature", () => { await waitFor(() => { expect(result.current).toStrictEqual({ - config: undefined, + config: { key: undefined, version: undefined, payload: undefined }, isEnabled: false, isLoading: false, track: expect.any(Function), @@ -258,7 +259,11 @@ describe("useFeature", () => { expect(result.current).toStrictEqual({ isEnabled: true, isLoading: false, - config: { model: "gpt-something", temperature: 0.5 }, + config: { + key: "gpt3", + version: 2, + payload: { model: "gpt-something", temperature: 0.5 }, + }, track: expect.any(Function), requestFeedback: expect.any(Function), }); From 66f661ad2975ed43a3bbdd703923695751a322c9 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 22 Jan 2025 16:33:09 +0000 Subject: [PATCH 08/34] feat(browser-sdk, react-sdk, openfeature-browser-sdk): enhance feature configuration handling and improve documentation - Updated feature configuration structure across browser-sdk and react-sdk to use `targetingVersion` and `value` instead of `version` and `payload`. - Refactored related types and interfaces to improve type safety and clarity in feature handling. - Enhanced README documentation to reflect changes in feature configuration and usage examples. - Adjusted test cases to validate the new configuration structure and ensure accurate feature representation. These changes improve the developer experience and maintainability of the SDKs. --- packages/browser-sdk/README.md | 32 ++++--- packages/browser-sdk/src/client.ts | 10 +-- .../browser-sdk/src/feature/featureCache.ts | 6 +- packages/browser-sdk/src/feature/features.ts | 58 ++++++------- packages/browser-sdk/test/features.test.ts | 17 ++-- packages/browser-sdk/test/mocks/handlers.ts | 19 ++++- packages/browser-sdk/test/usage.test.ts | 4 +- .../openfeature-browser-provider/README.md | 2 +- .../src/index.test.ts | 85 ++++++++++--------- .../openfeature-browser-provider/src/index.ts | 23 +++-- packages/react-sdk/README.md | 11 ++- packages/react-sdk/src/index.tsx | 8 +- packages/react-sdk/test/usage.test.tsx | 12 ++- 13 files changed, 165 insertions(+), 122 deletions(-) diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index 3804ca97..f334f745 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -27,8 +27,12 @@ const bucketClient = new BucketClient({ publishableKey, user, company }); await bucketClient.initialize(); -const { isEnabled, config, track, requestFeedback } = - bucketClient.getFeature("huddle"); +const { + isEnabled, + config: { value: question }, + track, + requestFeedback, +} = bucketClient.getFeature("huddle"); if (isEnabled) { // Show feature. When retrieving `isEnabled` the client automatically @@ -38,10 +42,10 @@ if (isEnabled) { // On usage, call `track` to let Bucket know that a user interacted with the feature track(); - // The `config` is a user-supplied value in Bucket that can be dynamically evaluated + // The `value` is a user-supplied value in Bucket that can be dynamically evaluated // with respect to the current context. Here, it is assumed that one could either get - // a config value that matches the context or not. - const question = config?.question ?? "Tell us what you think of Huddles"; + // a config value that matches the context or use a default. + const question = value?.question ?? "Tell us what you think of Huddles"; // Use `requestFeedback` to create "Send feedback" buttons easily for specific // features. This is not related to `track` and you can call them individually. @@ -133,7 +137,7 @@ To retrieve features along with their targeting information, use `getFeature(key const huddle = bucketClient.getFeature("huddle"); // { // isEnabled: true, -// config: any, +// config: { key: "zoom", targetingVersion: 2, value: { ... } }, // track: () => Promise // requestFeedback: (options: RequestFeedbackData) => void // } @@ -161,10 +165,10 @@ generate a `check` event, contrary to the `isEnabled` property on the object ret ### Remote config Similar to `isEnabled`, each feature has a `config` property. This configuration is set by the user within Bucket. It is -similar to the way access is controlled, using matching rules. Each config-bound rule is given a configuration payload -(a JSON value) that is returned to the SDKs if the requested context matches that specific rule. It is possible to have -multiple rules with different configuration payloads. Whichever rule matches the context, provides the configuration -payload. +similar to the way access is managed -- using rules. Each config-bound rule is given a configuration value +(a JSON value) that is returned to the SDK if the requested context matches. It is possible to have +multiple rules with different configuration values. Whichever rule matches the context, provides the configuration +value. The config is accessible through the same methods as the `isEnabled` property: @@ -175,16 +179,16 @@ const features = bucketClient.getFeatures(); // isEnabled: true, // targetingVersion: 42, // config?: { -// name: "gpt-3.5", +// key: "gpt-3.5", // targetingVersion: 2, -// payload: { maxTokens: 10000, model: "gpt-3.5-beta1" } +// value: { maxTokens: 10000, model: "gpt-3.5-beta1" } // } // } // } ``` -The `name` is given by the user in Bucket for each configuration variant, and `version` is maintained by Bucket similar -to `targetingVersion`. The `payload` is the actual JSON value supplied by the user and serves as context-based +The `key` is given by the user in Bucket for each configuration value, and `targetingVersion` is maintained by Bucket similar +to access `targetingVersion`. The `value` is the actual JSON value supplied by the user and serves as context-based configuration. Just as `isEnabled`, accessing `config` on the object returned by `getFeatures` does not automatically diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 8ade560e..fa5de08e 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -186,14 +186,14 @@ export type FeatureDynamicConfig = /** * The version of the matched configuration value. */ - version: number; + targetingVersion?: number; /** * The user-supplied data. */ - payload: any; + value: any; } - | { key: undefined; version: undefined; payload: undefined }; + | { key: undefined; targetingVersion: undefined; value: undefined }; /** * A feature. @@ -546,8 +546,8 @@ export class BucketClient { private missingConfig: FeatureDynamicConfig = { key: undefined, - version: undefined, - payload: undefined, + targetingVersion: undefined, + value: undefined, }; /** * Return a feature. Accessing `isEnabled` or `config` will automatically send a `check` event. diff --git a/packages/browser-sdk/src/feature/featureCache.ts b/packages/browser-sdk/src/feature/featureCache.ts index b4cb8ed8..2bbd695b 100644 --- a/packages/browser-sdk/src/feature/featureCache.ts +++ b/packages/browser-sdk/src/feature/featureCache.ts @@ -36,7 +36,11 @@ export function parseAPIFeaturesResponse( isEnabled: feature.isEnabled, targetingVersion: feature.targetingVersion, key, - config: feature.config, + config: feature.config && { + key: feature.config.key, + targetingVersion: feature.config.version, + value: feature.config.payload, + }, }; } diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 45952b40..99df2532 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -39,12 +39,12 @@ export type FetchedFeature = { /** * The version of the matched configuration value. */ - version: number; + targetingVersion?: number; /** * The user-supplied data. */ - payload: any; + value: any; }; }; @@ -62,6 +62,11 @@ export type RawFeature = FetchedFeature & { export type RawFeatures = Record; +export type FallbackFeatureConfig = { + key: string; + value: any; +} | null; + export type FeaturesOptions = { /** * Feature keys for which `isEnabled` should fallback to true @@ -69,7 +74,7 @@ export type FeaturesOptions = { * is supplied instead of array, the values of each key represent the * configuration values and `isEnabled` is assume `true`. */ - fallbackFeatures?: string[] | Record; + fallbackFeatures?: string[] | Record; /** * Timeout in milliseconds @@ -86,7 +91,7 @@ export type FeaturesOptions = { }; type Config = { - fallbackFeatures: Record; + fallbackFeatures: Record; timeoutMs: number; staleWhileRevalidate: boolean; }; @@ -97,19 +102,6 @@ export const DEFAULT_FEATURES_CONFIG: Config = { staleWhileRevalidate: false, }; -// Deep merge two objects. -export type FeaturesResponse = { - /** - * `true` if call was successful - */ - success: boolean; - - /** - * List of enabled features - */ - features: FetchedFeatures; -}; - export function validateFeaturesResponse(response: any) { if (!isObject(response)) { return; @@ -238,20 +230,22 @@ export class FeaturesClient { expireTimeMs: options?.expireTimeMs ?? FEATURES_EXPIRE_MS, }); + let fallbackFeatures: Record; + if (Array.isArray(options?.fallbackFeatures)) { - options = { - ...options, - fallbackFeatures: options.fallbackFeatures.reduce( - (acc, key) => { - acc[key] = null; - return acc; - }, - {} as Record, - ), - }; + fallbackFeatures = options.fallbackFeatures.reduce( + (acc, key) => { + acc[key] = null; + return acc; + }, + {} as Record, + ); + } else { + fallbackFeatures = options?.fallbackFeatures ?? {}; } - this.config = { ...DEFAULT_FEATURES_CONFIG, ...options }; + this.config = { ...DEFAULT_FEATURES_CONFIG, ...options, fallbackFeatures }; + this.rateLimiter = options?.rateLimiter ?? new RateLimiter(FEATURE_EVENTS_PER_MIN, this.logger); @@ -420,6 +414,7 @@ export class FeaturesClient { private async maybeFetchFeatures(): Promise { const cacheKey = this.fetchParams().toString(); const cachedItem = this.cache.get(cacheKey); + console.log(cacheKey); if (cachedItem) { if (!cachedItem.stale) return cachedItem.features; @@ -466,7 +461,12 @@ export class FeaturesClient { acc[key] = { key, isEnabled: true, - config, + config: config + ? { + key: config.key, + value: config.value, + } + : undefined, }; return acc; }, diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index cf2dd7ce..b50ba8c9 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -122,6 +122,7 @@ describe("FeaturesClient", () => { vi.mocked(httpClient.get).mockRejectedValue( new Error("Failed to fetch features"), ); + const featuresClient = newFeaturesClient({ fallbackFeatures: ["huddle"], }); @@ -130,7 +131,7 @@ describe("FeaturesClient", () => { expect(featuresClient.getFeatures()).toStrictEqual({ huddle: { isEnabled: true, - config: null, + config: undefined, key: "huddle", isEnabledOverride: null, }, @@ -145,7 +146,10 @@ describe("FeaturesClient", () => { ); const featuresClient = newFeaturesClient({ fallbackFeatures: { - huddle: { key: "john", version: 1, payload: { something: "else" } }, + huddle: { + key: "john", + value: { something: "else" }, + }, }, }); @@ -153,23 +157,24 @@ describe("FeaturesClient", () => { expect(featuresClient.getFeatures()).toStrictEqual({ huddle: { isEnabled: true, - config: { key: "john", version: 1, payload: { something: "else" } }, + config: { key: "john", value: { something: "else" } }, key: "huddle", isEnabledOverride: null, }, }); }); - test("caches response", async () => { + test.only("caches response", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); - const featuresClient = newFeaturesClient(); - await featuresClient.initialize(); + const featuresClient1 = newFeaturesClient(); + await featuresClient1.initialize(); expect(httpClient.get).toBeCalledTimes(1); const featuresClient2 = newFeaturesClient(); await featuresClient2.initialize(); + const features = featuresClient2.getFeatures(); expect(features).toEqual(featuresResult); diff --git a/packages/browser-sdk/test/mocks/handlers.ts b/packages/browser-sdk/test/mocks/handlers.ts index b05879e7..1f1a90d2 100644 --- a/packages/browser-sdk/test/mocks/handlers.ts +++ b/packages/browser-sdk/test/mocks/handlers.ts @@ -1,13 +1,18 @@ import { DefaultBodyType, http, HttpResponse, StrictRequest } from "msw"; -import { FeaturesResponse, RawFeatures } from "../../src/feature/features"; +import { RawFeatures } from "../../src/feature/features"; export const testChannel = "testChannel"; -export const featureResponse: FeaturesResponse = { +export const featureResponse = { success: true, features: { - featureA: { isEnabled: true, key: "featureA", targetingVersion: 1 }, + featureA: { + isEnabled: true, + key: "featureA", + targetingVersion: 1, + config: undefined, + }, featureB: { isEnabled: true, targetingVersion: 11, @@ -25,7 +30,13 @@ export const featuresResult = Object.entries(featureResponse.features).reduce( (acc, [key, feature]) => { acc[key] = { ...feature!, - key: key, + config: feature.config + ? { + key: feature.config.key, + targetingVersion: feature.config.version, + value: feature.config.payload, + } + : undefined, isEnabledOverride: null, }; return acc; diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 5c712cd9..e9201d04 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -456,8 +456,8 @@ describe(`sends "check" events `, () => { const featureB = client.getFeature("featureB"); expect(featureB.config).toEqual({ key: "gpt3", - version: 12, - payload: { + targetingVersion: 12, + value: { model: "gpt-something", temperature: 0.5, }, diff --git a/packages/openfeature-browser-provider/README.md b/packages/openfeature-browser-provider/README.md index cdb60e1f..41143a28 100644 --- a/packages/openfeature-browser-provider/README.md +++ b/packages/openfeature-browser-provider/README.md @@ -37,7 +37,7 @@ const client = OpenFeature.getClient(); // use client const boolValue = client.getBooleanValue("huddles", false); -// use more complex, config-enabled functionality. +// use more complex, dynamic config-enabled functionality. const feedbackConfig = client.getObjectValue("ask-feedback", { question: "How are you enjoying this feature?", }); diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index 7a2a6e79..286baff4 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -90,32 +90,50 @@ describe("BucketBrowserSDKProvider", () => { }); describe("resolveBooleanEvaluation", () => { - function mockFeature(enabled: boolean, config: any) { + function mockFeature( + enabled: boolean, + configKey: string | undefined, + configValue: any | undefined, + ) { + const config = + configKey !== undefined + ? { + key: configKey, + targetingVersion: 1, + value: configValue, + } + : { + key: undefined, + targetingVersion: undefined, + value: undefined, + }; + bucketClientMock.getFeature = vi.fn().mockReturnValue({ isEnabled: enabled, - config: config, + config, }); bucketClientMock.getFeatures = vi.fn().mockReturnValue({ [testFlagKey]: { isEnabled: enabled, - config: { - name: "test", - version: 1, - payload: config, - }, + config, targetingVersion: 1, }, }); } it("calls the client correctly when evaluating", async () => { - mockFeature(true, true); + mockFeature(true, "key", true); await provider.initialize(); const val = ofClient.getBooleanDetails(testFlagKey, false); - expect(val).toBeDefined(); + expect(val).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "TARGETING_MATCH", + value: true, + }); expect(bucketClientMock.getFeatures).toHaveBeenCalled(); expect(bucketClientMock.getFeature).toHaveBeenCalledWith(testFlagKey); @@ -131,8 +149,8 @@ describe("BucketBrowserSDKProvider", () => { [false, true, true, true, "DISABLED"], ])( "should return the correct result when evaluating boolean %s, %s, %s, %s, %s`", - async (enabled, config, def, expected, reason) => { - mockFeature(enabled, config); + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, "key", value); expect(ofClient.getBooleanDetails(testFlagKey, def)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -149,8 +167,8 @@ describe("BucketBrowserSDKProvider", () => { [false, 4, -4, -4, "DISABLED"], ])( "should return the correct result when evaluating number %s, %s, %s, %s, %s`", - async (enabled, config, def, expected, reason) => { - mockFeature(enabled, config); + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, value ? "key" : undefined, value); expect(ofClient.getNumberDetails(testFlagKey, def)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -162,8 +180,8 @@ describe("BucketBrowserSDKProvider", () => { it.each([["string"], [true], [{}]])( "should handle type mismatch when evaluating number as %s`", - async (config) => { - mockFeature(true, config); + async (value) => { + mockFeature(true, "key", value); expect(ofClient.getNumberDetails(testFlagKey, -1)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -176,14 +194,12 @@ describe("BucketBrowserSDKProvider", () => { ); it.each([ - [true, "1", "-1", "1", "TARGETING_MATCH"], - [true, null, "-2", "-2", "DEFAULT"], - [false, "2", "-3", "-3", "DISABLED"], - [false, "3", "-4", "-4", "DISABLED"], + [true, { anything: 1 }, "default", "key", "TARGETING_MATCH"], + [false, 1337, "default", "default", "DISABLED"], ])( "should return the correct result when evaluating string %s, %s, %s, %s, %s`", - async (enabled, config, def, expected, reason) => { - mockFeature(enabled, config); + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, "key", value); expect(ofClient.getStringDetails(testFlagKey, def)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -193,21 +209,6 @@ describe("BucketBrowserSDKProvider", () => { }, ); - it.each([[15], [true], [{}]])( - "should handle type mismatch when evaluating string as %s`", - async (config) => { - mockFeature(true, config); - expect(ofClient.getStringDetails(testFlagKey, "hello")).toEqual({ - flagKey: "a-key", - flagMetadata: {}, - reason: "ERROR", - errorCode: "TYPE_MISMATCH", - errorMessage: "", - value: "hello", - }); - }, - ); - it.each([ [true, [], [1], [], "TARGETING_MATCH"], [true, null, [2], [2], "DEFAULT"], @@ -215,8 +216,8 @@ describe("BucketBrowserSDKProvider", () => { [false, [5], [6], [6], "DISABLED"], ])( "should return the correct result when evaluating array %s, %s, %s, %s, %s`", - async (enabled, config, def, expected, reason) => { - mockFeature(enabled, config); + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, value ? "key" : undefined, value); expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -233,8 +234,8 @@ describe("BucketBrowserSDKProvider", () => { [false, { a: 5 }, { a: 6 }, { a: 6 }, "DISABLED"], ])( "should return the correct result when evaluating object %s, %s, %s, %s, %s`", - async (enabled, config, def, expected, reason) => { - mockFeature(enabled, config); + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, value ? "key" : undefined, value); expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -246,8 +247,8 @@ describe("BucketBrowserSDKProvider", () => { it.each([["string"], [15], [true]])( "should handle type mismatch when evaluating object as %s`", - async (config) => { - mockFeature(true, config); + async (value) => { + mockFeature(true, "key", value); expect(ofClient.getObjectDetails(testFlagKey, { obj: true })).toEqual({ flagKey: "a-key", flagMetadata: {}, diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index 995ab7ef..f9d95a85 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -133,24 +133,31 @@ export class BucketBrowserSDKProvider implements Provider { }; } - if (feature.config === null || feature.config === undefined) { + if (!feature.config.key) { return { value: defaultValue, reason: StandardResolutionReasons.DEFAULT, }; } - if (typeof feature.config !== expType) { + if (expType === "string") { + return { + value: feature.config.key, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + } + + if (typeof feature.config.value !== expType) { return { value: defaultValue, reason: StandardResolutionReasons.ERROR, errorCode: ErrorCode.TYPE_MISMATCH, - errorMessage: `Expected ${expType} but got ${typeof feature.config}`, + errorMessage: `Expected ${expType} but got ${typeof feature.config.value}`, }; } return { - value: feature.config as T, + value: feature.config.value as T, reason: StandardResolutionReasons.TARGETING_MATCH, }; } @@ -194,8 +201,10 @@ export class BucketBrowserSDKProvider implements Provider { this._clientOptions.logger?.error("client not initialized"); } - this._client?.track(trackingEventName, trackingEventDetails).catch((e) => { - this._clientOptions.logger?.error("error tracking event", e); - }); + this._client + ?.track(trackingEventName, trackingEventDetails) + .catch((e: any) => { + this._clientOptions.logger?.error("error tracking event", e); + }); } } diff --git a/packages/react-sdk/README.md b/packages/react-sdk/README.md index 006004c4..2169d0bd 100644 --- a/packages/react-sdk/README.md +++ b/packages/react-sdk/README.md @@ -96,8 +96,13 @@ Returns the state of a given features for the current context. import { useFeature } from "@bucketco/react-sdk"; function StartHuddleButton() { - const { isLoading, isEnabled, config, track, requestFeedback } = - useFeature("huddle"); + const { + isLoading, + isEnabled, + config: { key, value }, + track, + requestFeedback, + } = useFeature("huddle"); if (isLoading) { return ; @@ -113,7 +118,7 @@ function StartHuddleButton() { ; - * } + * return ; * } * ``` */ @@ -221,7 +218,7 @@ export function useFeature(key: TKey): Feature { return { isLoading, isEnabled: false, - config: EMPTY_FEATURE_CONFIG, + config: {} as EmptyFeatureConfig, track, requestFeedback, }; @@ -252,7 +249,7 @@ export function useFeature(key: TKey): Feature { }, get config() { sendCheckEvent(); - return feature?.config ?? EMPTY_FEATURE_CONFIG; + return feature?.config ?? ({} as EmptyFeatureConfig); }, }; } diff --git a/packages/react-sdk/test/usage.test.tsx b/packages/react-sdk/test/usage.test.tsx index f435747b..a6c0013d 100644 --- a/packages/react-sdk/test/usage.test.tsx +++ b/packages/react-sdk/test/usage.test.tsx @@ -224,7 +224,7 @@ describe("useFeature", () => { expect(result.current).toStrictEqual({ isEnabled: false, isLoading: true, - config: { key: undefined, targetingVersion: undefined, value: undefined }, + config: {}, track: expect.any(Function), requestFeedback: expect.any(Function), }); @@ -239,11 +239,7 @@ describe("useFeature", () => { await waitFor(() => { expect(result.current).toStrictEqual({ - config: { - key: undefined, - targetingVersion: undefined, - value: undefined, - }, + config: {}, isEnabled: false, isLoading: false, track: expect.any(Function), diff --git a/yarn.lock b/yarn.lock index cc264e6f..812c4c4b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,7 +882,19 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:2.6.0, @bucketco/browser-sdk@workspace:packages/browser-sdk": +"@bucketco/browser-sdk@npm:2.4.0": + version: 2.4.0 + resolution: "@bucketco/browser-sdk@npm:2.4.0" + dependencies: + "@floating-ui/dom": "npm:^1.6.8" + canonical-json: "npm:^0.0.4" + js-cookie: "npm:^3.0.5" + preact: "npm:^10.22.1" + checksum: 10c0/b33a9fdafa4a857ac4f815fe69b602b37527a37d54270abd479b754da998d030f5a70b738c662ab57fa4f6374b8e1fbd052feb8bdbd8b78367086dcedc5a5432 + languageName: node + linkType: hard + +"@bucketco/browser-sdk@npm:2.5.1, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" dependencies: @@ -972,7 +984,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/openfeature-browser-provider@workspace:packages/openfeature-browser-provider" dependencies: - "@bucketco/browser-sdk": "npm:2.6.0" + "@bucketco/browser-sdk": "npm:2.4.0" "@bucketco/eslint-config": "npm:0.0.2" "@bucketco/tsconfig": "npm:0.0.2" "@openfeature/core": "npm:1.5.0" @@ -1018,7 +1030,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/react-sdk@workspace:packages/react-sdk" dependencies: - "@bucketco/browser-sdk": "npm:2.6.0" + "@bucketco/browser-sdk": "npm:2.5.1" "@bucketco/eslint-config": "workspace:^" "@bucketco/tsconfig": "workspace:^" "@testing-library/react": "npm:^15.0.7" From f1383e6fe75bc84f67a57f0a49a64c9e914528ee Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Fri, 24 Jan 2025 10:43:56 +0000 Subject: [PATCH 11/34] fix(browser-sdk): update README to reflect changes in feature configuration - Changed the reference from `value` to `payload` in the README documentation to accurately describe the updated feature configuration structure. - Ensured that the example usage aligns with the latest implementation for better clarity and understanding. These updates improve the documentation and help developers understand the new configuration handling. --- packages/browser-sdk/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index e3c57fd0..4a3ed79c 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -29,7 +29,7 @@ await bucketClient.initialize(); const { isEnabled, - config: { value: question }, + config: { payload: question }, track, requestFeedback, } = bucketClient.getFeature("huddle"); @@ -44,7 +44,7 @@ if (isEnabled) { // The `payload` is a user-supplied JSON in Bucket that is dynamically picked // out depending on the user/company. - const question = value?.question ?? "Tell us what you think of Huddles"; + const question = payload?.question ?? "Tell us what you think of Huddles"; // Use `requestFeedback` to create "Send feedback" buttons easily for specific // features. This is not related to `track` and you can call them individually. From cd276f42b91438230a32bf7abcbb9f101fb4ac31 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Fri, 24 Jan 2025 10:51:51 +0000 Subject: [PATCH 12/34] feat(openfeature-browser-provider): update browser-sdk dependency and enhance feature evaluation - Updated the dependency on `@bucketco/browser-sdk` from version 2.4.0 to 2.5.1 in both `yarn.lock` and `package.json`. - Refactored feature evaluation methods to improve type handling and support for various data types (boolean, string, number, object). - Enhanced README documentation with new examples for dynamic configuration usage. - Improved test coverage for feature evaluation scenarios, ensuring accurate results across different data types. These changes enhance the functionality and robustness of the openfeature-browser-provider, improving the developer experience and maintainability. --- .../openfeature-browser-provider/README.md | 7 +- .../openfeature-browser-provider/package.json | 2 +- .../src/index.test.ts | 157 +++++++++++++++++- .../openfeature-browser-provider/src/index.ts | 108 +++++++----- yarn.lock | 14 +- 5 files changed, 229 insertions(+), 59 deletions(-) diff --git a/packages/openfeature-browser-provider/README.md b/packages/openfeature-browser-provider/README.md index 4a15758b..41143a28 100644 --- a/packages/openfeature-browser-provider/README.md +++ b/packages/openfeature-browser-provider/README.md @@ -36,9 +36,12 @@ const client = OpenFeature.getClient(); // use client const boolValue = client.getBooleanValue("huddles", false); -``` -Bucket only supports boolean values. +// use more complex, dynamic config-enabled functionality. +const feedbackConfig = client.getObjectValue("ask-feedback", { + question: "How are you enjoying this feature?", +}); +``` Initializing the Bucket Browser Provider will also initialize [automatic feedback surveys](https://github.com/bucketco/bucket-javascript-sdk/tree/main/packages/browser-sdk#qualitative-feedback). diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index 6338dd25..e0ee878f 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -35,7 +35,7 @@ } }, "dependencies": { - "@bucketco/browser-sdk": "2.4.0" + "@bucketco/browser-sdk": "2.5.1" }, "devDependencies": { "@bucketco/eslint-config": "0.0.2", diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index 54b4e700..c8418a9d 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -90,22 +90,169 @@ describe("BucketBrowserSDKProvider", () => { }); describe("resolveBooleanEvaluation", () => { - it("calls the client correctly for boolean evaluation", async () => { + function mockFeature( + enabled: boolean, + configKey: string | undefined, + configPayload: any | undefined, + ) { + const config = { + key: configKey, + payload: configPayload, + }; + bucketClientMock.getFeature = vi.fn().mockReturnValue({ - isEnabled: true, + isEnabled: enabled, + config, }); + bucketClientMock.getFeatures = vi.fn().mockReturnValue({ [testFlagKey]: { - isEnabled: true, - targetingVersion: 1, + isEnabled: enabled, + config: { + key: "key", + payload: configPayload, + }, }, }); + } + + it("calls the client correctly when evaluating", async () => { + mockFeature(true, "key", true); await provider.initialize(); - ofClient.getBooleanDetails(testFlagKey, false); + const val = ofClient.getBooleanDetails(testFlagKey, false); + + expect(val).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "TARGETING_MATCH", + value: true, + }); + expect(bucketClientMock.getFeatures).toHaveBeenCalled(); expect(bucketClientMock.getFeature).toHaveBeenCalledWith(testFlagKey); }); + + it.each([ + [true, true, false, true, "TARGETING_MATCH"], + [true, false, false, true, "TARGETING_MATCH"], + [true, null, false, true, "TARGETING_MATCH"], + [true, { obj: true }, false, true, "TARGETING_MATCH"], + [true, 15, false, true, "TARGETING_MATCH"], + [false, true, false, false, "DISABLED"], + [false, true, true, true, "DISABLED"], + ])( + "should return the correct result when evaluating boolean %s, %s, %s, %s, %s`", + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, "key", value); + expect(ofClient.getBooleanDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([ + [true, 1, -1, 1, "TARGETING_MATCH"], + [true, null, -2, -2, "DEFAULT"], + [false, 3, -3, -3, "DISABLED"], + [false, 4, -4, -4, "DISABLED"], + ])( + "should return the correct result when evaluating number %s, %s, %s, %s, %s`", + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, value ? "key" : undefined, value); + expect(ofClient.getNumberDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([["string"], [true], [{}]])( + "should handle type mismatch when evaluating number as %s`", + async (value) => { + mockFeature(true, "key", value); + expect(ofClient.getNumberDetails(testFlagKey, -1)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "TYPE_MISMATCH", + errorMessage: "", + value: -1, + }); + }, + ); + + it.each([ + [true, { anything: 1 }, "default", "key", "TARGETING_MATCH"], + [false, 1337, "default", "default", "DISABLED"], + ])( + "should return the correct result when evaluating string %s, %s, %s, %s, %s`", + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, "key", value); + expect(ofClient.getStringDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([ + [true, [], [1], [], "TARGETING_MATCH"], + [true, null, [2], [2], "DEFAULT"], + [false, [3], [4], [4], "DISABLED"], + [false, [5], [6], [6], "DISABLED"], + ])( + "should return the correct result when evaluating array %s, %s, %s, %s, %s`", + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, value ? "key" : undefined, value); + expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([ + [true, {}, { a: 1 }, {}, "TARGETING_MATCH"], + [true, null, { a: 2 }, { a: 2 }, "DEFAULT"], + [false, { a: 3 }, { a: 4 }, { a: 4 }, "DISABLED"], + [false, { a: 5 }, { a: 6 }, { a: 6 }, "DISABLED"], + ])( + "should return the correct result when evaluating object %s, %s, %s, %s, %s`", + async (enabled, value, def, expected, reason) => { + mockFeature(enabled, value ? "key" : undefined, value); + expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: reason, + value: expected, + }); + }, + ); + + it.each([["string"], [15], [true]])( + "should handle type mismatch when evaluating object as %s`", + async (value) => { + mockFeature(true, "key", value); + expect(ofClient.getObjectDetails(testFlagKey, { obj: true })).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "TYPE_MISMATCH", + errorMessage: "", + value: { obj: true }, + }); + }, + ); }); describe("track", () => { diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index 48894ecb..df10699c 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -44,8 +44,8 @@ export class BucketBrowserSDKProvider implements Provider { private _client?: BucketClient; - private _clientOptions: InitOptions; - private _contextTranslator: ContextTranslationFn; + private readonly _clientOptions: InitOptions; + private readonly _contextTranslator: ContextTranslationFn; public events = new OpenFeatureEventEmitter(); @@ -100,66 +100,96 @@ export class BucketBrowserSDKProvider implements Provider { await this.initialize(newContext); } - resolveBooleanEvaluation( + private resolveFeature( flagKey: string, - defaultValue: boolean, - ): ResolutionDetails { - if (!this._client) + defaultValue: T, + ): ResolutionDetails { + const expType = typeof defaultValue; + + if (!this._client) { return { value: defaultValue, reason: StandardResolutionReasons.DEFAULT, errorCode: ErrorCode.PROVIDER_NOT_READY, - } satisfies ResolutionDetails; + errorMessage: "Bucket client not initialized", + } satisfies ResolutionDetails; + } const features = this._client.getFeatures(); if (flagKey in features) { const feature = this._client.getFeature(flagKey); + + if (!feature.isEnabled) { + return { + value: defaultValue, + reason: StandardResolutionReasons.DISABLED, + }; + } + + if (expType === "boolean") { + return { + value: true as T, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + } + + if (!feature.config.key) { + return { + value: defaultValue, + reason: StandardResolutionReasons.DEFAULT, + }; + } + + if (expType === "string") { + return { + value: feature.config.payload as T, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + } + + if (typeof feature.config.payload !== expType) { + return { + value: defaultValue, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.TYPE_MISMATCH, + errorMessage: `Expected ${expType} but got ${typeof feature.config.payload}`, + }; + } + return { - value: feature.isEnabled, + value: feature.config.payload as T, reason: StandardResolutionReasons.TARGETING_MATCH, - } satisfies ResolutionDetails; + }; } return { value: defaultValue, reason: StandardResolutionReasons.DEFAULT, - } satisfies ResolutionDetails; + errorCode: ErrorCode.FLAG_NOT_FOUND, + errorMessage: `Flag ${flagKey} not found`, + }; } - resolveNumberEvaluation( - _flagKey: string, - defaultValue: number, - ): ResolutionDetails { - return { - value: defaultValue, - errorCode: ErrorCode.TYPE_MISMATCH, - reason: StandardResolutionReasons.ERROR, - errorMessage: "Bucket doesn't support number flags", - }; + resolveBooleanEvaluation(flagKey: string, defaultValue: boolean) { + return this.resolveFeature(flagKey, defaultValue); + } + + resolveNumberEvaluation(flagKey: string, defaultValue: number) { + return this.resolveFeature(flagKey, defaultValue); } resolveObjectEvaluation( - _flagKey: string, + flagKey: string, defaultValue: T, - ): ResolutionDetails { - return { - value: defaultValue, - errorCode: ErrorCode.TYPE_MISMATCH, - reason: StandardResolutionReasons.ERROR, - errorMessage: "Bucket doesn't support object flags", - }; + ) { + return this.resolveFeature(flagKey, defaultValue); } resolveStringEvaluation( - _flagKey: string, + flagKey: string, defaultValue: string, ): ResolutionDetails { - return { - value: defaultValue, - errorCode: ErrorCode.TYPE_MISMATCH, - reason: StandardResolutionReasons.ERROR, - errorMessage: "Bucket doesn't support string flags", - }; + return this.resolveFeature(flagKey, defaultValue); } track( @@ -171,8 +201,10 @@ export class BucketBrowserSDKProvider implements Provider { this._clientOptions.logger?.error("client not initialized"); } - this._client?.track(trackingEventName, trackingEventDetails).catch((e) => { - this._clientOptions.logger?.error("error tracking event", e); - }); + this._client + ?.track(trackingEventName, trackingEventDetails) + .catch((e: any) => { + this._clientOptions.logger?.error("error tracking event", e); + }); } } diff --git a/yarn.lock b/yarn.lock index 812c4c4b..45002e2f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,18 +882,6 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:2.4.0": - version: 2.4.0 - resolution: "@bucketco/browser-sdk@npm:2.4.0" - dependencies: - "@floating-ui/dom": "npm:^1.6.8" - canonical-json: "npm:^0.0.4" - js-cookie: "npm:^3.0.5" - preact: "npm:^10.22.1" - checksum: 10c0/b33a9fdafa4a857ac4f815fe69b602b37527a37d54270abd479b754da998d030f5a70b738c662ab57fa4f6374b8e1fbd052feb8bdbd8b78367086dcedc5a5432 - languageName: node - linkType: hard - "@bucketco/browser-sdk@npm:2.5.1, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" @@ -984,7 +972,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/openfeature-browser-provider@workspace:packages/openfeature-browser-provider" dependencies: - "@bucketco/browser-sdk": "npm:2.4.0" + "@bucketco/browser-sdk": "npm:2.5.1" "@bucketco/eslint-config": "npm:0.0.2" "@bucketco/tsconfig": "npm:0.0.2" "@openfeature/core": "npm:1.5.0" From 295f05d80ffb499225d090b3d57a1fae69c46e76 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 18:54:18 +0000 Subject: [PATCH 13/34] fix(browser-sdk): update feature remote config type definition Simplified the feature remote config type by removing unnecessary properties and aligning with recent configuration changes. Minor refactoring of sendCheckEvent method to use shorthand property syntax. --- packages/browser-sdk/src/client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index a34c94e0..94f27984 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -218,7 +218,7 @@ export type FeatureRemoteConfig = */ payload: any; } - | { key: undefined; targetingVersion: undefined; value: undefined }; + | { key: undefined; payload: undefined }; /** * A feature. @@ -616,7 +616,7 @@ export class BucketClient { function sendCheckEvent() { fClient .sendCheckEvent({ - key: key, + key, version: f?.targetingVersion, value, }) From 0dd5757a40d9e956f6f5ae65a099953404b556e9 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 18:59:10 +0000 Subject: [PATCH 14/34] chore(react-sdk): remove unused import from index.tsx Cleaned up an unnecessary import of Features from a development-specific path in the main index file. --- packages/react-sdk/src/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index e1203c28..e2ee4e7f 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -21,7 +21,6 @@ import { UnassignedFeedback, } from "@bucketco/browser-sdk"; -import { Features } from "../dev/nextjs-flag-demo/components/Flags"; import { version } from "../package.json"; export interface Features {} From 717342364744248eb417653a968b8c11c4d86928 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 19:11:01 +0000 Subject: [PATCH 15/34] feat(browser-sdk): enhance fallback feature configuration handling Update FallbackFeatureConfig type to support boolean and object overrides, allowing more flexible feature flag configuration. Modify feature fallback logic to handle both simple boolean flags and complex feature configurations. --- packages/browser-sdk/src/feature/features.ts | 37 +++++++++++--------- packages/browser-sdk/test/features.test.ts | 7 ++++ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 79d18d52..3e4a89b9 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -63,10 +63,12 @@ export type RawFeature = FetchedFeature & { export type RawFeatures = Record; -export type FallbackFeatureConfig = { - key: string; - payload: any; -} | null; +export type FallbackFeatureOverride = + | { + key: string; + payload: any; + } + | true; export type FeaturesOptions = { /** @@ -75,7 +77,7 @@ export type FeaturesOptions = { * is supplied instead of array, the values of each key represent the * configuration values and `isEnabled` is assume `true`. */ - fallbackFeatures?: string[] | Record; + fallbackFeatures?: string[] | Record; /** * Timeout in milliseconds when fetching features @@ -99,7 +101,7 @@ export type FeaturesOptions = { }; type Config = { - fallbackFeatures: Record; + fallbackFeatures: Record; timeoutMs: number; staleWhileRevalidate: boolean; }; @@ -236,15 +238,15 @@ export class FeaturesClient { expireTimeMs: options?.expireTimeMs ?? FEATURES_EXPIRE_MS, }); - let fallbackFeatures: Record; + let fallbackFeatures: Record; if (Array.isArray(options?.fallbackFeatures)) { fallbackFeatures = options.fallbackFeatures.reduce( (acc, key) => { - acc[key] = null; + acc[key] = true; return acc; }, - {} as Record, + {} as Record, ); } else { fallbackFeatures = options?.fallbackFeatures ?? {}; @@ -477,16 +479,17 @@ export class FeaturesClient { // fetch failed, nothing cached => return fallbacks return Object.entries(this.config.fallbackFeatures).reduce( - (acc, [key, config]) => { + (acc, [key, override]) => { acc[key] = { key, - isEnabled: true, - config: config - ? { - key: config.key, - payload: config.payload, - } - : undefined, + isEnabled: !!override, + config: + typeof override === "object" && "key" in override + ? { + key: override.key, + payload: override.payload, + } + : undefined, }; return acc; }, diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index 98104f25..dcb4fb52 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -153,6 +153,7 @@ describe("FeaturesClient", () => { key: "john", payload: { something: "else" }, }, + zoom: true, }, }); @@ -164,6 +165,12 @@ describe("FeaturesClient", () => { key: "huddle", isEnabledOverride: null, }, + zoom: { + isEnabled: true, + config: undefined, + key: "zoom", + isEnabledOverride: null, + }, }); }); From 862df2508fe14f6b172dfe3c6899faa6f5a5b98d Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 19:12:12 +0000 Subject: [PATCH 16/34] test(browser-sdk): remove .only from features test Remove the .only modifier from the "caches response" test to ensure all tests are run during test execution. --- packages/browser-sdk/test/features.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index dcb4fb52..b7f61108 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -174,7 +174,7 @@ describe("FeaturesClient", () => { }); }); - test.only("caches response", async () => { + test("caches response", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); const featuresClient1 = newFeaturesClient(); From e22475b6c7857fa8a646ffed7cb9c2e0985fa600 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 19:24:43 +0000 Subject: [PATCH 17/34] test(browser-sdk): update feature test expectations Modify test assertions to reflect changes in feature configuration handling, including expected feature state and override behavior. --- packages/browser-sdk/test/features.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index b7f61108..9213a939 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -354,15 +354,12 @@ describe("FeaturesClient", () => { updated = true; }); - expect(client.getFeatures().featureB.isEnabled).toBe(false); + expect(client.getFeatures().featureB.isEnabled).toBe(true); expect(client.getFeatures().featureB.isEnabledOverride).toBe(null); - expect(client.getFetchedFeatures()?.featureB).toBeUndefined(); - client.setFeatureOverride("featureC", true); expect(updated).toBe(true); - expect(client.getFeatures().featureC.isEnabled).toBe(false); - expect(client.getFeatures().featureC.isEnabledOverride).toBe(true); + expect(client.getFeatures().featureC).toBeUndefined(); }); }); From 31ee5561c8b5986ddab020b36adf4b5fe5f1a54b Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 19:33:47 +0000 Subject: [PATCH 18/34] refactor(react-sdk): simplify feature config retrieval Update useFeature hook to return a reduced feature configuration with only key and payload properties, aligning with recent configuration changes in the SDK. --- packages/react-sdk/src/index.tsx | 6 +++++- packages/react-sdk/test/usage.test.tsx | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index e2ee4e7f..355e0535 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -247,6 +247,10 @@ export function useFeature(key: TKey): Feature { }); } + const reducedConfig = feature?.config + ? { key: feature.config.key, payload: feature.config.payload } + : ({} as EmptyFeatureConfig); + return { isLoading, track, @@ -257,7 +261,7 @@ export function useFeature(key: TKey): Feature { }, get config() { sendCheckEvent(); - return feature?.config ?? ({} as EmptyFeatureConfig); + return reducedConfig; }, }; } diff --git a/packages/react-sdk/test/usage.test.tsx b/packages/react-sdk/test/usage.test.tsx index 573f4e1b..5132a886 100644 --- a/packages/react-sdk/test/usage.test.tsx +++ b/packages/react-sdk/test/usage.test.tsx @@ -263,8 +263,7 @@ describe("useFeature", () => { isLoading: false, config: { key: "gpt3", - targetingVersion: 2, - value: { model: "gpt-something", temperature: 0.5 }, + payload: { model: "gpt-something", temperature: 0.5 }, }, track: expect.any(Function), requestFeedback: expect.any(Function), From 2e62475d106132417e5b6f6a85ebcf588e68fdfd Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 19:35:18 +0000 Subject: [PATCH 19/34] refactor(react-sdk): simplify feature config access and reduce payload Update useFeature hook to create a reduced configuration object with only key and payload, streamlining config retrieval and aligning with recent configuration changes in the SDK. --- packages/react-sdk/src/index.tsx | 6 +++++- packages/react-sdk/test/usage.test.tsx | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index e2ee4e7f..355e0535 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -247,6 +247,10 @@ export function useFeature(key: TKey): Feature { }); } + const reducedConfig = feature?.config + ? { key: feature.config.key, payload: feature.config.payload } + : ({} as EmptyFeatureConfig); + return { isLoading, track, @@ -257,7 +261,7 @@ export function useFeature(key: TKey): Feature { }, get config() { sendCheckEvent(); - return feature?.config ?? ({} as EmptyFeatureConfig); + return reducedConfig; }, }; } diff --git a/packages/react-sdk/test/usage.test.tsx b/packages/react-sdk/test/usage.test.tsx index 573f4e1b..5132a886 100644 --- a/packages/react-sdk/test/usage.test.tsx +++ b/packages/react-sdk/test/usage.test.tsx @@ -263,8 +263,7 @@ describe("useFeature", () => { isLoading: false, config: { key: "gpt3", - targetingVersion: 2, - value: { model: "gpt-something", temperature: 0.5 }, + payload: { model: "gpt-something", temperature: 0.5 }, }, track: expect.any(Function), requestFeedback: expect.any(Function), From 13f5f2f638e95e651179bd33e3e090c05c0b95ad Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Mon, 27 Jan 2025 20:16:25 +0000 Subject: [PATCH 20/34] test(browser-sdk): improve test coverage and error handling for flag evaluations Update test cases for boolean, number, string, and object flag evaluations to: - Enhance test descriptions with more context - Add type mismatch test scenarios - Refine feature flag evaluation logic - Improve error handling for different input types --- .../src/index.test.ts | 33 +++++++++++++------ .../openfeature-browser-provider/src/index.ts | 9 +---- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index c8418a9d..fedab961 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -135,16 +135,14 @@ describe("BucketBrowserSDKProvider", () => { it.each([ [true, true, false, true, "TARGETING_MATCH"], - [true, false, false, true, "TARGETING_MATCH"], - [true, null, false, true, "TARGETING_MATCH"], - [true, { obj: true }, false, true, "TARGETING_MATCH"], - [true, 15, false, true, "TARGETING_MATCH"], + [true, false, false, false, "TARGETING_MATCH"], + [true, null, true, true, "DEFAULT"], [false, true, false, false, "DISABLED"], [false, true, true, true, "DISABLED"], ])( - "should return the correct result when evaluating boolean %s, %s, %s, %s, %s`", + "should return the correct result when evaluating boolean. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", async (enabled, value, def, expected, reason) => { - mockFeature(enabled, "key", value); + mockFeature(enabled, value !== null ? "key" : undefined, value); expect(ofClient.getBooleanDetails(testFlagKey, def)).toEqual({ flagKey: "a-key", flagMetadata: {}, @@ -154,13 +152,28 @@ describe("BucketBrowserSDKProvider", () => { }, ); + it.each([[null], [{ obj: true }], [15]])( + "should handle type mismatch when evaluating boolean as %s.`", + async (value) => { + mockFeature(true, "key", value); + expect(ofClient.getBooleanDetails(testFlagKey, true)).toEqual({ + flagKey: "a-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "TYPE_MISMATCH", + errorMessage: "", + value: true, + }); + }, + ); + it.each([ [true, 1, -1, 1, "TARGETING_MATCH"], [true, null, -2, -2, "DEFAULT"], [false, 3, -3, -3, "DISABLED"], [false, 4, -4, -4, "DISABLED"], ])( - "should return the correct result when evaluating number %s, %s, %s, %s, %s`", + "should return the correct result when evaluating number. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", async (enabled, value, def, expected, reason) => { mockFeature(enabled, value ? "key" : undefined, value); expect(ofClient.getNumberDetails(testFlagKey, def)).toEqual({ @@ -191,7 +204,7 @@ describe("BucketBrowserSDKProvider", () => { [true, { anything: 1 }, "default", "key", "TARGETING_MATCH"], [false, 1337, "default", "default", "DISABLED"], ])( - "should return the correct result when evaluating string %s, %s, %s, %s, %s`", + "should return the correct result when evaluating string. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", async (enabled, value, def, expected, reason) => { mockFeature(enabled, "key", value); expect(ofClient.getStringDetails(testFlagKey, def)).toEqual({ @@ -209,7 +222,7 @@ describe("BucketBrowserSDKProvider", () => { [false, [3], [4], [4], "DISABLED"], [false, [5], [6], [6], "DISABLED"], ])( - "should return the correct result when evaluating array %s, %s, %s, %s, %s`", + "should return the correct result when evaluating array. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", async (enabled, value, def, expected, reason) => { mockFeature(enabled, value ? "key" : undefined, value); expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ @@ -227,7 +240,7 @@ describe("BucketBrowserSDKProvider", () => { [false, { a: 3 }, { a: 4 }, { a: 4 }, "DISABLED"], [false, { a: 5 }, { a: 6 }, { a: 6 }, "DISABLED"], ])( - "should return the correct result when evaluating object %s, %s, %s, %s, %s`", + "should return the correct result when evaluating object. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", async (enabled, value, def, expected, reason) => { mockFeature(enabled, value ? "key" : undefined, value); expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index df10699c..e694f6c1 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -126,13 +126,6 @@ export class BucketBrowserSDKProvider implements Provider { }; } - if (expType === "boolean") { - return { - value: true as T, - reason: StandardResolutionReasons.TARGETING_MATCH, - }; - } - if (!feature.config.key) { return { value: defaultValue, @@ -142,7 +135,7 @@ export class BucketBrowserSDKProvider implements Provider { if (expType === "string") { return { - value: feature.config.payload as T, + value: feature.config.key as T, reason: StandardResolutionReasons.TARGETING_MATCH, }; } From fa735d875c9d4e929927d80d908881b8948ff5c5 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 28 Jan 2025 11:32:54 +0000 Subject: [PATCH 21/34] docs(browser-sdk,react-sdk): clarify remote config documentation Update README files for browser and react SDKs to provide more detailed explanations of remote configuration handling, including: - Clarifying optional payload behavior - Adding guidance on handling undefined config - Improving code examples for remote config usage --- packages/browser-sdk/README.md | 10 +++- packages/browser-sdk/src/client.ts | 2 +- packages/browser-sdk/src/feature/features.ts | 4 +- packages/react-sdk/README.md | 58 +++++++++++++++++++- 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index 4a3ed79c..49a796b4 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -164,7 +164,8 @@ generate a `check` event, contrary to the `isEnabled` property on the object ret ### Remote config Similar to `isEnabled`, each feature has a `config` property. This configuration is managed from within Bucket. -It is managed similar to the way access to features is managed, but instead the binary `isEnabled` you can have multiple configuration values which are given to different user/companies. +It is managed similar to the way access to features is managed, but instead of the binary `isEnabled` you can have +multiple configuration values which are given to different user/companies. ```ts const features = bucketClient.getFeatures(); @@ -172,7 +173,7 @@ const features = bucketClient.getFeatures(); // huddle: { // isEnabled: true, // targetingVersion: 42, -// config?: { +// config: { // key: "gpt-3.5", // payload: { maxTokens: 10000, model: "gpt-3.5-beta1" } // } @@ -180,7 +181,10 @@ const features = bucketClient.getFeatures(); // } ``` -The `key` is always present while the `payload` is a optional JSON value for arbitrary configuration needs. ` +The `key` is always present while the `payload` is a optional JSON value for arbitrary configuration needs. +If feature has no configuration or, no configuration value was matched against the context, the `config` object +will be empty, thus, `key` will be `undefined`. Make sure to check against this case when trying to use the +configuration in your application. Just as `isEnabled`, accessing `config` on the object returned by `getFeatures` does not automatically generate a `check` event, contrary to the `config` property on the object returned by `getFeature`. diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 94f27984..76d03bcb 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -214,7 +214,7 @@ export type FeatureRemoteConfig = key: string; /** - * The user-supplied data. + * The optional user-supplied payload data. */ payload: any; } diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 3e4a89b9..7c11420a 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -43,9 +43,9 @@ export type FetchedFeature = { version?: number; /** - * The user-supplied data. + * The optional user-supplied payload data. */ - payload: any; + payload?: any; }; }; diff --git a/packages/react-sdk/README.md b/packages/react-sdk/README.md index 0db4a4bf..2484c79c 100644 --- a/packages/react-sdk/README.md +++ b/packages/react-sdk/README.md @@ -68,7 +68,7 @@ import { BucketProvider } from "@bucketco/react-sdk"; ```tsx function LoadingBucket({ children }) { - const {isLoading} = useFeature("myFeature") + const { isLoading } = useFeature("myFeature") if (isLoading) { return } @@ -86,6 +86,62 @@ import { BucketProvider } from "@bucketco/react-sdk"; - `enableTracking` (default: `true`): Set to `false` to stop sending tracking events and user/company updates to Bucket. Useful when you're impersonating a user. +## Feature toggles + +Bucket determines which features are active for a given `user`/`company`. The `user`/`company` are given in the `BucketProvider` as props. + +If you supply `user` or `company` objects, they must include at least the `id` property otherwise they will be ignored in their entirety. +In addition to the `id`, you must also supply anything additional that you want to be able to evaluate feature targeting rules against. +The additional attributes are supplied using the `otherContext` prop. + +Attributes cannot be nested (multiple levels) and must be either strings, integers or booleans. + +- `name` is a special attribute and is used to display name for user/company +- for `user`, `email` is also special and will be highlighted in the Bucket UI if available + +```tsx + + + {/* children here are shown when loading finishes */} + + +``` + +To retrieve features along with their targeting information, use `useFeature(key: string)` hook (described in a section below). + +Note that accessing `isEnabled` on the object returned by `useFeature()` automatically +generates a `check` event. + +## Remote config + +Similar to `isEnabled`, each feature accessed using `useFeature()` hook, has a `config` property. This configuration +is managed from within Bucket. It is managed similar to the way access to features is managed, but instead of the +binary `isEnabled` you can have multiple configuration values which are given to different user/companies. + +```ts +const { + isEnabled, + config: { key, payload }, +} = useFeature("huddles"); + +// isEnabled: true, +// key: "gpt-3.5", +// payload: { maxTokens: 10000, model: "gpt-3.5-beta1" } +``` + +The `key` is always present while the `payload` is a optional JSON value for arbitrary configuration needs. +If feature has no configuration or, no configuration value was matched against the context, the `config` object +will be empty, thus, `key` will be `undefined`. Make sure to check against this case when trying to use the +configuration in your application. + +Note that, similar to `isEnabled`, accessing `config` on the object returned by `useFeature()` automatically +generates a `check` event. + ## Hooks ### `useFeature()` From f986feb03e94685ff29bb1a9a3a1b65c46b1571c Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 28 Jan 2025 12:24:13 +0000 Subject: [PATCH 22/34] feat(browser-sdk): export FallbackFeatureOverride type Add FallbackFeatureOverride to the exported types, extending the SDK's type exports for feature configuration --- packages/browser-sdk/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/browser-sdk/src/index.ts b/packages/browser-sdk/src/index.ts index 7b1186cb..2b4e7bf8 100644 --- a/packages/browser-sdk/src/index.ts +++ b/packages/browser-sdk/src/index.ts @@ -5,6 +5,7 @@ export { BucketClient } from "./client"; export type { BucketContext, CompanyContext, UserContext } from "./context"; export type { CheckEvent, + FallbackFeatureOverride, FeaturesOptions, RawFeature, RawFeatures, From 7c6c47c7992c62fd766e7c17dbed89ace1f463f1 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 28 Jan 2025 12:32:11 +0000 Subject: [PATCH 23/34] chore(release): bump browser-sdk and react-sdk to 3.0.0-alpha.1 Update package versions and lock file to reflect the new alpha release, ensuring consistent versioning across SDKs --- packages/browser-sdk/package.json | 2 +- packages/react-sdk/package.json | 4 ++-- yarn.lock | 16 ++-------------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/browser-sdk/package.json b/packages/browser-sdk/package.json index b6117ec5..b391f4e4 100644 --- a/packages/browser-sdk/package.json +++ b/packages/browser-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/browser-sdk", - "version": "3.0.0-alpha.0", + "version": "3.0.0-alpha.1", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { diff --git a/packages/react-sdk/package.json b/packages/react-sdk/package.json index c8d9a1fa..6dc4cadf 100644 --- a/packages/react-sdk/package.json +++ b/packages/react-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/react-sdk", - "version": "3.0.0-alpha.0", + "version": "3.0.0-alpha.1", "license": "MIT", "repository": { "type": "git", @@ -34,7 +34,7 @@ } }, "dependencies": { - "@bucketco/browser-sdk": "3.0.0-alpha.0", + "@bucketco/browser-sdk": "3.0.0-alpha.1", "canonical-json": "^0.0.4", "rollup": "^4.2.0" }, diff --git a/yarn.lock b/yarn.lock index e79fc1d5..1081f5ec 100644 --- a/yarn.lock +++ b/yarn.lock @@ -894,19 +894,7 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:3.0.0-alpha.0": - version: 3.0.0-alpha.0 - resolution: "@bucketco/browser-sdk@npm:3.0.0-alpha.0" - dependencies: - "@floating-ui/dom": "npm:^1.6.8" - canonical-json: "npm:^0.0.4" - js-cookie: "npm:^3.0.5" - preact: "npm:^10.22.1" - checksum: 10c0/eba1a2f15c49c1aeca37c6d2b218e50d92e2034c51a9acdd6884ce143156dc7f285f9141f5e6cfcc2aed0186c0b8fc61bb9939aca737a1e76d1a2d9d65a7b74c - languageName: node - linkType: hard - -"@bucketco/browser-sdk@workspace:packages/browser-sdk": +"@bucketco/browser-sdk@npm:3.0.0-alpha.1, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" dependencies: @@ -1042,7 +1030,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/react-sdk@workspace:packages/react-sdk" dependencies: - "@bucketco/browser-sdk": "npm:3.0.0-alpha.0" + "@bucketco/browser-sdk": "npm:3.0.0-alpha.1" "@bucketco/eslint-config": "workspace:^" "@bucketco/tsconfig": "workspace:^" "@testing-library/react": "npm:^15.0.7" From a75eec8e3d9590680254db5c806bb1e151edce67 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 28 Jan 2025 12:58:17 +0000 Subject: [PATCH 24/34] chore(deps): upgrade @bucketco/browser-sdk to 3.0.0-alpha.0 --- yarn.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/yarn.lock b/yarn.lock index 26c20adb..2523eca3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,15 +882,15 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:2.4.0": - version: 2.4.0 - resolution: "@bucketco/browser-sdk@npm:2.4.0" +"@bucketco/browser-sdk@npm:3.0.0-alpha.0": + version: 3.0.0-alpha.0 + resolution: "@bucketco/browser-sdk@npm:3.0.0-alpha.0" dependencies: "@floating-ui/dom": "npm:^1.6.8" canonical-json: "npm:^0.0.4" js-cookie: "npm:^3.0.5" preact: "npm:^10.22.1" - checksum: 10c0/b33a9fdafa4a857ac4f815fe69b602b37527a37d54270abd479b754da998d030f5a70b738c662ab57fa4f6374b8e1fbd052feb8bdbd8b78367086dcedc5a5432 + checksum: 10c0/eba1a2f15c49c1aeca37c6d2b218e50d92e2034c51a9acdd6884ce143156dc7f285f9141f5e6cfcc2aed0186c0b8fc61bb9939aca737a1e76d1a2d9d65a7b74c languageName: node linkType: hard From 9d89947963a322030d4b646469df55659e80298a Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 28 Jan 2025 13:03:06 +0000 Subject: [PATCH 25/34] chore(deps): upgrade @bucketco/browser-sdk to 3.0.0-alpha.1 --- packages/openfeature-browser-provider/package.json | 2 +- yarn.lock | 14 +------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index 84b60320..4665815e 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -35,7 +35,7 @@ } }, "dependencies": { - "@bucketco/browser-sdk": "3.0.0-alpha.0" + "@bucketco/browser-sdk": "3.0.0-alpha.1" }, "devDependencies": { "@bucketco/eslint-config": "0.0.2", diff --git a/yarn.lock b/yarn.lock index 2523eca3..5fd2bdb7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,18 +882,6 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:3.0.0-alpha.0": - version: 3.0.0-alpha.0 - resolution: "@bucketco/browser-sdk@npm:3.0.0-alpha.0" - dependencies: - "@floating-ui/dom": "npm:^1.6.8" - canonical-json: "npm:^0.0.4" - js-cookie: "npm:^3.0.5" - preact: "npm:^10.22.1" - checksum: 10c0/eba1a2f15c49c1aeca37c6d2b218e50d92e2034c51a9acdd6884ce143156dc7f285f9141f5e6cfcc2aed0186c0b8fc61bb9939aca737a1e76d1a2d9d65a7b74c - languageName: node - linkType: hard - "@bucketco/browser-sdk@npm:3.0.0-alpha.1, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" @@ -984,7 +972,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/openfeature-browser-provider@workspace:packages/openfeature-browser-provider" dependencies: - "@bucketco/browser-sdk": "npm:3.0.0-alpha.0" + "@bucketco/browser-sdk": "npm:3.0.0-alpha.1" "@bucketco/eslint-config": "npm:0.0.2" "@bucketco/tsconfig": "npm:0.0.2" "@openfeature/core": "npm:1.5.0" From 91a7ae0fb22b216f13e9c742b83c1f32ec0c7a7b Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 29 Jan 2025 17:18:14 +0000 Subject: [PATCH 26/34] chore(deps): upgrade @bucketco/browser-sdk to 3.0.0-alpha.1 --- yarn.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/yarn.lock b/yarn.lock index 498e0466..f84b8c83 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,15 +882,15 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:2.4.0": - version: 2.4.0 - resolution: "@bucketco/browser-sdk@npm:2.4.0" +"@bucketco/browser-sdk@npm:3.0.0-alpha.1": + version: 3.0.0-alpha.1 + resolution: "@bucketco/browser-sdk@npm:3.0.0-alpha.1" dependencies: "@floating-ui/dom": "npm:^1.6.8" canonical-json: "npm:^0.0.4" js-cookie: "npm:^3.0.5" preact: "npm:^10.22.1" - checksum: 10c0/b33a9fdafa4a857ac4f815fe69b602b37527a37d54270abd479b754da998d030f5a70b738c662ab57fa4f6374b8e1fbd052feb8bdbd8b78367086dcedc5a5432 + checksum: 10c0/4dd602d70e12d577445c262834605c6a90d5e7ee9d8a1a018295aa7cc1773227d3fd442f7e12fbf23092c13207c0d5ac6a89d1b0d7cef7d7e24b5b9999e573c8 languageName: node linkType: hard From aee63d5ded1ce72f1eb69b2e872a77314903b187 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 4 Feb 2025 10:48:04 +0000 Subject: [PATCH 27/34] fix(node-sdk): update Feature type to support undefined config --- packages/node-sdk/package.json | 2 +- packages/node-sdk/src/types.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/node-sdk/package.json b/packages/node-sdk/package.json index 4e4c8236..68a3b4f3 100644 --- a/packages/node-sdk/package.json +++ b/packages/node-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/node-sdk", - "version": "1.6.0-alpha.0", + "version": "1.6.0-alpha.1", "license": "MIT", "repository": { "type": "git", diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index e8637774..28e67598 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -133,7 +133,7 @@ export type FeatureRemoteConfig = * Describes a feature */ export interface Feature< - TConfig extends FeatureRemoteConfig | never = EmptyFeatureRemoteConfig, + TConfig extends FeatureRemoteConfig | undefined = EmptyFeatureRemoteConfig, > { /** * The key of the feature. @@ -148,7 +148,7 @@ export interface Feature< /* * Optional user-defined configuration. */ - config: TConfig extends never ? EmptyFeatureRemoteConfig : TConfig; + config: TConfig extends undefined ? EmptyFeatureRemoteConfig : TConfig; /** * Track feature usage in Bucket. From 2029f91083acaa0f822d8a61ffb0ff512751e4d4 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 06:24:55 +0000 Subject: [PATCH 28/34] chore(node-sdk): bump version to 1.6.0-alpha.3 --- packages/node-sdk/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node-sdk/package.json b/packages/node-sdk/package.json index 68a3b4f3..041bbc27 100644 --- a/packages/node-sdk/package.json +++ b/packages/node-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/node-sdk", - "version": "1.6.0-alpha.1", + "version": "1.6.0-alpha.3", "license": "MIT", "repository": { "type": "git", From dc93f9310780b07bdb42d1d344ecd4be4f027703 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Sat, 8 Feb 2025 11:20:05 +0000 Subject: [PATCH 29/34] feat(openfeature-browser-provider): Enhance provider with improved feature resolution and lifecycle management - Update provider to handle feature resolution with more robust type checking - Add support for provider lifecycle methods, including stopping the client - Improve error handling for different evaluation scenarios - Update dependencies to use @bucketco/browser-sdk 3.0.0-alpha.2 - Bump package version to 0.4.0-alpha.0 --- .../openfeature-browser-provider/package.json | 4 +- .../src/index.test.ts | 207 ++++++++---------- .../openfeature-browser-provider/src/index.ts | 96 ++++---- yarn.lock | 14 +- 4 files changed, 145 insertions(+), 176 deletions(-) diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index 0bc41c89..037c3402 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/openfeature-browser-provider", - "version": "0.3.1", + "version": "0.4.0-alpha.0", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { @@ -35,7 +35,7 @@ } }, "dependencies": { - "@bucketco/browser-sdk": "3.0.0-alpha.1" + "@bucketco/browser-sdk": "3.0.0-alpha.2" }, "devDependencies": { "@bucketco/eslint-config": "0.0.2", diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index fedab961..9f8b4e1f 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -1,5 +1,5 @@ import { Client, OpenFeature } from "@openfeature/web-sdk"; -import { beforeAll, beforeEach, describe, expect, it, Mock, vi } from "vitest"; +import { beforeEach, describe, expect, it, Mock, vi } from "vitest"; import { BucketClient } from "@bucketco/browser-sdk"; @@ -27,23 +27,27 @@ describe("BucketBrowserSDKProvider", () => { getFeature: vi.fn(), initialize: vi.fn().mockResolvedValue({}), track: vi.fn(), + stop: vi.fn(), }; const newBucketClient = BucketClient as Mock; newBucketClient.mockReturnValue(bucketClientMock); - beforeAll(() => { + beforeEach(async () => { + await OpenFeature.clearProviders(); + provider = new BucketBrowserSDKProvider({ publishableKey }); OpenFeature.setProvider(provider); ofClient = OpenFeature.getClient(); }); + beforeEach(() => { vi.clearAllMocks(); }); const contextTranslatorFn = vi.fn(); - describe("initialize", () => { + describe("lifecycle", () => { it("should call initialize function with correct arguments", async () => { await provider.initialize(); expect(BucketClient).toHaveBeenCalledTimes(1); @@ -59,6 +63,11 @@ describe("BucketBrowserSDKProvider", () => { expect(bucketClientMock.initialize).toHaveBeenCalledTimes(1); expect(provider.status).toBe("READY"); }); + + it("should call stop function when provider is closed", async () => { + await OpenFeature.clearProviders(); + expect(bucketClientMock.stop).toHaveBeenCalledTimes(1); + }); }); describe("contextTranslator", () => { @@ -90,10 +99,14 @@ describe("BucketBrowserSDKProvider", () => { }); describe("resolveBooleanEvaluation", () => { + beforeEach(async () => { + await provider.initialize(); + }); + function mockFeature( enabled: boolean, - configKey: string | undefined, - configPayload: any | undefined, + configKey?: string | null, + configPayload?: any, ) { const config = { key: configKey, @@ -116,14 +129,40 @@ describe("BucketBrowserSDKProvider", () => { }); } + it("returns error if provider is not initialized", async () => { + await OpenFeature.clearProviders(); + + const val = ofClient.getBooleanDetails(testFlagKey, true); + + expect(val).toMatchObject({ + flagKey: testFlagKey, + flagMetadata: {}, + reason: "ERROR", + errorCode: "PROVIDER_NOT_READY", + value: true, + }); + }); + + it("returns error if flag is not found", async () => { + mockFeature(true, "key", true); + const val = ofClient.getBooleanDetails("missing-key", true); + + expect(val).toMatchObject({ + flagKey: "missing-key", + flagMetadata: {}, + reason: "ERROR", + errorCode: "FLAG_NOT_FOUND", + value: true, + }); + }); + it("calls the client correctly when evaluating", async () => { mockFeature(true, "key", true); - await provider.initialize(); const val = ofClient.getBooleanDetails(testFlagKey, false); - expect(val).toEqual({ - flagKey: "a-key", + expect(val).toMatchObject({ + flagKey: testFlagKey, flagMetadata: {}, reason: "TARGETING_MATCH", value: true, @@ -134,135 +173,71 @@ describe("BucketBrowserSDKProvider", () => { }); it.each([ - [true, true, false, true, "TARGETING_MATCH"], - [true, false, false, false, "TARGETING_MATCH"], - [true, null, true, true, "DEFAULT"], - [false, true, false, false, "DISABLED"], - [false, true, true, true, "DISABLED"], + [true, false, true, "TARGETING_MATCH", undefined], + [null, true, true, "ERROR", "FLAG_NOT_FOUND"], + [null, false, false, "ERROR", "FLAG_NOT_FOUND"], ])( - "should return the correct result when evaluating boolean. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", - async (enabled, value, def, expected, reason) => { - mockFeature(enabled, value !== null ? "key" : undefined, value); - expect(ofClient.getBooleanDetails(testFlagKey, def)).toEqual({ - flagKey: "a-key", - flagMetadata: {}, - reason: reason, - value: expected, - }); - }, - ); + "should return the correct result when evaluating boolean. enabled: %s, value: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", + (enabled, def, expected, reason, errorCode) => { + mockFeature(enabled ?? false); + const flagKey = enabled ? testFlagKey : "missing-key"; - it.each([[null], [{ obj: true }], [15]])( - "should handle type mismatch when evaluating boolean as %s.`", - async (value) => { - mockFeature(true, "key", value); - expect(ofClient.getBooleanDetails(testFlagKey, true)).toEqual({ - flagKey: "a-key", + expect(ofClient.getBooleanDetails(flagKey, def)).toMatchObject({ + flagKey, flagMetadata: {}, - reason: "ERROR", - errorCode: "TYPE_MISMATCH", - errorMessage: "", - value: true, - }); - }, - ); - - it.each([ - [true, 1, -1, 1, "TARGETING_MATCH"], - [true, null, -2, -2, "DEFAULT"], - [false, 3, -3, -3, "DISABLED"], - [false, 4, -4, -4, "DISABLED"], - ])( - "should return the correct result when evaluating number. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", - async (enabled, value, def, expected, reason) => { - mockFeature(enabled, value ? "key" : undefined, value); - expect(ofClient.getNumberDetails(testFlagKey, def)).toEqual({ - flagKey: "a-key", - flagMetadata: {}, - reason: reason, + reason, value: expected, + ...(errorCode ? { errorCode } : {}), }); }, ); - it.each([["string"], [true], [{}]])( - "should handle type mismatch when evaluating number as %s`", - async (value) => { - mockFeature(true, "key", value); - expect(ofClient.getNumberDetails(testFlagKey, -1)).toEqual({ - flagKey: "a-key", - flagMetadata: {}, - reason: "ERROR", - errorCode: "TYPE_MISMATCH", - errorMessage: "", - value: -1, - }); - }, - ); - - it.each([ - [true, { anything: 1 }, "default", "key", "TARGETING_MATCH"], - [false, 1337, "default", "default", "DISABLED"], - ])( - "should return the correct result when evaluating string. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", - async (enabled, value, def, expected, reason) => { - mockFeature(enabled, "key", value); - expect(ofClient.getStringDetails(testFlagKey, def)).toEqual({ - flagKey: "a-key", - flagMetadata: {}, - reason: reason, - value: expected, - }); - }, - ); + it("should return error when evaluating number", async () => { + expect(ofClient.getNumberDetails(testFlagKey, 1)).toMatchObject({ + flagKey: testFlagKey, + flagMetadata: {}, + reason: "ERROR", + errorCode: "GENERAL", + value: 1, + }); + }); it.each([ - [true, [], [1], [], "TARGETING_MATCH"], - [true, null, [2], [2], "DEFAULT"], - [false, [3], [4], [4], "DISABLED"], - [false, [5], [6], [6], "DISABLED"], + ["key-1", "default", "key-1", "TARGETING_MATCH"], + [null, "default", "default", "DEFAULT"], + [undefined, "default", "default", "DEFAULT"], ])( - "should return the correct result when evaluating array. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", - async (enabled, value, def, expected, reason) => { - mockFeature(enabled, value ? "key" : undefined, value); - expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ - flagKey: "a-key", + "should return the correct result when evaluating string. configKey: %s, def: %s, expected: %s, reason: %s, errorCode: %s`", + (configKey, def, expected, reason) => { + mockFeature(true, configKey, {}); + expect(ofClient.getStringDetails(testFlagKey, def)).toMatchObject({ + flagKey: testFlagKey, flagMetadata: {}, - reason: reason, + reason, value: expected, }); }, ); it.each([ - [true, {}, { a: 1 }, {}, "TARGETING_MATCH"], - [true, null, { a: 2 }, { a: 2 }, "DEFAULT"], - [false, { a: 3 }, { a: 4 }, { a: 4 }, "DISABLED"], - [false, { a: 5 }, { a: 6 }, { a: 6 }, "DISABLED"], + [{}, { a: 1 }, {}, "TARGETING_MATCH", undefined], + ["string", "default", "string", "TARGETING_MATCH", undefined], + [15, -15, 15, "TARGETING_MATCH", undefined], + [true, false, true, "TARGETING_MATCH", undefined], + [null, { a: 2 }, { a: 2 }, "ERROR", "TYPE_MISMATCH"], + [100, "string", "string", "ERROR", "TYPE_MISMATCH"], + [true, 1337, 1337, "ERROR", "TYPE_MISMATCH"], + ["string", 1337, 1337, "ERROR", "TYPE_MISMATCH"], ])( - "should return the correct result when evaluating object. enabled: %s, value: %s, default: %s, expected: %s, reason: %s`", - async (enabled, value, def, expected, reason) => { - mockFeature(enabled, value ? "key" : undefined, value); - expect(ofClient.getObjectDetails(testFlagKey, def)).toEqual({ - flagKey: "a-key", + "should return the correct result when evaluating object. payload: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", + (value, def, expected, reason, errorCode) => { + mockFeature(true, "config-key", value); + expect(ofClient.getObjectDetails(testFlagKey, def)).toMatchObject({ + flagKey: testFlagKey, flagMetadata: {}, - reason: reason, + reason, value: expected, - }); - }, - ); - - it.each([["string"], [15], [true]])( - "should handle type mismatch when evaluating object as %s`", - async (value) => { - mockFeature(true, "key", value); - expect(ofClient.getObjectDetails(testFlagKey, { obj: true })).toEqual({ - flagKey: "a-key", - flagMetadata: {}, - reason: "ERROR", - errorCode: "TYPE_MISMATCH", - errorMessage: "", - value: { obj: true }, + ...(errorCode ? { errorCode } : {}), }); }, ); diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index e694f6c1..a63558fa 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -11,7 +11,7 @@ import { TrackingEventDetails, } from "@openfeature/web-sdk"; -import { BucketClient, InitOptions } from "@bucketco/browser-sdk"; +import { BucketClient, Feature, InitOptions } from "@bucketco/browser-sdk"; // eslint-disable-next-line @typescript-eslint/no-explicit-any export type ContextTranslationFn = ( @@ -100,12 +100,11 @@ export class BucketBrowserSDKProvider implements Provider { await this.initialize(newContext); } - private resolveFeature( + private resolveFeature( flagKey: string, defaultValue: T, + resolveFn: (feature: Feature) => ResolutionDetails, ): ResolutionDetails { - const expType = typeof defaultValue; - if (!this._client) { return { value: defaultValue, @@ -117,42 +116,7 @@ export class BucketBrowserSDKProvider implements Provider { const features = this._client.getFeatures(); if (flagKey in features) { - const feature = this._client.getFeature(flagKey); - - if (!feature.isEnabled) { - return { - value: defaultValue, - reason: StandardResolutionReasons.DISABLED, - }; - } - - if (!feature.config.key) { - return { - value: defaultValue, - reason: StandardResolutionReasons.DEFAULT, - }; - } - - if (expType === "string") { - return { - value: feature.config.key as T, - reason: StandardResolutionReasons.TARGETING_MATCH, - }; - } - - if (typeof feature.config.payload !== expType) { - return { - value: defaultValue, - reason: StandardResolutionReasons.ERROR, - errorCode: ErrorCode.TYPE_MISMATCH, - errorMessage: `Expected ${expType} but got ${typeof feature.config.payload}`, - }; - } - - return { - value: feature.config.payload as T, - reason: StandardResolutionReasons.TARGETING_MATCH, - }; + return resolveFn(this._client.getFeature(flagKey)); } return { @@ -164,25 +128,67 @@ export class BucketBrowserSDKProvider implements Provider { } resolveBooleanEvaluation(flagKey: string, defaultValue: boolean) { - return this.resolveFeature(flagKey, defaultValue); + return this.resolveFeature(flagKey, defaultValue, (feature) => { + return { + value: feature.isEnabled, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + }); } - resolveNumberEvaluation(flagKey: string, defaultValue: number) { - return this.resolveFeature(flagKey, defaultValue); + resolveNumberEvaluation(_: string, defaultValue: number) { + return { + value: defaultValue, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.GENERAL, + errorMessage: + "Bucket doesn't support this method. Use `resolveObjectEvaluation` instead.", + }; } resolveObjectEvaluation( flagKey: string, defaultValue: T, ) { - return this.resolveFeature(flagKey, defaultValue); + return this.resolveFeature(flagKey, defaultValue, (feature) => { + const expType = typeof defaultValue; + const payload = feature.config?.payload; + + const payloadType = payload === null ? "null" : typeof payload; + + if (payloadType !== expType) { + return { + value: defaultValue, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.TYPE_MISMATCH, + errorMessage: `Expected remote config payload of type \`${expType}\` but got \`${payloadType}\`.`, + }; + } + + return { + value: payload, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + }); } resolveStringEvaluation( flagKey: string, defaultValue: string, ): ResolutionDetails { - return this.resolveFeature(flagKey, defaultValue); + return this.resolveFeature(flagKey, defaultValue, (feature) => { + if (!feature.config.key) { + return { + value: defaultValue, + reason: StandardResolutionReasons.DEFAULT, + }; + } + + return { + value: feature.config.key as string, + reason: StandardResolutionReasons.TARGETING_MATCH, + }; + }); } track( diff --git a/yarn.lock b/yarn.lock index 4fd59933..4db77033 100644 --- a/yarn.lock +++ b/yarn.lock @@ -882,18 +882,6 @@ __metadata: languageName: node linkType: hard -"@bucketco/browser-sdk@npm:3.0.0-alpha.1": - version: 3.0.0-alpha.1 - resolution: "@bucketco/browser-sdk@npm:3.0.0-alpha.1" - dependencies: - "@floating-ui/dom": "npm:^1.6.8" - canonical-json: "npm:^0.0.4" - js-cookie: "npm:^3.0.5" - preact: "npm:^10.22.1" - checksum: 10c0/4dd602d70e12d577445c262834605c6a90d5e7ee9d8a1a018295aa7cc1773227d3fd442f7e12fbf23092c13207c0d5ac6a89d1b0d7cef7d7e24b5b9999e573c8 - languageName: node - linkType: hard - "@bucketco/browser-sdk@npm:3.0.0-alpha.2, @bucketco/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@bucketco/browser-sdk@workspace:packages/browser-sdk" @@ -993,7 +981,7 @@ __metadata: version: 0.0.0-use.local resolution: "@bucketco/openfeature-browser-provider@workspace:packages/openfeature-browser-provider" dependencies: - "@bucketco/browser-sdk": "npm:3.0.0-alpha.1" + "@bucketco/browser-sdk": "npm:3.0.0-alpha.2" "@bucketco/eslint-config": "npm:0.0.2" "@bucketco/tsconfig": "npm:0.0.2" "@openfeature/core": "npm:1.5.0" From 02779b3bd33e03d5f203c0802b192560bb4cfaf9 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Sat, 8 Feb 2025 13:45:54 +0000 Subject: [PATCH 30/34] feat(openfeature-node-provider): Enhance provider with improved feature resolution and context handling - Update provider to support more robust feature resolution methods - Improve context translation with additional user and company attributes - Add support for variant tracking in feature evaluations - Enhance error handling for different evaluation scenarios - Update dependencies to use @bucketco/node-sdk 1.6.0-alpha.3 - Bump package version to 0.3.0-alpha.0 --- .../openfeature-browser-provider/README.md | 81 +++- .../src/index.test.ts | 136 +++++-- .../openfeature-browser-provider/src/index.ts | 61 +-- packages/openfeature-node-provider/README.md | 96 ++++- .../openfeature-node-provider/package.json | 4 +- .../src/index.test.ts | 347 ++++++++++++++---- .../openfeature-node-provider/src/index.ts | 143 ++++++-- yarn.lock | 13 +- 8 files changed, 663 insertions(+), 218 deletions(-) diff --git a/packages/openfeature-browser-provider/README.md b/packages/openfeature-browser-provider/README.md index e1774185..f1cced0f 100644 --- a/packages/openfeature-browser-provider/README.md +++ b/packages/openfeature-browser-provider/README.md @@ -15,8 +15,8 @@ The OpenFeature SDK is required as peer dependency. The minimum required version of `@openfeature/web-sdk` currently is `1.0`. -``` -$ npm install @openfeature/web-sdk @bucketco/openfeature-browser-provider +```shell +npm install @openfeature/web-sdk @bucketco/openfeature-browser-provider ``` ## Sample initialization @@ -46,6 +46,59 @@ const feedbackConfig = client.getObjectValue("ask-feedback", { Initializing the Bucket Browser Provider will also initialize [automatic feedback surveys](https://github.com/bucketco/bucket-javascript-sdk/tree/main/packages/browser-sdk#qualitative-feedback). +## Feature resolution methods + +The Bucket OpenFeature Provider implements the OpenFeature evaluation interface for different value types. Each method handles the resolution of feature flags according to the OpenFeature specification. + +### Common behavior + +All resolution methods share these behaviors: + +- Return default value with `PROVIDER_NOT_READY` if client is not initialized, +- Return default value with `FLAG_NOT_FOUND` if flag doesn't exist, +- Return default value with `ERROR` if there was a type mismatch, +- Return evaluated value with `TARGETING_MATCH` on successful resolution. + +### Type-Specific Methods + +#### Boolean Resolution + +```ts +client.getBooleanValue("my-flag", false); +``` + +Returns the feature's enabled state. This is the most common use case for feature flags. + +#### String Resolution + +```ts +client.getStringValue("my-flag", "default"); +``` + +Returns the feature's remote config key (also known as "variant"). Useful for multi-variate use cases. + +#### Number Resolution + +```ts +client.getNumberValue("my-flag", 0); +``` + +Not directly supported by Bucket. Use `getObjectValue` instead for numeric configurations. + +#### Object Resolution + +```ts +// works for any type: +client.getObjectValue("my-flag", { defaultValue: true }); +client.getObjectValue("my-flag", "string-value"); +client.getObjectValue("my-flag", 199); +``` + +Returns the feature's remote config payload with type validation. This is the most flexible method, +allowing for complex configuration objects or simple types. + +The object resolution performs runtime type checking between the default value and the feature payload to ensure type safety. + ## Context To convert the OpenFeature context to a Bucket appropriate context @@ -64,11 +117,18 @@ const publishableKey = ""; const contextTranslator = (context?: EvaluationContext) => { return { user: { - id: context["trackingKey"], - name: context["name"], - email: context["email"], + id: context.targetingKey ?? context["userId"], + email: context["email"]?.toString(), + name: context["name"]?.toString(), + avatar: context["avatar"]?.toString(), + country: context["country"]?.toString(), + }, + company: { + id: context["companyId"], + name: context["companyName"]?.toString(), + avatar: context["companyAvatar"]?.toString(), + plan: context["companyPlan"]?.toString(), }, - company: { id: context["orgId"], name: context["orgName"] }, }; }; @@ -84,7 +144,7 @@ To update the context, call `OpenFeature.setContext(myNewContext);` await OpenFeature.setContext({ userId: "my-key" }); ``` -# Tracking feature usage +## Tracking feature usage The Bucket OpenFeature Provider supports the OpenFeature tracking API natively. @@ -106,8 +166,7 @@ const client = OpenFeature.getClient(); client.track("huddles"); ``` -# License - -MIT License +## License -Copyright (c) 2025 Bucket ApS +> MIT License +> Copyright (c) 2025 Bucket ApS diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index 9f8b4e1f..e6df8ecd 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, Mock, vi } from "vitest"; import { BucketClient } from "@bucketco/browser-sdk"; -import { BucketBrowserSDKProvider } from "."; +import { BucketBrowserSDKProvider, defaultContextTranslator } from "."; vi.mock("@bucketco/browser-sdk", () => { const actualModule = vi.importActual("@bucketco/browser-sdk"); @@ -30,8 +30,8 @@ describe("BucketBrowserSDKProvider", () => { stop: vi.fn(), }; - const newBucketClient = BucketClient as Mock; - newBucketClient.mockReturnValue(bucketClientMock); + const mockBucketClient = BucketClient as Mock; + mockBucketClient.mockReturnValue(bucketClientMock); beforeEach(async () => { await OpenFeature.clearProviders(); @@ -68,6 +68,16 @@ describe("BucketBrowserSDKProvider", () => { await OpenFeature.clearProviders(); expect(bucketClientMock.stop).toHaveBeenCalledTimes(1); }); + + it("onContextChange re-initializes client", async () => { + const p = new BucketBrowserSDKProvider({ publishableKey }); + expect(p["_client"]).toBeUndefined(); + expect(mockBucketClient).toHaveBeenCalledTimes(0); + + await p.onContextChange({}, {}); + expect(mockBucketClient).toHaveBeenCalledTimes(1); + expect(p["_client"]).toBeDefined(); + }); }); describe("contextTranslator", () => { @@ -75,13 +85,26 @@ describe("BucketBrowserSDKProvider", () => { const ofContext = { userId: "123", email: "ron@bucket.co", + avatar: "https://bucket.co/avatar.png", groupId: "456", groupName: "bucket", + groupAvatar: "https://bucket.co/group-avatar.png", + groupPlan: "pro", }; const bucketContext = { - user: { id: "123", name: "John Doe", email: "john@acme.com" }, - company: { id: "456", name: "Acme, Inc." }, + user: { + id: "123", + name: "John Doe", + email: "john@acme.com", + avatar: "https://acme.com/avatar.png", + }, + company: { + id: "456", + name: "Acme, Inc.", + plan: "pro", + avatar: "https://acme.com/company-avatar.png", + }, }; contextTranslatorFn.mockReturnValue(bucketContext); @@ -89,16 +112,61 @@ describe("BucketBrowserSDKProvider", () => { publishableKey, contextTranslator: contextTranslatorFn, }); + await provider.initialize(ofContext); + expect(contextTranslatorFn).toHaveBeenCalledWith(ofContext); - expect(newBucketClient).toHaveBeenCalledWith({ + expect(mockBucketClient).toHaveBeenCalledWith({ publishableKey, ...bucketContext, }); }); + + it("defaultContextTranslator provides the correct context", async () => { + expect( + defaultContextTranslator({ + userId: 123, + name: "John Doe", + email: "ron@bucket.co", + avatar: "https://bucket.co/avatar.png", + companyId: "456", + companyName: "Acme, Inc.", + companyAvatar: "https://acme.com/company-avatar.png", + companyPlan: "pro", + }), + ).toEqual({ + user: { + id: 123, + name: "John Doe", + email: "ron@bucket.co", + avatar: "https://bucket.co/avatar.png", + }, + company: { + id: "456", + name: "Acme, Inc.", + plan: "pro", + avatar: "https://acme.com/company-avatar.png", + }, + }); + }); + + it("defaultContextTranslator uses targetingKey if provided", async () => { + expect( + defaultContextTranslator({ + targetingKey: "123", + }), + ).toMatchObject({ + user: { + id: "123", + }, + company: { + id: undefined, + }, + }); + }); }); - describe("resolveBooleanEvaluation", () => { + describe("resolving flags", () => { beforeEach(async () => { await provider.initialize(); }); @@ -165,6 +233,7 @@ describe("BucketBrowserSDKProvider", () => { flagKey: testFlagKey, flagMetadata: {}, reason: "TARGETING_MATCH", + variant: "key", value: true, }); @@ -174,20 +243,23 @@ describe("BucketBrowserSDKProvider", () => { it.each([ [true, false, true, "TARGETING_MATCH", undefined], - [null, true, true, "ERROR", "FLAG_NOT_FOUND"], - [null, false, false, "ERROR", "FLAG_NOT_FOUND"], + [undefined, true, true, "ERROR", "FLAG_NOT_FOUND"], + [undefined, false, false, "ERROR", "FLAG_NOT_FOUND"], ])( "should return the correct result when evaluating boolean. enabled: %s, value: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", (enabled, def, expected, reason, errorCode) => { - mockFeature(enabled ?? false); + const configKey = enabled !== undefined ? "variant-1" : undefined; const flagKey = enabled ? testFlagKey : "missing-key"; + mockFeature(enabled ?? false, configKey); + expect(ofClient.getBooleanDetails(flagKey, def)).toMatchObject({ flagKey, flagMetadata: {}, reason, value: expected, ...(errorCode ? { errorCode } : {}), + ...(configKey ? { variant: configKey } : {}), }); }, ); @@ -207,37 +279,41 @@ describe("BucketBrowserSDKProvider", () => { [null, "default", "default", "DEFAULT"], [undefined, "default", "default", "DEFAULT"], ])( - "should return the correct result when evaluating string. configKey: %s, def: %s, expected: %s, reason: %s, errorCode: %s`", - (configKey, def, expected, reason) => { - mockFeature(true, configKey, {}); + "should return the correct result when evaluating string. variant: %s, def: %s, expected: %s, reason: %s, errorCode: %s`", + (variant, def, expected, reason) => { + mockFeature(true, variant, {}); expect(ofClient.getStringDetails(testFlagKey, def)).toMatchObject({ flagKey: testFlagKey, flagMetadata: {}, reason, value: expected, + ...(variant ? { variant } : {}), }); }, ); it.each([ - [{}, { a: 1 }, {}, "TARGETING_MATCH", undefined], - ["string", "default", "string", "TARGETING_MATCH", undefined], - [15, -15, 15, "TARGETING_MATCH", undefined], - [true, false, true, "TARGETING_MATCH", undefined], - [null, { a: 2 }, { a: 2 }, "ERROR", "TYPE_MISMATCH"], - [100, "string", "string", "ERROR", "TYPE_MISMATCH"], - [true, 1337, 1337, "ERROR", "TYPE_MISMATCH"], - ["string", 1337, 1337, "ERROR", "TYPE_MISMATCH"], + ["one", {}, { a: 1 }, {}, "TARGETING_MATCH", undefined], + ["two", "string", "default", "string", "TARGETING_MATCH", undefined], + ["three", 15, 16, 15, "TARGETING_MATCH", undefined], + ["four", true, true, true, "TARGETING_MATCH", undefined], + ["five", 100, "string", "string", "ERROR", "TYPE_MISMATCH"], + ["six", 1337, true, true, "ERROR", "TYPE_MISMATCH"], + ["seven", "string", 1337, 1337, "ERROR", "TYPE_MISMATCH"], + [undefined, null, { a: 2 }, { a: 2 }, "ERROR", "TYPE_MISMATCH"], + [undefined, undefined, "a", "a", "ERROR", "TYPE_MISMATCH"], ])( - "should return the correct result when evaluating object. payload: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", - (value, def, expected, reason, errorCode) => { - mockFeature(true, "config-key", value); + "should return the correct result when evaluating object. variant: %s, value: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", + (variant, value, def, expected, reason, errorCode) => { + mockFeature(true, variant, value); + expect(ofClient.getObjectDetails(testFlagKey, def)).toMatchObject({ flagKey: testFlagKey, flagMetadata: {}, reason, value: expected, ...(errorCode ? { errorCode } : {}), + ...(variant && !errorCode ? { variant } : {}), }); }, ); @@ -255,16 +331,4 @@ describe("BucketBrowserSDKProvider", () => { }); }); }); - - describe("onContextChange", () => { - it("re-initialize client", async () => { - const p = new BucketBrowserSDKProvider({ publishableKey }); - expect(p["_client"]).toBeUndefined(); - expect(newBucketClient).toHaveBeenCalledTimes(0); - - await p.onContextChange({}, {}); - expect(newBucketClient).toHaveBeenCalledTimes(1); - expect(p["_client"]).toBeDefined(); - }); - }); }); diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index a63558fa..bc7baca9 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -13,26 +13,27 @@ import { import { BucketClient, Feature, InitOptions } from "@bucketco/browser-sdk"; -// eslint-disable-next-line @typescript-eslint/no-explicit-any export type ContextTranslationFn = ( context?: EvaluationContext, ) => Record; -// eslint-disable-next-line @typescript-eslint/no-explicit-any export function defaultContextTranslator( context?: EvaluationContext, ): Record { if (!context) return {}; return { user: { - id: context["trackingKey"], - email: context["email"], - name: context["name"], + id: context.targetingKey ?? context["userId"], + email: context["email"]?.toString(), + name: context["name"]?.toString(), + avatar: context["avatar"]?.toString(), + country: context["country"]?.toString(), }, company: { id: context["companyId"], - name: context["companyName"], - plan: context["companyPlan"], + name: context["companyName"]?.toString(), + plan: context["companyPlan"]?.toString(), + avatar: context["companyAvatar"]?.toString(), }, }; } @@ -131,12 +132,13 @@ export class BucketBrowserSDKProvider implements Provider { return this.resolveFeature(flagKey, defaultValue, (feature) => { return { value: feature.isEnabled, + variant: feature.config.key, reason: StandardResolutionReasons.TARGETING_MATCH, }; }); } - resolveNumberEvaluation(_: string, defaultValue: number) { + resolveNumberEvaluation(_flagKey: string, defaultValue: number) { return { value: defaultValue, reason: StandardResolutionReasons.ERROR, @@ -146,46 +148,51 @@ export class BucketBrowserSDKProvider implements Provider { }; } - resolveObjectEvaluation( + resolveStringEvaluation( flagKey: string, - defaultValue: T, - ) { + defaultValue: string, + ): ResolutionDetails { return this.resolveFeature(flagKey, defaultValue, (feature) => { - const expType = typeof defaultValue; - const payload = feature.config?.payload; - - const payloadType = payload === null ? "null" : typeof payload; - - if (payloadType !== expType) { + if (!feature.config.key) { return { value: defaultValue, - reason: StandardResolutionReasons.ERROR, - errorCode: ErrorCode.TYPE_MISMATCH, - errorMessage: `Expected remote config payload of type \`${expType}\` but got \`${payloadType}\`.`, + reason: StandardResolutionReasons.DEFAULT, }; } return { - value: payload, + value: feature.config.key as string, + variant: feature.config.key, reason: StandardResolutionReasons.TARGETING_MATCH, }; }); } - resolveStringEvaluation( + resolveObjectEvaluation( flagKey: string, - defaultValue: string, - ): ResolutionDetails { + defaultValue: T, + ) { return this.resolveFeature(flagKey, defaultValue, (feature) => { - if (!feature.config.key) { + const expType = typeof defaultValue; + + const payloadType = + feature.config.payload === null + ? "null" + : typeof feature.config.payload; + + if (payloadType !== expType) { return { value: defaultValue, - reason: StandardResolutionReasons.DEFAULT, + reason: StandardResolutionReasons.ERROR, + variant: feature.config.key, + errorCode: ErrorCode.TYPE_MISMATCH, + errorMessage: `Expected remote config payload of type \`${expType}\` but got \`${payloadType}\`.`, }; } return { - value: feature.config.key as string, + value: feature.config.payload, + variant: feature.config.key, reason: StandardResolutionReasons.TARGETING_MATCH, }; }); diff --git a/packages/openfeature-node-provider/README.md b/packages/openfeature-node-provider/README.md index f9183534..75699165 100644 --- a/packages/openfeature-node-provider/README.md +++ b/packages/openfeature-node-provider/README.md @@ -4,26 +4,23 @@ The official OpenFeature Node.js provider for [Bucket](https://bucket.co) featur ## Installation -``` -$ npm install @bucketco/openfeature-node-provider +```shell +npm install @bucketco/openfeature-node-provider ``` -#### Required peer dependencies +### Required peer dependencies The OpenFeature SDK is required as peer dependency. - The minimum required version of `@openfeature/server-sdk` currently is `1.13.5`. - The minimum required version of `@bucketco/node-sdk` currently is `2.0.0`. -``` -$ npm install @openfeature/server-sdk @bucketco/node-sdk +```shell +npm install @openfeature/server-sdk @bucketco/node-sdk ``` ## Usage The provider uses the [Bucket Node.js SDK](https://docs.bucket.co/quickstart/supported-languages-frameworks/node.js-sdk). - The available options can be found in the [Bucket Node.js SDK](https://github.com/bucketco/bucket-javascript-sdk/tree/main/packages/node-sdk#initialization-options). ### Example using the default configuration @@ -56,6 +53,59 @@ const enterpriseFeatureEnabled = await client.getBooleanValue( ); ``` +## Feature resolution methods + +The Bucket OpenFeature Provider implements the OpenFeature evaluation interface for different value types. Each method handles the resolution of feature flags according to the OpenFeature specification. + +### Common behavior + +All resolution methods share these behaviors: + +- Return default value with `PROVIDER_NOT_READY` if client is not initialized, +- Return default value with `FLAG_NOT_FOUND` if flag doesn't exist, +- Return default value with `ERROR` if there was a type mismatch, +- Return evaluated value with `TARGETING_MATCH` on successful resolution. + +### Type-Specific Methods + +#### Boolean Resolution + +```ts +client.getBooleanValue("my-flag", false); +``` + +Returns the feature's enabled state. This is the most common use case for feature flags. + +#### String Resolution + +```ts +client.getStringValue("my-flag", "default"); +``` + +Returns the feature's remote config key (also known as "variant"). Useful for multi-variate use cases. + +#### Number Resolution + +```ts +client.getNumberValue("my-flag", 0); +``` + +Not directly supported by Bucket. Use `getObjectValue` instead for numeric configurations. + +#### Object Resolution + +```ts +// works for any type: +client.getObjectValue("my-flag", { defaultValue: true }); +client.getObjectValue("my-flag", "string-value"); +client.getObjectValue("my-flag", 199); +``` + +Returns the feature's remote config payload with type validation. This is the most flexible method, +allowing for complex configuration objects or simple types. + +The object resolution performs runtime type checking between the default value and the feature payload to ensure type safety. + ## Translating Evaluation Context Bucket uses a context object of the following shape: @@ -69,11 +119,19 @@ export type BucketContext = { /** * The user context. If the user is set, the user ID is required. */ - user?: { id: string; [k: string]: any }; + user?: { + id: string; + name?: string; + email?: string; + avatar?: string; + [k: string]: any; + }; + /** * The company context. If the company is set, the company ID is required. */ - company?: { id: string; [k: string]: any }; + company?: { id: string; name?: string; avatar?: string; [k: string]: any }; + /** * The other context. This is used for any additional context that is not related to user or company. */ @@ -91,14 +149,17 @@ import { BucketNodeProvider } from "@openfeature/bucket-node-provider"; const contextTranslator = (context: EvaluationContext): BucketContext => { return { user: { - id: context.targetingKey, + id: context.targetingKey ?? context["userId"]?.toString(), name: context["name"]?.toString(), email: context["email"]?.toString(), + avatar: context["avatar"]?.toString(), country: context["country"]?.toString(), }, company: { - id: context["companyId"], - name: context["companyName"], + id: context["companyId"]?.toString(), + name: context["companyName"]?.toString(), + avatar: context["companyAvatar"]?.toString(), + plan: context["companyPlan"]?.toString(), }, }; }; @@ -115,7 +176,7 @@ It's straight forward to start sending tracking events through OpenFeature. Simply call the "track" method on the OpenFeature client: -```ts +```typescript import { BucketNodeProvider } from "@bucketco/openfeature-node-provider"; import { OpenFeature } from "@openfeature/server-sdk"; @@ -132,8 +193,7 @@ const enterpriseFeatureEnabled = await client.track( ); ``` -# License - -MIT License +## License -Copyright (c) 2025 Bucket ApS +> MIT License +> Copyright (c) 2025 Bucket ApS diff --git a/packages/openfeature-node-provider/package.json b/packages/openfeature-node-provider/package.json index 06eca2f5..5cf1a21a 100644 --- a/packages/openfeature-node-provider/package.json +++ b/packages/openfeature-node-provider/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/openfeature-node-provider", - "version": "0.2.1", + "version": "0.3.0-alpha.0", "license": "MIT", "repository": { "type": "git", @@ -44,7 +44,7 @@ "vitest": "~1.6.0" }, "dependencies": { - "@bucketco/node-sdk": ">=1.4.2" + "@bucketco/node-sdk": "1.6.0-alpha.3" }, "peerDependencies": { "@openfeature/server-sdk": ">=1.16.1" diff --git a/packages/openfeature-node-provider/src/index.test.ts b/packages/openfeature-node-provider/src/index.test.ts index 8f506df1..73956ea3 100644 --- a/packages/openfeature-node-provider/src/index.test.ts +++ b/packages/openfeature-node-provider/src/index.test.ts @@ -1,9 +1,9 @@ -import { ErrorCode } from "@openfeature/core"; -import { beforeAll, beforeEach, describe, expect, it, Mock, vi } from "vitest"; +import { ProviderStatus } from "@openfeature/server-sdk"; +import { beforeEach, describe, expect, it, Mock, vi } from "vitest"; import { BucketClient } from "@bucketco/node-sdk"; -import { BucketNodeProvider } from "./index"; +import { BucketNodeProvider, defaultContextTranslator } from "./index"; vi.mock("@bucketco/node-sdk", () => { const actualModule = vi.importActual("@bucketco/node-sdk"); @@ -17,6 +17,7 @@ vi.mock("@bucketco/node-sdk", () => { const bucketClientMock = { getFeatures: vi.fn(), + getFeature: vi.fn(), initialize: vi.fn().mockResolvedValue({}), flush: vi.fn(), track: vi.fn(), @@ -35,6 +36,8 @@ const bucketContext = { company: { id: "99" }, }; +const testFlagKey = "a-key"; + beforeEach(() => { vi.clearAllMocks(); }); @@ -42,121 +45,305 @@ beforeEach(() => { describe("BucketNodeProvider", () => { let provider: BucketNodeProvider; - const newBucketClient = BucketClient as Mock; - newBucketClient.mockReturnValue(bucketClientMock); + const mockBucketClient = BucketClient as Mock; + mockBucketClient.mockReturnValue(bucketClientMock); + + let mockTranslatorFn: Mock; + + function mockFeature( + enabled: boolean, + configKey?: string | null, + configPayload?: any, + ) { + const config = { + key: configKey, + payload: configPayload, + }; + + bucketClientMock.getFeature = vi.fn().mockReturnValue({ + isEnabled: enabled, + config, + }); + + bucketClientMock.getFeatures = vi.fn().mockReturnValue({ + [testFlagKey]: { + isEnabled: enabled, + config: { + key: "key", + payload: configPayload, + }, + }, + }); + } - const translatorFn = vi.fn().mockReturnValue(bucketContext); + beforeEach(async () => { + mockTranslatorFn = vi.fn().mockReturnValue(bucketContext); - beforeAll(async () => { provider = new BucketNodeProvider({ secretKey, - contextTranslator: translatorFn, + contextTranslator: mockTranslatorFn, }); + await provider.initialize(); }); - it("calls the constructor", () => { - provider = new BucketNodeProvider({ - secretKey, - contextTranslator: translatorFn, + describe("contextTranslator", () => { + it("defaultContextTranslator provides the correct context", async () => { + expect( + defaultContextTranslator({ + userId: 123, + name: "John Doe", + email: "ron@bucket.co", + avatar: "https://bucket.co/avatar.png", + companyId: "456", + companyName: "Acme, Inc.", + companyAvatar: "https://acme.com/company-avatar.png", + companyPlan: "pro", + }), + ).toEqual({ + user: { + id: 123, + name: "John Doe", + email: "ron@bucket.co", + avatar: "https://bucket.co/avatar.png", + }, + company: { + id: "456", + name: "Acme, Inc.", + plan: "pro", + avatar: "https://acme.com/company-avatar.png", + }, + }); + }); + + it("defaultContextTranslator uses targetingKey if provided", async () => { + expect( + defaultContextTranslator({ + targetingKey: "123", + }), + ).toMatchObject({ + user: { + id: "123", + }, + company: { + id: undefined, + }, + }); }); - expect(newBucketClient).toHaveBeenCalledTimes(1); - expect(newBucketClient).toHaveBeenCalledWith({ secretKey }); }); - it("uses the contextTranslator function", async () => { - const track = vi.fn(); - bucketClientMock.getFeatures.mockReturnValue({ - booleanTrue: { - isEnabled: true, - key: "booleanTrue", - track, - }, + describe("lifecycle", () => { + it("calls the constructor of BucketClient", () => { + mockBucketClient.mockClear(); + + provider = new BucketNodeProvider({ + secretKey, + contextTranslator: mockTranslatorFn, + }); + + expect(mockBucketClient).toHaveBeenCalledTimes(1); + expect(mockBucketClient).toHaveBeenCalledWith({ secretKey }); }); - await provider.resolveBooleanEvaluation("booleanTrue", false, context); - expect(translatorFn).toHaveBeenCalledTimes(1); - expect(translatorFn).toHaveBeenCalledWith(context); - expect(bucketClientMock.getFeatures).toHaveBeenCalledTimes(1); - expect(bucketClientMock.getFeatures).toHaveBeenCalledWith(bucketContext); + it("should set the status to READY if initialization succeeds", async () => { + provider = new BucketNodeProvider({ + secretKey, + contextTranslator: mockTranslatorFn, + }); + + await provider.initialize(); + + expect(provider.status).toBe(ProviderStatus.READY); + }); + + it("should keep the status as READY after closing", async () => { + provider = new BucketNodeProvider({ + secretKey: "invalid", + contextTranslator: mockTranslatorFn, + }); + + await provider.initialize(); + await provider.onClose(); + + expect(provider.status).toBe(ProviderStatus.READY); + }); + + it("calls flush when provider is closed", async () => { + await provider.onClose(); + expect(bucketClientMock.flush).toHaveBeenCalledTimes(1); + }); + + it("uses the contextTranslator function", async () => { + mockFeature(true); + + await provider.resolveBooleanEvaluation(testFlagKey, false, context); + + expect(mockTranslatorFn).toHaveBeenCalledTimes(1); + expect(mockTranslatorFn).toHaveBeenCalledWith(context); + + expect(bucketClientMock.getFeatures).toHaveBeenCalledTimes(1); + expect(bucketClientMock.getFeatures).toHaveBeenCalledWith(bucketContext); + }); }); - describe("method resolveBooleanEvaluation", () => { - it("should return right value if key exists", async () => { - const result = await provider.resolveBooleanEvaluation( - "booleanTrue", - false, - context, - ); - expect(result.value).toEqual(true); - expect(result.errorCode).toBeUndefined(); + describe("resolving flags", () => { + beforeEach(async () => { + await provider.initialize(); }); - it("should return the default value if key does not exists", async () => { - const result = await provider.resolveBooleanEvaluation( - "non-existent", + it("returns error if provider is not initialized", async () => { + provider = new BucketNodeProvider({ + secretKey: "invalid", + contextTranslator: mockTranslatorFn, + }); + + const val = await provider.resolveBooleanEvaluation( + testFlagKey, true, context, ); - expect(result.value).toEqual(true); - expect(result.errorCode).toEqual(ErrorCode.FLAG_NOT_FOUND); + + expect(val).toMatchObject({ + reason: "ERROR", + errorCode: "PROVIDER_NOT_READY", + value: true, + }); }); - }); - describe("method resolveNumberEvaluation", () => { - it("should return the default value and an error message", async () => { - const result = await provider.resolveNumberEvaluation("number1", 42); - expect(result.value).toEqual(42); - expect(result.errorCode).toEqual(ErrorCode.GENERAL); - expect(result.errorMessage).toEqual( - `Bucket doesn't support number flags`, + it("returns error if flag is not found", async () => { + mockFeature(true, "key", true); + const val = await provider.resolveBooleanEvaluation( + "missing-key", + true, + context, ); + + expect(val).toMatchObject({ + reason: "ERROR", + errorCode: "FLAG_NOT_FOUND", + value: true, + }); }); - }); - describe("method resolveStringEvaluation", () => { - it("should return the default value and an error message", async () => { - const result = await provider.resolveStringEvaluation( - "number1", - "defaultValue", + it("calls the client correctly when evaluating", async () => { + mockFeature(true, "key", true); + + const val = await provider.resolveBooleanEvaluation( + testFlagKey, + false, + context, ); - expect(result.value).toEqual("defaultValue"); - expect(result.errorCode).toEqual(ErrorCode.GENERAL); - expect(result.errorMessage).toEqual( - `Bucket doesn't support string flags`, + + expect(val).toMatchObject({ + reason: "TARGETING_MATCH", + value: true, + }); + + expect(bucketClientMock.getFeatures).toHaveBeenCalled(); + expect(bucketClientMock.getFeature).toHaveBeenCalledWith( + bucketContext, + testFlagKey, ); }); - }); - describe("method resolveObjectEvaluation", () => { - it("should return the default value and an error message", async () => { - const defaultValue = { key: "value" }; - const result = await provider.resolveObjectEvaluation( - "number1", - defaultValue, - ); - expect(result.value).toEqual(defaultValue); - expect(result.errorCode).toEqual(ErrorCode.GENERAL); - expect(result.errorMessage).toEqual( - `Bucket doesn't support object flags`, - ); + + it.each([ + [true, false, true, "TARGETING_MATCH", undefined], + [undefined, true, true, "ERROR", "FLAG_NOT_FOUND"], + [undefined, false, false, "ERROR", "FLAG_NOT_FOUND"], + ])( + "should return the correct result when evaluating boolean. enabled: %s, value: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", + async (enabled, def, expected, reason, errorCode) => { + const configKey = enabled !== undefined ? "variant-1" : undefined; + + mockFeature(enabled ?? false, configKey); + const flagKey = enabled ? testFlagKey : "missing-key"; + + expect( + await provider.resolveBooleanEvaluation(flagKey, def, context), + ).toMatchObject({ + reason, + value: expected, + ...(configKey ? { variant: configKey } : {}), + ...(errorCode ? { errorCode } : {}), + }); + }, + ); + + it("should return error when context is missing user ID", async () => { + mockTranslatorFn.mockReturnValue({ user: {} }); + + expect( + await provider.resolveBooleanEvaluation(testFlagKey, true, context), + ).toMatchObject({ + reason: "ERROR", + errorCode: "INVALID_CONTEXT", + value: true, + }); }); - }); - describe("onClose", () => { - it("calls flush", async () => { - await provider.onClose(); - expect(bucketClientMock.flush).toHaveBeenCalledTimes(1); + it("should return error when evaluating number", async () => { + expect( + await provider.resolveNumberEvaluation(testFlagKey, 1), + ).toMatchObject({ + reason: "ERROR", + errorCode: "GENERAL", + value: 1, + }); }); + + it.each([ + ["key-1", "default", "key-1", "TARGETING_MATCH"], + [null, "default", "default", "DEFAULT"], + [undefined, "default", "default", "DEFAULT"], + ])( + "should return the correct result when evaluating string. variant: %s, def: %s, expected: %s, reason: %s, errorCode: %s`", + async (variant, def, expected, reason) => { + mockFeature(true, variant, {}); + expect( + await provider.resolveStringEvaluation(testFlagKey, def, context), + ).toMatchObject({ + reason, + value: expected, + ...(variant ? { variant } : {}), + }); + }, + ); + + it.each([ + [{}, { a: 1 }, {}, "TARGETING_MATCH", undefined], + ["string", "default", "string", "TARGETING_MATCH", undefined], + [15, -15, 15, "TARGETING_MATCH", undefined], + [true, false, true, "TARGETING_MATCH", undefined], + [null, { a: 2 }, { a: 2 }, "ERROR", "TYPE_MISMATCH"], + [100, "string", "string", "ERROR", "TYPE_MISMATCH"], + [true, 1337, 1337, "ERROR", "TYPE_MISMATCH"], + ["string", 1337, 1337, "ERROR", "TYPE_MISMATCH"], + [undefined, "default", "default", "ERROR", "TYPE_MISMATCH"], + ])( + "should return the correct result when evaluating object. payload: %s, default: %s, expected: %s, reason: %s, errorCode: %s`", + async (value, def, expected, reason, errorCode) => { + const configKey = value === undefined ? undefined : "config-key"; + mockFeature(true, configKey, value); + expect( + await provider.resolveObjectEvaluation(testFlagKey, def, context), + ).toMatchObject({ + reason, + value: expected, + ...(errorCode ? { errorCode, variant: configKey } : {}), + }); + }, + ); }); describe("track", () => { it("should track", async () => { - expect(translatorFn).toHaveBeenCalledTimes(0); + expect(mockTranslatorFn).toHaveBeenCalledTimes(0); provider.track("event", context, { action: "click", }); - expect(translatorFn).toHaveBeenCalledTimes(1); - expect(translatorFn).toHaveBeenCalledWith(context); + + expect(mockTranslatorFn).toHaveBeenCalledTimes(1); + expect(mockTranslatorFn).toHaveBeenCalledWith(context); expect(bucketClientMock.track).toHaveBeenCalledTimes(1); expect(bucketClientMock.track).toHaveBeenCalledWith("42", "event", { attributes: { action: "click" }, diff --git a/packages/openfeature-node-provider/src/index.ts b/packages/openfeature-node-provider/src/index.ts index 7f2f512a..c8e5a06a 100644 --- a/packages/openfeature-node-provider/src/index.ts +++ b/packages/openfeature-node-provider/src/index.ts @@ -21,17 +21,22 @@ type ProviderOptions = ClientOptions & { contextTranslator?: (context: EvaluationContext) => BucketContext; }; -const defaultTranslator = (context: EvaluationContext): BucketContext => { +export const defaultContextTranslator = ( + context: EvaluationContext, +): BucketContext => { const user = { - id: context.targetingKey ?? context["id"]?.toString(), + id: context.targetingKey ?? context["userId"], name: context["name"]?.toString(), email: context["email"]?.toString(), + avatar: context["avatar"]?.toString(), country: context["country"]?.toString(), }; const company = { - id: context["companyId"]?.toString(), + id: context["companyId"], name: context["companyName"]?.toString(), + avatar: context["companyAvatar"]?.toString(), + plan: context["companyPlan"]?.toString(), }; return { @@ -61,7 +66,7 @@ export class BucketNodeProvider implements Provider { constructor({ contextTranslator, ...opts }: ProviderOptions) { this._client = new BucketClient(opts); - this.contextTranslator = contextTranslator ?? defaultTranslator; + this.contextTranslator = contextTranslator ?? defaultContextTranslator; } public async initialize(): Promise { @@ -69,42 +74,90 @@ export class BucketNodeProvider implements Provider { this.status = ServerProviderStatus.READY; } - resolveBooleanEvaluation( + private resolveFeature( flagKey: string, - defaultValue: boolean, - context: EvaluationContext, - ): Promise> { - const features = this._client.getFeatures(this.contextTranslator(context)); + defaultValue: T, + context: BucketContext, + resolveFn: ( + feature: ReturnType, + ) => Promise>, + ): Promise> { + if (this.status !== ServerProviderStatus.READY) { + return Promise.resolve({ + value: defaultValue, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.PROVIDER_NOT_READY, + errorMessage: "Bucket client not initialized", + }); + } - const feature = features[flagKey]; - if (!feature) { + if (!context.user?.id) { return Promise.resolve({ value: defaultValue, - source: "bucket-node", - flagKey, - errorCode: ErrorCode.FLAG_NOT_FOUND, reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.INVALID_CONTEXT, + errorMessage: "At least a user ID is required", }); } + const features = this._client.getFeatures(context); + if (flagKey in features) { + return resolveFn(this._client.getFeature(context, flagKey)); + } + return Promise.resolve({ - value: feature.isEnabled, - source: "bucket-node", - flagKey, - reason: StandardResolutionReasons.TARGETING_MATCH, + value: defaultValue, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.FLAG_NOT_FOUND, + errorMessage: `Flag ${flagKey} not found`, }); } + + resolveBooleanEvaluation( + flagKey: string, + defaultValue: boolean, + context: EvaluationContext, + ): Promise> { + return this.resolveFeature( + flagKey, + defaultValue, + this.contextTranslator(context), + (feature) => { + return Promise.resolve({ + value: feature.isEnabled, + variant: feature.config?.key, + reason: StandardResolutionReasons.TARGETING_MATCH, + }); + }, + ); + } + resolveStringEvaluation( - _flagKey: string, + flagKey: string, defaultValue: string, + context: EvaluationContext, ): Promise> { - return Promise.resolve({ - value: defaultValue, - reason: StandardResolutionReasons.ERROR, - errorCode: ErrorCode.GENERAL, - errorMessage: "Bucket doesn't support string flags", - }); + return this.resolveFeature( + flagKey, + defaultValue, + this.contextTranslator(context), + (feature) => { + if (!feature.config.key) { + return Promise.resolve({ + value: defaultValue, + reason: StandardResolutionReasons.DEFAULT, + }); + } + + return Promise.resolve({ + value: feature.config.key as string, + variant: feature.config.key, + reason: StandardResolutionReasons.TARGETING_MATCH, + }); + }, + ); } + resolveNumberEvaluation( _flagKey: string, defaultValue: number, @@ -113,19 +166,43 @@ export class BucketNodeProvider implements Provider { value: defaultValue, reason: StandardResolutionReasons.ERROR, errorCode: ErrorCode.GENERAL, - errorMessage: "Bucket doesn't support number flags", + errorMessage: + "Bucket doesn't support this method. Use `resolveObjectEvaluation` instead.", }); } + resolveObjectEvaluation( - _flagKey: string, + flagKey: string, defaultValue: T, + context: EvaluationContext, ): Promise> { - return Promise.resolve({ - value: defaultValue, - reason: StandardResolutionReasons.ERROR, - errorCode: ErrorCode.GENERAL, - errorMessage: "Bucket doesn't support object flags", - }); + return this.resolveFeature( + flagKey, + defaultValue, + this.contextTranslator(context), + (feature) => { + const expType = typeof defaultValue; + const payload = feature.config.payload; + + const payloadType = payload === null ? "null" : typeof payload; + + if (payloadType !== expType) { + return Promise.resolve({ + value: defaultValue, + variant: feature.config.key, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.TYPE_MISMATCH, + errorMessage: `Expected remote config payload of type \`${expType}\` but got \`${payloadType}\`.`, + }); + } + + return Promise.resolve({ + value: payload, + variant: feature.config.key, + reason: StandardResolutionReasons.TARGETING_MATCH, + }); + }, + ); } track( diff --git a/yarn.lock b/yarn.lock index 4db77033..2200fab5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -946,16 +946,7 @@ __metadata: languageName: unknown linkType: soft -"@bucketco/node-sdk@npm:>=1.4.2": - version: 1.5.0 - resolution: "@bucketco/node-sdk@npm:1.5.0" - dependencies: - "@bucketco/flag-evaluation": "npm:~0.1.0" - checksum: 10c0/63230400c0c0fa6ccf8708550bbcf583cc58bd18a2b99e19ec1dde43bce593c43136790ff3f0573f171c123c6a0555eebafcefdfa5cc71a2e706079fdb1ebe39 - languageName: node - linkType: hard - -"@bucketco/node-sdk@workspace:packages/node-sdk": +"@bucketco/node-sdk@npm:1.6.0-alpha.3, @bucketco/node-sdk@workspace:packages/node-sdk": version: 0.0.0-use.local resolution: "@bucketco/node-sdk@workspace:packages/node-sdk" dependencies: @@ -1005,7 +996,7 @@ __metadata: dependencies: "@babel/core": "npm:~7.24.7" "@bucketco/eslint-config": "npm:~0.0.2" - "@bucketco/node-sdk": "npm:>=1.4.2" + "@bucketco/node-sdk": "npm:1.6.0-alpha.3" "@bucketco/tsconfig": "npm:~0.0.2" "@openfeature/core": "npm:^1.5.0" "@openfeature/server-sdk": "npm:>=1.16.1" From 4dd9deacfa971b4be3802ed44ef49dfbf445495f Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Sat, 8 Feb 2025 13:58:07 +0000 Subject: [PATCH 31/34] fix(openfeature-providers): Ensure consistent type conversion for user and company IDs - Convert user and company IDs to strings in both browser and node providers - Update test cases to reflect string-based ID handling - Maintain consistent context translation across providers --- packages/openfeature-browser-provider/src/index.test.ts | 2 +- packages/openfeature-browser-provider/src/index.ts | 4 ++-- packages/openfeature-node-provider/src/index.test.ts | 2 +- packages/openfeature-node-provider/src/index.ts | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/openfeature-browser-provider/src/index.test.ts b/packages/openfeature-browser-provider/src/index.test.ts index e6df8ecd..4cf03ec7 100644 --- a/packages/openfeature-browser-provider/src/index.test.ts +++ b/packages/openfeature-browser-provider/src/index.test.ts @@ -136,7 +136,7 @@ describe("BucketBrowserSDKProvider", () => { }), ).toEqual({ user: { - id: 123, + id: "123", name: "John Doe", email: "ron@bucket.co", avatar: "https://bucket.co/avatar.png", diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index bc7baca9..2e85bd9d 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -23,14 +23,14 @@ export function defaultContextTranslator( if (!context) return {}; return { user: { - id: context.targetingKey ?? context["userId"], + id: context.targetingKey ?? context["userId"]?.toString(), email: context["email"]?.toString(), name: context["name"]?.toString(), avatar: context["avatar"]?.toString(), country: context["country"]?.toString(), }, company: { - id: context["companyId"], + id: context["companyId"]?.toString(), name: context["companyName"]?.toString(), plan: context["companyPlan"]?.toString(), avatar: context["companyAvatar"]?.toString(), diff --git a/packages/openfeature-node-provider/src/index.test.ts b/packages/openfeature-node-provider/src/index.test.ts index 73956ea3..9d2b63b9 100644 --- a/packages/openfeature-node-provider/src/index.test.ts +++ b/packages/openfeature-node-provider/src/index.test.ts @@ -102,7 +102,7 @@ describe("BucketNodeProvider", () => { }), ).toEqual({ user: { - id: 123, + id: "123", name: "John Doe", email: "ron@bucket.co", avatar: "https://bucket.co/avatar.png", diff --git a/packages/openfeature-node-provider/src/index.ts b/packages/openfeature-node-provider/src/index.ts index c8e5a06a..10991e02 100644 --- a/packages/openfeature-node-provider/src/index.ts +++ b/packages/openfeature-node-provider/src/index.ts @@ -25,7 +25,7 @@ export const defaultContextTranslator = ( context: EvaluationContext, ): BucketContext => { const user = { - id: context.targetingKey ?? context["userId"], + id: context.targetingKey ?? context["userId"]?.toString(), name: context["name"]?.toString(), email: context["email"]?.toString(), avatar: context["avatar"]?.toString(), @@ -33,7 +33,7 @@ export const defaultContextTranslator = ( }; const company = { - id: context["companyId"], + id: context["companyId"]?.toString(), name: context["companyName"]?.toString(), avatar: context["companyAvatar"]?.toString(), plan: context["companyPlan"]?.toString(), From 98734c492182783fd4a853bf965cf3ea70735c67 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Sat, 8 Feb 2025 14:15:41 +0000 Subject: [PATCH 32/34] fix(openfeature-providers): Improve type checking and payload resolution in browser and node providers - Update type checking logic to handle undefined and null payloads - Ensure consistent feature resolution across browser and node providers - Simplify payload type detection and comparison --- packages/openfeature-browser-provider/src/index.ts | 11 ++++++----- packages/openfeature-node-provider/src/index.ts | 12 +++++++----- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/openfeature-browser-provider/src/index.ts b/packages/openfeature-browser-provider/src/index.ts index 2e85bd9d..9184c4fd 100644 --- a/packages/openfeature-browser-provider/src/index.ts +++ b/packages/openfeature-browser-provider/src/index.ts @@ -175,12 +175,13 @@ export class BucketBrowserSDKProvider implements Provider { return this.resolveFeature(flagKey, defaultValue, (feature) => { const expType = typeof defaultValue; - const payloadType = - feature.config.payload === null - ? "null" - : typeof feature.config.payload; + const payloadType = typeof feature.config.payload; - if (payloadType !== expType) { + if ( + feature.config.payload === undefined || + feature.config.payload === null || + payloadType !== expType + ) { return { value: defaultValue, reason: StandardResolutionReasons.ERROR, diff --git a/packages/openfeature-node-provider/src/index.ts b/packages/openfeature-node-provider/src/index.ts index 10991e02..ad771095 100644 --- a/packages/openfeature-node-provider/src/index.ts +++ b/packages/openfeature-node-provider/src/index.ts @@ -182,11 +182,13 @@ export class BucketNodeProvider implements Provider { this.contextTranslator(context), (feature) => { const expType = typeof defaultValue; - const payload = feature.config.payload; + const payloadType = typeof feature.config.payload; - const payloadType = payload === null ? "null" : typeof payload; - - if (payloadType !== expType) { + if ( + feature.config.payload === undefined || + feature.config.payload === null || + payloadType !== expType + ) { return Promise.resolve({ value: defaultValue, variant: feature.config.key, @@ -197,7 +199,7 @@ export class BucketNodeProvider implements Provider { } return Promise.resolve({ - value: payload, + value: feature.config.payload, variant: feature.config.key, reason: StandardResolutionReasons.TARGETING_MATCH, }); From 5fd428076564bd3ba245d1cd149eca885324cf7f Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 12 Feb 2025 14:07:51 +0100 Subject: [PATCH 33/34] feat(node-sdk): improved flag events (#307) This PR improves the handling of check/evaluate events in the age of `remote config`. Fixes some additional small nagging issues along the way. --- docs.sh | 43 +- packages/browser-sdk/README.md | 47 +-- packages/browser-sdk/example/browser.html | 43 ++ .../browser-sdk/example/typescript/app.ts | 51 --- .../browser-sdk/example/typescript/index.html | 23 - packages/browser-sdk/index.html | 71 ---- packages/browser-sdk/package.json | 2 +- packages/browser-sdk/src/client.ts | 171 ++------ packages/browser-sdk/src/config.ts | 1 - .../browser-sdk/src/feature/featureCache.ts | 23 +- packages/browser-sdk/src/feature/features.ts | 239 ++--------- packages/browser-sdk/src/feedback/feedback.ts | 11 +- .../src/feedback/ui/FeedbackDialog.css | 99 +++++ .../src/feedback/ui/FeedbackDialog.tsx | 244 +++++++++-- .../src/feedback/ui/FeedbackForm.tsx | 5 +- packages/browser-sdk/src/feedback/ui/Plug.tsx | 4 +- .../src/feedback/ui/StarRating.tsx | 17 +- .../ui/config/defaultTranslations.tsx | 1 + .../src/{ => feedback}/ui/constants.ts | 1 - .../src/{ => feedback}/ui/icons/Check.tsx | 0 .../{ => feedback}/ui/icons/CheckCircle.tsx | 0 .../src/{ => feedback}/ui/icons/Close.tsx | 0 .../{ => feedback}/ui/icons/Dissatisfied.tsx | 0 .../src/{ => feedback}/ui/icons/Logo.tsx | 4 +- .../src/{ => feedback}/ui/icons/Neutral.tsx | 0 .../src/{ => feedback}/ui/icons/Satisfied.tsx | 0 .../ui/icons/VeryDissatisfied.tsx | 0 .../{ => feedback}/ui/icons/VerySatisfied.tsx | 0 packages/browser-sdk/src/feedback/ui/index.ts | 24 +- .../packages/floating-ui-preact-dom/README.md | 0 .../packages/floating-ui-preact-dom/arrow.ts | 0 .../packages/floating-ui-preact-dom/index.ts | 0 .../packages/floating-ui-preact-dom/types.ts | 0 .../floating-ui-preact-dom/useFloating.ts | 0 .../floating-ui-preact-dom/utils/deepEqual.ts | 0 .../floating-ui-preact-dom/utils/getDPR.ts | 0 .../utils/roundByDPR.ts | 0 .../utils/useLatestRef.ts | 0 packages/browser-sdk/src/feedback/ui/types.ts | 27 +- packages/browser-sdk/src/index.ts | 11 +- packages/browser-sdk/src/toolbar/Features.css | 74 ---- packages/browser-sdk/src/toolbar/Features.tsx | 113 ----- packages/browser-sdk/src/toolbar/Switch.css | 22 - packages/browser-sdk/src/toolbar/Switch.tsx | 50 --- packages/browser-sdk/src/toolbar/Toolbar.css | 163 -------- packages/browser-sdk/src/toolbar/Toolbar.tsx | 146 ------- packages/browser-sdk/src/toolbar/index.css | 3 - packages/browser-sdk/src/toolbar/index.ts | 23 - packages/browser-sdk/src/ui/Dialog.css | 113 ----- packages/browser-sdk/src/ui/Dialog.tsx | 263 ------------ packages/browser-sdk/src/ui/types.ts | 33 -- packages/browser-sdk/src/ui/utils.ts | 65 --- packages/browser-sdk/test/client.test.ts | 12 - .../test/e2e/feedback-widget.browser.spec.ts | 63 +-- .../test/e2e/give-feedback-button.html | 13 - packages/browser-sdk/test/features.test.ts | 111 +---- packages/browser-sdk/test/mocks/handlers.ts | 65 +-- packages/browser-sdk/test/usage.test.ts | 198 +++------ packages/node-sdk/README.md | 320 +++++++++++--- .../docs/type-check-payload-failed.png | Bin 0 -> 69185 bytes packages/node-sdk/package.json | 2 +- packages/node-sdk/src/client.ts | 212 +++++++--- packages/node-sdk/src/types.ts | 30 +- packages/node-sdk/test/client.test.ts | 395 ++++++++++++++---- .../example/package.json | 1 - packages/react-sdk/README.md | 74 +--- packages/react-sdk/package.json | 7 +- packages/react-sdk/src/index.tsx | 92 +--- packages/react-sdk/test/usage.test.tsx | 31 -- yarn.lock | 218 +--------- 70 files changed, 1386 insertions(+), 2688 deletions(-) create mode 100644 packages/browser-sdk/example/browser.html delete mode 100644 packages/browser-sdk/example/typescript/app.ts delete mode 100644 packages/browser-sdk/example/typescript/index.html delete mode 100644 packages/browser-sdk/index.html rename packages/browser-sdk/src/{ => feedback}/ui/constants.ts (95%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/Check.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/CheckCircle.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/Close.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/Dissatisfied.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/Logo.tsx (98%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/Neutral.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/Satisfied.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/VeryDissatisfied.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/icons/VerySatisfied.tsx (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/README.md (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/arrow.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/index.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/types.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/useFloating.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/utils/deepEqual.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/utils/getDPR.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/utils/roundByDPR.ts (100%) rename packages/browser-sdk/src/{ => feedback}/ui/packages/floating-ui-preact-dom/utils/useLatestRef.ts (100%) delete mode 100644 packages/browser-sdk/src/toolbar/Features.css delete mode 100644 packages/browser-sdk/src/toolbar/Features.tsx delete mode 100644 packages/browser-sdk/src/toolbar/Switch.css delete mode 100644 packages/browser-sdk/src/toolbar/Switch.tsx delete mode 100644 packages/browser-sdk/src/toolbar/Toolbar.css delete mode 100644 packages/browser-sdk/src/toolbar/Toolbar.tsx delete mode 100644 packages/browser-sdk/src/toolbar/index.css delete mode 100644 packages/browser-sdk/src/toolbar/index.ts delete mode 100644 packages/browser-sdk/src/ui/Dialog.css delete mode 100644 packages/browser-sdk/src/ui/Dialog.tsx delete mode 100644 packages/browser-sdk/src/ui/types.ts delete mode 100644 packages/browser-sdk/src/ui/utils.ts delete mode 100644 packages/browser-sdk/test/e2e/give-feedback-button.html create mode 100644 packages/node-sdk/docs/type-check-payload-failed.png diff --git a/docs.sh b/docs.sh index e67f36f5..d5ecc670 100755 --- a/docs.sh +++ b/docs.sh @@ -15,11 +15,46 @@ typedoc # We can fix this by removing the number at the end of the anchor. SEDCOMMAND='s/globals.md#(.*)-[0-9]+/globals.md#\1/g' -FILES=$(find dist/docs/@bucketco -name "globals.md") +# Find all markdown files including globals.md +FILES=$(find dist/docs/@bucketco -name "*.md") +echo "Processing markdown files..." for file in $FILES do - sed -r $SEDCOMMAND $file > $file.fixed - rm $file - mv $file.fixed $file + echo "Processing $file..." + + # Fix anchor links in globals.md files + if [[ "$file" == *"globals.md" ]]; then + sed -r "$SEDCOMMAND" "$file" > "$file.fixed" + rm "$file" + mv "$file.fixed" "$file" + fi + + # Create a temporary file for processing + tmp_file="${file}.tmp" + + # Process NOTE blocks - handle multi-line + awk ' + BEGIN { in_block = 0; content = ""; } + /^> \[!NOTE\]/ { in_block = 1; print "{% hint style=\"info\" %}"; next; } + /^> \[!TIP\]/ { in_block = 1; print "{% hint style=\"success\" %}"; next; } + /^> \[!IMPORTANT\]/ { in_block = 1; print "{% hint style=\"warning\" %}"; next; } + /^> \[!WARNING\]/ { in_block = 1; print "{% hint style=\"warning\" %}"; next; } + /^> \[!CAUTION\]/ { in_block = 1; print "{% hint style=\"danger\" %}"; next; } + in_block && /^>/ { + content = content substr($0, 3) "\n"; + next; + } + in_block && !/^>/ { + printf "%s", content; + print "{% endhint %}"; + in_block = 0; + content = ""; + } + !in_block { print; } + ' "$file" > "$tmp_file" + + mv "$tmp_file" "$file" done + +echo "Processing complete!" diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index eec910a2..63e88c60 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -27,28 +27,19 @@ const bucketClient = new BucketClient({ publishableKey, user, company }); await bucketClient.initialize(); -const { - isEnabled, - config: { payload: question }, - track, - requestFeedback, -} = bucketClient.getFeature("huddle"); +const { isEnabled, track, requestFeedback } = bucketClient.getFeature("huddle"); if (isEnabled) { - // Show feature. When retrieving `isEnabled` the client automatically + // show feature. When retrieving `isEnabled` the client automatically // sends a "check" event for the "huddle" feature which is shown in the // Bucket UI. // On usage, call `track` to let Bucket know that a user interacted with the feature track(); - // The `payload` is a user-supplied JSON in Bucket that is dynamically picked - // out depending on the user/company. - const question = payload?.question ?? "Tell us what you think of Huddles"; - // Use `requestFeedback` to create "Send feedback" buttons easily for specific // features. This is not related to `track` and you can call them individually. - requestFeedback({ title: question }); + requestFeedback({ title: "Tell us what you think of Huddles" }); } // `track` just calls `bucketClient.track()` to send an event using the same feature key @@ -147,7 +138,6 @@ To retrieve features along with their targeting information, use `getFeature(key const huddle = bucketClient.getFeature("huddle"); // { // isEnabled: true, -// config: { key: "zoom", payload: { ... } }, // track: () => Promise // requestFeedback: (options: RequestFeedbackData) => void // } @@ -161,7 +151,6 @@ const features = bucketClient.getFeatures(); // huddle: { // isEnabled: true, // targetingVersion: 42, -// config: ... // } // } ``` @@ -170,35 +159,7 @@ const features = bucketClient.getFeatures(); by down-stream clients, like the React SDK. Note that accessing `isEnabled` on the object returned by `getFeatures` does not automatically -generate a `check` event, contrary to the `isEnabled` property on the object returned by `getFeature`. - -### Remote config - -Similar to `isEnabled`, each feature has a `config` property. This configuration is managed from within Bucket. -It is managed similar to the way access to features is managed, but instead of the binary `isEnabled` you can have -multiple configuration values which are given to different user/companies. - -```ts -const features = bucketClient.getFeatures(); -// { -// huddle: { -// isEnabled: true, -// targetingVersion: 42, -// config: { -// key: "gpt-3.5", -// payload: { maxTokens: 10000, model: "gpt-3.5-beta1" } -// } -// } -// } -``` - -The `key` is always present while the `payload` is a optional JSON value for arbitrary configuration needs. -If feature has no configuration or, no configuration value was matched against the context, the `config` object -will be empty, thus, `key` will be `undefined`. Make sure to check against this case when trying to use the -configuration in your application. - -Just as `isEnabled`, accessing `config` on the object returned by `getFeatures` does not automatically -generate a `check` event, contrary to the `config` property on the object returned by `getFeature`. +generate a `check` event, contrary to the `isEnabled` property on the object return from `getFeature`. ### Tracking feature usage diff --git a/packages/browser-sdk/example/browser.html b/packages/browser-sdk/example/browser.html new file mode 100644 index 00000000..175497d1 --- /dev/null +++ b/packages/browser-sdk/example/browser.html @@ -0,0 +1,43 @@ + + + + + + Bucket feature management + + + Loading... + + + + + + + + diff --git a/packages/browser-sdk/example/typescript/app.ts b/packages/browser-sdk/example/typescript/app.ts deleted file mode 100644 index 72ee20cd..00000000 --- a/packages/browser-sdk/example/typescript/app.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { BucketClient } from "../../src"; - -const urlParams = new URLSearchParams(window.location.search); -const publishableKey = urlParams.get("publishableKey"); -const featureKey = urlParams.get("featureKey") ?? "huddles"; - -const featureList = ["huddles"]; - -if (!publishableKey) { - throw Error("publishableKey is missing"); -} - -const bucket = new BucketClient({ - publishableKey, - user: { id: "42" }, - company: { id: "1" }, - toolbar: { - show: true, - position: { placement: "bottom-right" }, - }, - featureList, -}); - -document - .getElementById("startHuddle") - ?.addEventListener("click", () => bucket.track(featureKey)); -document.getElementById("giveFeedback")?.addEventListener("click", (event) => - bucket.requestFeedback({ - featureKey, - position: { type: "POPOVER", anchor: event.currentTarget as HTMLElement }, - }), -); - -bucket.initialize().then(() => { - console.log("Bucket initialized"); - const loadingElem = document.getElementById("loading"); - if (loadingElem) loadingElem.style.display = "none"; -}); - -bucket.onFeaturesUpdated(() => { - const { isEnabled } = bucket.getFeature("huddles"); - - const startHuddleElem = document.getElementById("start-huddle"); - if (isEnabled) { - // show the start-huddle button - if (startHuddleElem) startHuddleElem.style.display = "block"; - } else { - // hide the start-huddle button - if (startHuddleElem) startHuddleElem.style.display = "none"; - } -}); diff --git a/packages/browser-sdk/example/typescript/index.html b/packages/browser-sdk/example/typescript/index.html deleted file mode 100644 index cec72d21..00000000 --- a/packages/browser-sdk/example/typescript/index.html +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - Bucket feature management - - - Loading... - - - - - diff --git a/packages/browser-sdk/index.html b/packages/browser-sdk/index.html deleted file mode 100644 index 597eadec..00000000 --- a/packages/browser-sdk/index.html +++ /dev/null @@ -1,71 +0,0 @@ - - - - - - - - Bucket Browser SDK - - -
- Loading... - - - - - - - - diff --git a/packages/browser-sdk/package.json b/packages/browser-sdk/package.json index 734debca..10109b93 100644 --- a/packages/browser-sdk/package.json +++ b/packages/browser-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/browser-sdk", - "version": "3.0.0-alpha.2", + "version": "2.5.2", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index a3d34a19..09e6ca50 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -14,12 +14,10 @@ import { RequestFeedbackOptions, } from "./feedback/feedback"; import * as feedbackLib from "./feedback/ui"; -import { ToolbarPosition } from "./toolbar/Toolbar"; -import { API_BASE_URL, APP_BASE_URL, SSE_REALTIME_BASE_URL } from "./config"; +import { API_BASE_URL, SSE_REALTIME_BASE_URL } from "./config"; import { BucketContext, CompanyContext, UserContext } from "./context"; import { HttpClient } from "./httpClient"; import { Logger, loggerWithPrefix, quietConsoleLogger } from "./logger"; -import { showToolbarToggle } from "./toolbar"; const isMobile = typeof window !== "undefined" && window.innerWidth < 768; const isNode = typeof document === "undefined"; // deno supports "window" but not "document" according to https://remix.run/docs/en/main/guides/gotchas @@ -151,11 +149,6 @@ interface Config { */ apiBaseUrl: string; - /** - * Base URL of the Bucket web app. - */ - appBaseUrl: string; - /** * Base URL of Bucket servers for SSE connections used by AutoFeedback. */ @@ -167,16 +160,6 @@ interface Config { enableTracking: boolean; } -/** - * Toolbar options. - */ -export type ToolbarOptions = - | boolean - | { - show?: boolean; - position?: ToolbarPosition; - }; - /** * Feature definitions. */ @@ -229,11 +212,6 @@ export interface InitOptions { */ apiBaseUrl?: string; - /** - * Base URL of the Bucket web app. Links open ín this app by default. - */ - appBaseUrl?: string; - /** * @deprecated * Use `sseBaseUrl` instead. @@ -264,60 +242,26 @@ export interface InitOptions { * Whether to enable tracking. Defaults to `true`. */ enableTracking?: boolean; - - /** - * Toolbar configuration (alpha) - * @ignore - */ - toolbar?: ToolbarOptions; - - /** - * Local-first definition of features (alpha) - * @ignore - */ - featureList?: FeatureDefinitions; } const defaultConfig: Config = { apiBaseUrl: API_BASE_URL, - appBaseUrl: APP_BASE_URL, sseBaseUrl: SSE_REALTIME_BASE_URL, enableTracking: true, }; /** - * A remotely managed configuration value for a feature. - */ -export type FeatureRemoteConfig = - | { - /** - * The key of the matched configuration value. - */ - key: string; - - /** - * The optional user-supplied payload data. - */ - payload: any; - } - | { key: undefined; payload: undefined }; - -/** - * A feature. + * Represents a feature. */ export interface Feature { /** - * Result of feature flag evaluation. + * Result of feature flag evaluation */ isEnabled: boolean; - /* - * Optional user-defined configuration. - */ - config: FeatureRemoteConfig; - /** - * Function to send analytics events for this feature. + * Function to send analytics events for this feature + * */ track: () => Promise; @@ -328,33 +272,21 @@ export interface Feature { options: Omit, ) => void; } - -function shouldShowToolbar(opts: InitOptions) { - const toolbarOpts = opts.toolbar; - if (typeof toolbarOpts === "boolean") return toolbarOpts; - if (typeof toolbarOpts?.show === "boolean") return toolbarOpts.show; - - return ( - opts.featureList !== undefined && window?.location?.hostname === "localhost" - ); -} - /** * BucketClient lets you interact with the Bucket API. */ export class BucketClient { - private readonly publishableKey: string; - private readonly context: BucketContext; + private publishableKey: string; + private context: BucketContext; private config: Config; private requestFeedbackOptions: Partial; - private readonly httpClient: HttpClient; + private httpClient: HttpClient; - private readonly autoFeedback: AutoFeedback | undefined; + private autoFeedback: AutoFeedback | undefined; private autoFeedbackInit: Promise | undefined; - private readonly featuresClient: FeaturesClient; + private featuresClient: FeaturesClient; public readonly logger: Logger; - /** * Create a new BucketClient instance. */ @@ -370,10 +302,9 @@ export class BucketClient { this.config = { apiBaseUrl: opts?.apiBaseUrl ?? opts?.host ?? defaultConfig.apiBaseUrl, - appBaseUrl: opts?.appBaseUrl ?? opts?.host ?? defaultConfig.appBaseUrl, sseBaseUrl: opts?.sseBaseUrl ?? opts?.sseHost ?? defaultConfig.sseBaseUrl, enableTracking: opts?.enableTracking ?? defaultConfig.enableTracking, - }; + } satisfies Config; const feedbackOpts = handleDeprecatedFeedbackOptions(opts?.feedback); @@ -395,7 +326,6 @@ export class BucketClient { company: this.context.company, other: this.context.otherContext, }, - opts?.featureList || [], this.logger, opts?.features, ); @@ -421,15 +351,6 @@ export class BucketClient { ); } } - - if (shouldShowToolbar(opts)) { - this.logger.info("opening toolbar toggler"); - showToolbarToggle({ - bucketClient: this as unknown as BucketClient, - position: - typeof opts.toolbar === "object" ? opts.toolbar.position : undefined, - }); - } } /** @@ -460,13 +381,6 @@ export class BucketClient { } } - /** - * Get the current configuration. - */ - getConfig() { - return this.config; - } - /** * Update the user context. * Performs a shallow merge with the existing user context. @@ -496,7 +410,7 @@ export class BucketClient { * Performs a shallow merge with the existing company context. * Attempting to update the company ID will log a warning and be ignored. * - * @param company The company details. + * @param company */ async updateCompany(company: { [key: string]: string | number | undefined }) { if (company.id && company.id !== this.context.company?.id) { @@ -518,8 +432,6 @@ export class BucketClient { * Update the company context. * Performs a shallow merge with the existing company context. * Updates to the company ID will be ignored. - * - * @param otherContext Additional context. */ async updateOtherContext(otherContext: { [key: string]: string | number | undefined; @@ -537,7 +449,7 @@ export class BucketClient { * * Calling `client.stop()` will remove all listeners added here. * - * @param cb The callback to call when the update completes. + * @param cb this will be called when the features are updated. */ onFeaturesUpdated(cb: () => void) { return this.featuresClient.onUpdated(cb); @@ -546,8 +458,8 @@ export class BucketClient { /** * Track an event in Bucket. * - * @param eventName The name of the event. - * @param attributes Any attributes you want to attach to the event. + * @param eventName The name of the event + * @param attributes Any attributes you want to attach to the event */ async track(eventName: string, attributes?: Record | null) { if (!this.context.user) { @@ -575,8 +487,7 @@ export class BucketClient { /** * Submit user feedback to Bucket. Must include either `score` or `comment`, or both. * - * @param payload The feedback details to submit. - * @returns The server response. + * @returns */ async feedback(payload: Feedback) { const userId = @@ -669,52 +580,36 @@ export class BucketClient { /** * Returns a map of enabled features. * Accessing a feature will *not* send a check event - * and `isEnabled` does not take any feature overrides - * into account. * - * @returns Map of features. + * @returns Map of features */ getFeatures(): RawFeatures { return this.featuresClient.getFeatures(); } /** - * Return a feature. Accessing `isEnabled` or `config` will automatically send a `check` event. - * @returns A feature. + * Return a feature. Accessing `isEnabled` will automatically send a `check` event. + * @returns A feature */ getFeature(key: string): Feature { const f = this.getFeatures()[key]; const fClient = this.featuresClient; - const value = f?.isEnabledOverride ?? f?.isEnabled ?? false; - const config = f?.config - ? { - key: f.config.key, - payload: f.config.payload, - } - : { key: undefined, payload: undefined }; - - function sendCheckEvent() { - fClient - .sendCheckEvent({ - key, - version: f?.targetingVersion, - value, - }) - .catch(() => { - // ignore - }); - } + const value = f?.isEnabled ?? false; return { get isEnabled() { - sendCheckEvent(); + fClient + .sendCheckEvent({ + key: key, + version: f?.targetingVersion, + value, + }) + .catch(() => { + // ignore + }); return value; }, - get config() { - sendCheckEvent(); - return config; - }, track: () => this.track(key), requestFeedback: ( options: Omit, @@ -727,14 +622,6 @@ export class BucketClient { }; } - setFeatureOverride(key: string, isEnabled: boolean | null) { - this.featuresClient.setFeatureOverride(key, isEnabled); - } - - getFeatureOverride(key: string): boolean | null { - return this.featuresClient.getFeatureOverride(key); - } - sendCheckEvent(checkEvent: CheckEvent) { return this.featuresClient.sendCheckEvent(checkEvent); } diff --git a/packages/browser-sdk/src/config.ts b/packages/browser-sdk/src/config.ts index fd116c7f..e1baeec7 100644 --- a/packages/browser-sdk/src/config.ts +++ b/packages/browser-sdk/src/config.ts @@ -1,7 +1,6 @@ import { version } from "../package.json"; export const API_BASE_URL = "https://front.bucket.co"; -export const APP_BASE_URL = "https://app.bucket.co"; export const SSE_REALTIME_BASE_URL = "https://livemessaging.bucket.co"; export const SDK_VERSION_HEADER_NAME = "bucket-sdk-version"; diff --git a/packages/browser-sdk/src/feature/featureCache.ts b/packages/browser-sdk/src/feature/featureCache.ts index b4cb8ed8..1a66c441 100644 --- a/packages/browser-sdk/src/feature/featureCache.ts +++ b/packages/browser-sdk/src/feature/featureCache.ts @@ -1,4 +1,4 @@ -import { FetchedFeatures } from "./features"; +import { RawFeatures } from "./features"; interface StorageItem { get(): string | null; @@ -8,50 +8,45 @@ interface StorageItem { interface cacheEntry { expireAt: number; staleAt: number; - features: FetchedFeatures; + features: RawFeatures; } // Parse and validate an API feature response export function parseAPIFeaturesResponse( featuresInput: any, -): FetchedFeatures | undefined { +): RawFeatures | undefined { if (!isObject(featuresInput)) { return; } - const features: FetchedFeatures = {}; + const features: RawFeatures = {}; for (const key in featuresInput) { const feature = featuresInput[key]; - if ( typeof feature.isEnabled !== "boolean" || feature.key !== key || - typeof feature.targetingVersion !== "number" || - (feature.config && typeof feature.config !== "object") + typeof feature.targetingVersion !== "number" ) { return; } - features[key] = { isEnabled: feature.isEnabled, targetingVersion: feature.targetingVersion, key, - config: feature.config, }; } - return features; } export interface CacheResult { - features: FetchedFeatures; + features: RawFeatures; stale: boolean; } export class FeatureCache { private storage: StorageItem; - private readonly staleTimeMs: number; - private readonly expireTimeMs: number; + private staleTimeMs: number; + private expireTimeMs: number; constructor({ storage, @@ -72,7 +67,7 @@ export class FeatureCache { { features, }: { - features: FetchedFeatures; + features: RawFeatures; }, ) { let cacheData: CacheData = {}; diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 7c11420a..46d02a61 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -9,10 +9,7 @@ import { parseAPIFeaturesResponse, } from "./featureCache"; -/** - * A feature fetched from the server. - */ -export type FetchedFeature = { +export type RawFeature = { /** * Feature key */ @@ -27,91 +24,58 @@ export type FetchedFeature = { * Version of targeting rules */ targetingVersion?: number; - - /** - * Optional user-defined dynamic configuration. - */ - config?: { - /** - * The key of the matched configuration value. - */ - key: string; - - /** - * The version of the matched configuration value. - */ - version?: number; - - /** - * The optional user-supplied payload data. - */ - payload?: any; - }; }; const FEATURES_UPDATED_EVENT = "features-updated"; -export type FetchedFeatures = Record; - -// todo: on next major, come up with a better name for this type. Maybe `LocalFeature`. -export type RawFeature = FetchedFeature & { - /** - * If not null, the result is being overridden locally - */ - isEnabledOverride: boolean | null; -}; - -export type RawFeatures = Record; - -export type FallbackFeatureOverride = - | { - key: string; - payload: any; - } - | true; +export type RawFeatures = Record; export type FeaturesOptions = { /** * Feature keys for which `isEnabled` should fallback to true - * if SDK fails to fetch features from Bucket servers. If a record - * is supplied instead of array, the values of each key represent the - * configuration values and `isEnabled` is assume `true`. + * if SDK fails to fetch features from Bucket servers. */ - fallbackFeatures?: string[] | Record; + fallbackFeatures?: string[]; /** - * Timeout in milliseconds when fetching features + * Timeout in miliseconds */ timeoutMs?: number; /** - * If set to true stale features will be returned while refetching features + * If set to true client will return cached value when its stale + * but refetching */ staleWhileRevalidate?: boolean; - - /** - * If set, features will be cached between page loads for this duration - */ - expireTimeMs?: number; - - /** - * Stale features will be returned if staleWhileRevalidate is true if no new features can be fetched - */ staleTimeMs?: number; + expireTimeMs?: number; }; type Config = { - fallbackFeatures: Record; + fallbackFeatures: string[]; timeoutMs: number; staleWhileRevalidate: boolean; }; export const DEFAULT_FEATURES_CONFIG: Config = { - fallbackFeatures: {}, + fallbackFeatures: [], timeoutMs: 5000, staleWhileRevalidate: false, }; +// Deep merge two objects. +export type FeaturesResponse = { + /** + * `true` if call was successful + */ + success: boolean; + + /** + * List of enabled features + */ + features: RawFeatures; +}; + export function validateFeaturesResponse(response: any) { if (!isObject(response)) { return; @@ -120,9 +84,7 @@ export function validateFeaturesResponse(response: any) { if (typeof response.success !== "boolean" || !isObject(response.features)) { return; } - const features = parseAPIFeaturesResponse(response.features); - if (!features) { return; } @@ -176,37 +138,14 @@ type context = { export const FEATURES_EXPIRE_MS = 30 * 24 * 60 * 60 * 1000; // expire entirely after 30 days -const localStorageFetchedFeaturesKey = `__bucket_fetched_features`; -const localStorageOverridesKey = `__bucket_overrides`; - -type OverridesFeatures = Record; - -function setOverridesCache(overrides: OverridesFeatures) { - localStorage.setItem(localStorageOverridesKey, JSON.stringify(overrides)); -} - -function getOverridesCache(): OverridesFeatures { - const cachedOverrides = JSON.parse( - localStorage.getItem(localStorageOverridesKey) || "{}", - ); - - if (!isObject(cachedOverrides)) { - return {}; - } - - return cachedOverrides; -} +const localStorageCacheKey = `__bucket_features`; /** * @internal */ export class FeaturesClient { private cache: FeatureCache; - private fetchedFeatures: FetchedFeatures; - private featureOverrides: OverridesFeatures = {}; - - private features: RawFeatures = {}; - + private features: RawFeatures; private config: Config; private rateLimiter: RateLimiter; private readonly logger: Logger; @@ -217,63 +156,33 @@ export class FeaturesClient { constructor( private httpClient: HttpClient, private context: context, - private featureDefinitions: Readonly, logger: Logger, options?: FeaturesOptions & { cache?: FeatureCache; rateLimiter?: RateLimiter; }, ) { - this.fetchedFeatures = {}; + this.features = {}; this.logger = loggerWithPrefix(logger, "[Features]"); this.cache = options?.cache ? options.cache : new FeatureCache({ storage: { - get: () => localStorage.getItem(localStorageFetchedFeaturesKey), - set: (value) => - localStorage.setItem(localStorageFetchedFeaturesKey, value), + get: () => localStorage.getItem(localStorageCacheKey), + set: (value) => localStorage.setItem(localStorageCacheKey, value), }, staleTimeMs: options?.staleTimeMs ?? 0, expireTimeMs: options?.expireTimeMs ?? FEATURES_EXPIRE_MS, }); - - let fallbackFeatures: Record; - - if (Array.isArray(options?.fallbackFeatures)) { - fallbackFeatures = options.fallbackFeatures.reduce( - (acc, key) => { - acc[key] = true; - return acc; - }, - {} as Record, - ); - } else { - fallbackFeatures = options?.fallbackFeatures ?? {}; - } - - this.config = { ...DEFAULT_FEATURES_CONFIG, ...options, fallbackFeatures }; - + this.config = { ...DEFAULT_FEATURES_CONFIG, ...options }; this.rateLimiter = options?.rateLimiter ?? new RateLimiter(FEATURE_EVENTS_PER_MIN, this.logger); - - try { - const storedFeatureOverrides = getOverridesCache(); - for (const key in storedFeatureOverrides) { - if (this.featureDefinitions.includes(key)) { - this.featureOverrides[key] = storedFeatureOverrides[key]; - } - } - } catch (e) { - this.logger.warn("error getting feature overrides from cache", e); - this.featureOverrides = {}; - } } async initialize() { const features = (await this.maybeFetchFeatures()) || {}; - this.setFetchedFeatures(features); + this.setFeatures(features); } async setContext(context: context) { @@ -313,11 +222,7 @@ export class FeaturesClient { return this.features; } - getFetchedFeatures(): FetchedFeatures { - return this.fetchedFeatures; - } - - public async fetchFeatures(): Promise { + public async fetchFeatures(): Promise { const params = this.fetchParams(); try { const res = await this.httpClient.get({ @@ -341,7 +246,6 @@ export class FeaturesClient { JSON.stringify(errorBody), ); } - const typeRes = validateFeaturesResponse(await res.json()); if (!typeRes || !typeRes.success) { throw new Error("unable to validate response"); @@ -387,41 +291,11 @@ export class FeaturesClient { return checkEvent.value; } - private triggerFeaturesChanged() { - const mergedFeatures: RawFeatures = {}; - - // merge fetched features with overrides into `this.features` - for (const key in this.fetchedFeatures) { - const fetchedFeature = this.fetchedFeatures[key]; - if (!fetchedFeature) continue; - const isEnabledOverride = this.featureOverrides[key] ?? null; - mergedFeatures[key] = { - ...fetchedFeature, - isEnabledOverride, - }; - } - - // add any features that aren't in the fetched features - for (const key of this.featureDefinitions) { - if (!mergedFeatures[key]) { - mergedFeatures[key] = { - key, - isEnabled: false, - isEnabledOverride: this.featureOverrides[key] ?? null, - }; - } - } - - this.features = mergedFeatures; - + private setFeatures(features: RawFeatures) { + this.features = features; this.eventTarget.dispatchEvent(new Event(FEATURES_UPDATED_EVENT)); } - private setFetchedFeatures(features: FetchedFeatures) { - this.fetchedFeatures = features; - this.triggerFeaturesChanged(); - } - private fetchParams() { const flattenedContext = flattenJSON({ context: this.context }); const params = new URLSearchParams(flattenedContext); @@ -434,7 +308,7 @@ export class FeaturesClient { return params; } - private async maybeFetchFeatures(): Promise { + private async maybeFetchFeatures(): Promise { const cacheKey = this.fetchParams().toString(); const cachedItem = this.cache.get(cacheKey); @@ -451,7 +325,7 @@ export class FeaturesClient { this.cache.set(cacheKey, { features, }); - this.setFetchedFeatures(features); + this.setFeatures(features); }) .catch(() => { // we don't care about the result, we just want to re-fetch @@ -478,41 +352,12 @@ export class FeaturesClient { } // fetch failed, nothing cached => return fallbacks - return Object.entries(this.config.fallbackFeatures).reduce( - (acc, [key, override]) => { - acc[key] = { - key, - isEnabled: !!override, - config: - typeof override === "object" && "key" in override - ? { - key: override.key, - payload: override.payload, - } - : undefined, - }; - return acc; - }, - {} as FetchedFeatures, - ); - } - - setFeatureOverride(key: string, isEnabled: boolean | null) { - if (!(typeof isEnabled === "boolean" || isEnabled === null)) { - throw new Error("setFeatureOverride: isEnabled must be boolean or null"); - } - - if (isEnabled === null) { - delete this.featureOverrides[key]; - } else { - this.featureOverrides[key] = isEnabled; - } - setOverridesCache(this.featureOverrides); - - this.triggerFeaturesChanged(); - } - - getFeatureOverride(key: string): boolean | null { - return this.featureOverrides[key] ?? null; + return this.config.fallbackFeatures.reduce((acc, key) => { + acc[key] = { + key, + isEnabled: true, + }; + return acc; + }, {} as RawFeatures); } } diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index c649b283..b83428a9 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -1,9 +1,9 @@ import { HttpClient } from "../httpClient"; import { Logger } from "../logger"; import { AblySSEChannel, openAblySSEChannel } from "../sse"; -import { Position } from "../ui/types"; import { + FeedbackPosition, FeedbackSubmission, FeedbackTranslations, OpenFeedbackFormOptions, @@ -37,7 +37,7 @@ export type FeedbackOptions = { /** * Control the placement and behavior of the feedback form. */ - position?: Position; + position?: FeedbackPosition; /** * Add your own custom translations for the feedback form. @@ -69,7 +69,7 @@ export function handleDeprecatedFeedbackOptions( }; } -type FeatureIdentifier = +export type FeatureIdentifier = | { /** * Bucket feature ID. @@ -100,9 +100,6 @@ export type RequestFeedbackData = Omit< * * This can be used for side effects, such as storing a * copy of the feedback in your own application or CRM. - * - * @param {Object} data - * @param data. */ onAfterSubmit?: (data: FeedbackSubmission) => void; } & FeatureIdentifier; @@ -297,7 +294,7 @@ export class AutoFeedback { private httpClient: HttpClient, private feedbackPromptHandler: FeedbackPromptHandler = createDefaultFeedbackPromptHandler(), private userId: string, - private position: Position = DEFAULT_POSITION, + private position: FeedbackPosition = DEFAULT_POSITION, private feedbackTranslations: Partial = {}, ) {} diff --git a/packages/browser-sdk/src/feedback/ui/FeedbackDialog.css b/packages/browser-sdk/src/feedback/ui/FeedbackDialog.css index 96cb6f02..91a99ec7 100644 --- a/packages/browser-sdk/src/feedback/ui/FeedbackDialog.css +++ b/packages/browser-sdk/src/feedback/ui/FeedbackDialog.css @@ -1,3 +1,39 @@ +@keyframes scale { + from { + transform: scale(0.9); + } + to { + transform: scale(1); + } +} + +@keyframes floatUp { + from { + transform: translateY(15%); + } + to { + transform: translateY(0%); + } +} + +@keyframes floatDown { + from { + transform: translateY(-15%); + } + to { + transform: translateY(0%); + } +} + +@keyframes fade { + from { + opacity: 0; + } + to { + opacity: 1; + } +} + .dialog { position: fixed; width: 210px; @@ -33,8 +69,12 @@ } .arrow { + position: absolute; + width: 8px; + height: 8px; background-color: var(--bucket-feedback-dialog-background-color, #fff); box-shadow: var(--bucket-feedback-dialog-border, #d8d9df) -1px -1px 1px 0px; + transform: rotate(45deg); &.bottom { box-shadow: var(--bucket-feedback-dialog-border, #d8d9df) -1px -1px 1px 0px; @@ -94,3 +134,62 @@ .plug a:hover { opacity: 0.7; } + +/* Modal */ + +.dialog.modal { + margin: auto; + margin-top: 4rem; + + &[open] { + animation: /* easeOutQuint */ + scale 200ms cubic-bezier(0.22, 1, 0.36, 1), + fade 200ms cubic-bezier(0.22, 1, 0.36, 1); + + &::backdrop { + animation: fade 200ms cubic-bezier(0.22, 1, 0.36, 1); + } + } +} + +/* Anchored */ + +.dialog.anchored { + position: absolute; + + &[open] { + animation: /* easeOutQuint */ + scale 200ms cubic-bezier(0.22, 1, 0.36, 1), + fade 200ms cubic-bezier(0.22, 1, 0.36, 1); + } + &.bottom { + transform-origin: top center; + } + &.top { + transform-origin: bottom center; + } + &.left { + transform-origin: right center; + } + &.right { + transform-origin: left center; + } +} + +/* Unanchored */ + +.dialog[open].unanchored { + &.unanchored-bottom-left, + &.unanchored-bottom-right { + animation: /* easeOutQuint */ + floatUp 300ms cubic-bezier(0.22, 1, 0.36, 1), + fade 300ms cubic-bezier(0.22, 1, 0.36, 1); + } + + &.unanchored-top-left, + &.unanchored-top-right { + animation: /* easeOutQuint */ + floatDown 300ms cubic-bezier(0.22, 1, 0.36, 1), + fade 300ms cubic-bezier(0.22, 1, 0.36, 1); + } +} diff --git a/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx b/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx index 2893ebbe..8a0f771a 100644 --- a/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx +++ b/packages/browser-sdk/src/feedback/ui/FeedbackDialog.tsx @@ -1,22 +1,33 @@ import { Fragment, FunctionComponent, h } from "preact"; -import { useCallback, useState } from "preact/hooks"; - -import { feedbackContainerId } from "../../ui/constants"; -import { Dialog, useDialog } from "../../ui/Dialog"; -import { Close } from "../../ui/icons/Close"; +import { useCallback, useEffect, useRef, useState } from "preact/hooks"; import { DEFAULT_TRANSLATIONS } from "./config/defaultTranslations"; import { useTimer } from "./hooks/useTimer"; +import { Close } from "./icons/Close"; +import { + arrow, + autoUpdate, + flip, + offset, + shift, + useFloating, +} from "./packages/floating-ui-preact-dom"; +import { feedbackContainerId } from "./constants"; import { FeedbackForm } from "./FeedbackForm"; import styles from "./index.css?inline"; import { RadialProgress } from "./RadialProgress"; import { FeedbackScoreSubmission, FeedbackSubmission, + Offset, OpenFeedbackFormOptions, WithRequired, } from "./types"; +type Position = Partial< + Record<"top" | "left" | "right" | "bottom", number | string> +>; + export type FeedbackDialogProps = WithRequired< OpenFeedbackFormOptions, "onSubmit" | "position" @@ -36,6 +47,97 @@ export const FeedbackDialog: FunctionComponent = ({ onSubmit, onScoreSubmit, }) => { + const arrowRef = useRef(null); + const anchor = position.type === "POPOVER" ? position.anchor : null; + const { + refs, + floatingStyles, + middlewareData, + placement: actualPlacement, + } = useFloating({ + elements: { + reference: anchor, + }, + transform: false, + whileElementsMounted: autoUpdate, + middleware: [ + flip({ + padding: 10, + mainAxis: true, + crossAxis: true, + fallbackAxisSideDirection: "end", + }), + shift(), + offset(8), + arrow({ + element: arrowRef, + }), + ], + }); + + let unanchoredPosition: Position = {}; + if (position.type === "DIALOG") { + const offsetY = parseOffset(position.offset?.y); + const offsetX = parseOffset(position.offset?.x); + + switch (position.placement) { + case "top-left": + unanchoredPosition = { + top: offsetY, + left: offsetX, + }; + break; + case "top-right": + unanchoredPosition = { + top: offsetY, + right: offsetX, + }; + break; + case "bottom-left": + unanchoredPosition = { + bottom: offsetY, + left: offsetX, + }; + break; + case "bottom-right": + unanchoredPosition = { + bottom: offsetY, + right: offsetX, + }; + break; + } + } + + const { x: arrowX, y: arrowY } = middlewareData.arrow ?? {}; + + const staticSide = + { + top: "bottom", + right: "left", + bottom: "top", + left: "right", + }[actualPlacement.split("-")[0]] || "bottom"; + + const arrowStyles = { + left: arrowX != null ? `${arrowX}px` : "", + top: arrowY != null ? `${arrowY}px` : "", + right: "", + bottom: "", + [staticSide]: "-4px", + }; + + const close = useCallback(() => { + const dialog = refs.floating.current as HTMLDialogElement | null; + dialog?.close(); + autoClose.stop(); + onClose?.(); + }, [onClose]); + + const dismiss = useCallback(() => { + close(); + onDismiss?.(); + }, [close, onDismiss]); + const [feedbackId, setFeedbackId] = useState(undefined); const [scoreState, setScoreState] = useState< "idle" | "submitting" | "submitted" @@ -62,54 +164,112 @@ export const FeedbackDialog: FunctionComponent = ({ [feedbackId, onSubmit], ); - const { isOpen, close } = useDialog({ onClose, initialValue: true }); - const autoClose = useTimer({ enabled: position.type === "DIALOG", initialDuration: INACTIVE_DURATION_MS, onEnd: close, }); - const dismiss = useCallback(() => { - autoClose.stop(); - close(); - onDismiss?.(); - }, [autoClose, close, onDismiss]); + useEffect(() => { + // Only enable 'quick dismiss' for popovers + if (position.type === "MODAL" || position.type === "DIALOG") return; + + const escapeHandler = (e: KeyboardEvent) => { + if (e.key == "Escape") { + dismiss(); + } + }; + + const clickOutsideHandler = (e: MouseEvent) => { + if ( + !(e.target instanceof Element) || + !e.target.closest(`#${feedbackContainerId}`) + ) { + dismiss(); + } + }; + + const observer = new MutationObserver((mutations) => { + if (position.anchor === null) return; + + mutations.forEach((mutation) => { + const removedNodes = Array.from(mutation.removedNodes); + const hasBeenRemoved = removedNodes.some((node) => { + return node === position.anchor || node.contains(position.anchor); + }); + + if (hasBeenRemoved) { + close(); + } + }); + }); + + window.addEventListener("mousedown", clickOutsideHandler); + window.addEventListener("keydown", escapeHandler); + observer.observe(document.body, { + subtree: true, + childList: true, + }); + + return () => { + window.removeEventListener("mousedown", clickOutsideHandler); + window.removeEventListener("keydown", escapeHandler); + observer.disconnect(); + }; + }, [position.type, close]); return ( <> - - <> - - - - - + + + + + {anchor && ( +
+ )} + ); }; + +function parseOffset(offsetInput?: Offset["x"] | Offset["y"]) { + if (offsetInput === undefined) return "1rem"; + if (typeof offsetInput === "number") return offsetInput + "px"; + + return offsetInput; +} diff --git a/packages/browser-sdk/src/feedback/ui/FeedbackForm.tsx b/packages/browser-sdk/src/feedback/ui/FeedbackForm.tsx index f1b7b446..949ce658 100644 --- a/packages/browser-sdk/src/feedback/ui/FeedbackForm.tsx +++ b/packages/browser-sdk/src/feedback/ui/FeedbackForm.tsx @@ -1,9 +1,8 @@ import { FunctionComponent, h } from "preact"; import { useCallback, useEffect, useRef, useState } from "preact/hooks"; -import { Check } from "../../ui/icons/Check"; -import { CheckCircle } from "../../ui/icons/CheckCircle"; - +import { Check } from "./icons/Check"; +import { CheckCircle } from "./icons/CheckCircle"; import { Button } from "./Button"; import { Plug } from "./Plug"; import { StarRating } from "./StarRating"; diff --git a/packages/browser-sdk/src/feedback/ui/Plug.tsx b/packages/browser-sdk/src/feedback/ui/Plug.tsx index f315708f..dc8add02 100644 --- a/packages/browser-sdk/src/feedback/ui/Plug.tsx +++ b/packages/browser-sdk/src/feedback/ui/Plug.tsx @@ -1,12 +1,12 @@ import { FunctionComponent, h } from "preact"; -import { Logo } from "../../ui/icons/Logo"; +import { Logo } from "./icons/Logo"; export const Plug: FunctionComponent = () => { return ( ); diff --git a/packages/browser-sdk/src/feedback/ui/StarRating.tsx b/packages/browser-sdk/src/feedback/ui/StarRating.tsx index e8e439a5..ffe6ec1a 100644 --- a/packages/browser-sdk/src/feedback/ui/StarRating.tsx +++ b/packages/browser-sdk/src/feedback/ui/StarRating.tsx @@ -1,17 +1,12 @@ import { Fragment, FunctionComponent, h } from "preact"; import { useRef } from "preact/hooks"; -import { Dissatisfied } from "../../ui/icons/Dissatisfied"; -import { Neutral } from "../../ui/icons/Neutral"; -import { Satisfied } from "../../ui/icons/Satisfied"; -import { VeryDissatisfied } from "../../ui/icons/VeryDissatisfied"; -import { VerySatisfied } from "../../ui/icons/VerySatisfied"; -import { - arrow, - offset, - useFloating, -} from "../../ui/packages/floating-ui-preact-dom"; - +import { Dissatisfied } from "./icons/Dissatisfied"; +import { Neutral } from "./icons/Neutral"; +import { Satisfied } from "./icons/Satisfied"; +import { VeryDissatisfied } from "./icons/VeryDissatisfied"; +import { VerySatisfied } from "./icons/VerySatisfied"; +import { arrow, offset, useFloating } from "./packages/floating-ui-preact-dom"; import { FeedbackTranslations } from "./types"; const scores = [ diff --git a/packages/browser-sdk/src/feedback/ui/config/defaultTranslations.tsx b/packages/browser-sdk/src/feedback/ui/config/defaultTranslations.tsx index c7edc9a2..9bf80413 100644 --- a/packages/browser-sdk/src/feedback/ui/config/defaultTranslations.tsx +++ b/packages/browser-sdk/src/feedback/ui/config/defaultTranslations.tsx @@ -1,4 +1,5 @@ import { FeedbackTranslations } from "../types"; + /** * {@includeCode ./defaultTranslations.tsx} */ diff --git a/packages/browser-sdk/src/ui/constants.ts b/packages/browser-sdk/src/feedback/ui/constants.ts similarity index 95% rename from packages/browser-sdk/src/ui/constants.ts rename to packages/browser-sdk/src/feedback/ui/constants.ts index 8d430e46..0dc6af35 100644 --- a/packages/browser-sdk/src/ui/constants.ts +++ b/packages/browser-sdk/src/feedback/ui/constants.ts @@ -2,7 +2,6 @@ * ID of HTML DIV element which contains the feedback dialog */ export const feedbackContainerId = "bucket-feedback-dialog-container"; -export const toolbarContainerId = "bucket-toolbar-dialog-container"; /** * These events will be propagated to the feedback dialog diff --git a/packages/browser-sdk/src/ui/icons/Check.tsx b/packages/browser-sdk/src/feedback/ui/icons/Check.tsx similarity index 100% rename from packages/browser-sdk/src/ui/icons/Check.tsx rename to packages/browser-sdk/src/feedback/ui/icons/Check.tsx diff --git a/packages/browser-sdk/src/ui/icons/CheckCircle.tsx b/packages/browser-sdk/src/feedback/ui/icons/CheckCircle.tsx similarity index 100% rename from packages/browser-sdk/src/ui/icons/CheckCircle.tsx rename to packages/browser-sdk/src/feedback/ui/icons/CheckCircle.tsx diff --git a/packages/browser-sdk/src/ui/icons/Close.tsx b/packages/browser-sdk/src/feedback/ui/icons/Close.tsx similarity index 100% rename from packages/browser-sdk/src/ui/icons/Close.tsx rename to packages/browser-sdk/src/feedback/ui/icons/Close.tsx diff --git a/packages/browser-sdk/src/ui/icons/Dissatisfied.tsx b/packages/browser-sdk/src/feedback/ui/icons/Dissatisfied.tsx similarity index 100% rename from packages/browser-sdk/src/ui/icons/Dissatisfied.tsx rename to packages/browser-sdk/src/feedback/ui/icons/Dissatisfied.tsx diff --git a/packages/browser-sdk/src/ui/icons/Logo.tsx b/packages/browser-sdk/src/feedback/ui/icons/Logo.tsx similarity index 98% rename from packages/browser-sdk/src/ui/icons/Logo.tsx rename to packages/browser-sdk/src/feedback/ui/icons/Logo.tsx index c520b8eb..2164fb89 100644 --- a/packages/browser-sdk/src/ui/icons/Logo.tsx +++ b/packages/browser-sdk/src/feedback/ui/icons/Logo.tsx @@ -1,9 +1,11 @@ import { FunctionComponent, h } from "preact"; export const Logo: FunctionComponent> = ( - props = { height: "10px", width: "10px" }, + props, ) => ( = T & { [P in K]-?: T[P] }; +export type FeedbackPlacement = + | "bottom-right" + | "bottom-left" + | "top-right" + | "top-left"; + +export type Offset = { + /** + * Offset from the nearest horizontal screen edge after placement is resolved + */ + x?: string | number; + /** + * Offset from the nearest vertical screen edge after placement is resolved + */ + y?: string | number; +}; + +export type FeedbackPosition = + | { type: "MODAL" } + | { type: "DIALOG"; placement: FeedbackPlacement; offset?: Offset } + | { type: "POPOVER"; anchor: HTMLElement | null }; + export interface FeedbackSubmission { question: string; feedbackId?: string; @@ -26,7 +46,7 @@ export interface OpenFeedbackFormOptions { /** * Control the placement and behavior of the feedback form. */ - position?: Position; + position?: FeedbackPosition; /** * Add your own custom translations for the feedback form. @@ -47,6 +67,7 @@ export interface OpenFeedbackFormOptions { onClose?: () => void; onDismiss?: () => void; } + /** * You can use this to override text values in the feedback form * with desired language translation diff --git a/packages/browser-sdk/src/index.ts b/packages/browser-sdk/src/index.ts index 2b4e7bf8..f8cd11db 100644 --- a/packages/browser-sdk/src/index.ts +++ b/packages/browser-sdk/src/index.ts @@ -1,16 +1,14 @@ -// import "preact/debug"; - -export type { Feature, InitOptions, ToolbarOptions } from "./client"; +export type { Feature, InitOptions } from "./client"; export { BucketClient } from "./client"; export type { BucketContext, CompanyContext, UserContext } from "./context"; export type { CheckEvent, - FallbackFeatureOverride, FeaturesOptions, RawFeature, RawFeatures, } from "./feature/features"; export type { + FeatureIdentifier, Feedback, FeedbackOptions, FeedbackPrompt, @@ -24,12 +22,15 @@ export type { UnassignedFeedback, } from "./feedback/feedback"; export type { DEFAULT_TRANSLATIONS } from "./feedback/ui/config/defaultTranslations"; +export { feedbackContainerId, propagatedEvents } from "./feedback/ui/constants"; export type { + FeedbackPlacement, + FeedbackPosition, FeedbackScoreSubmission, FeedbackSubmission, FeedbackTranslations, + Offset, OnScoreSubmitResult, OpenFeedbackFormOptions, } from "./feedback/ui/types"; export type { Logger } from "./logger"; -export { feedbackContainerId, propagatedEvents } from "./ui/constants"; diff --git a/packages/browser-sdk/src/toolbar/Features.css b/packages/browser-sdk/src/toolbar/Features.css deleted file mode 100644 index ee17134f..00000000 --- a/packages/browser-sdk/src/toolbar/Features.css +++ /dev/null @@ -1,74 +0,0 @@ -.search-input { - background: transparent; - border: none; - color: white; - width: 100%; - font-size: var(--text-size); - - &::placeholder { - color: var(--gray500); - } - - &::-webkit-search-cancel-button { - -webkit-appearance: none; - display: inline-block; - width: 8px; - height: 8px; - margin-left: 10px; - background: linear-gradient( - 45deg, - rgba(0, 0, 0, 0) 0%, - rgba(0, 0, 0, 0) 43%, - #fff 45%, - #fff 55%, - rgba(0, 0, 0, 0) 57%, - rgba(0, 0, 0, 0) 100% - ), - linear-gradient( - 135deg, - transparent 0%, - transparent 43%, - #fff 45%, - #fff 55%, - transparent 57%, - transparent 100% - ); - cursor: pointer; - } -} - -.features-table { - width: 100%; - border-collapse: collapse; -} - -.feature-name-cell { - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - width: auto; - padding: 6px 6px 6px 0; -} - -.feature-link { - color: var(--text-color); - text-decoration: none; - &:hover { - text-decoration: underline; - } -} - -.feature-reset-cell { - width: 30px; - padding: 6px 0; - text-align: right; -} - -.reset { - color: var(--brand300); -} - -.feature-switch-cell { - padding: 6px 0 6px 6px; - width: 0; -} diff --git a/packages/browser-sdk/src/toolbar/Features.tsx b/packages/browser-sdk/src/toolbar/Features.tsx deleted file mode 100644 index 9867a57d..00000000 --- a/packages/browser-sdk/src/toolbar/Features.tsx +++ /dev/null @@ -1,113 +0,0 @@ -import { h } from "preact"; - -import { Switch } from "./Switch"; -import { FeatureItem } from "./Toolbar"; - -export function FeaturesTable({ - features, - setEnabledOverride, - appBaseUrl, -}: { - features: FeatureItem[]; - setEnabledOverride: (key: string, value: boolean | null) => void; - appBaseUrl: string; -}) { - if (features.length === 0) { - return
No features found
; - } - return ( - - - {features.map((feature) => ( - - ))} - -
- ); -} - -function FeatureRow({ - setEnabledOverride, - appBaseUrl, - feature, -}: { - feature: FeatureItem; - appBaseUrl: string; - setEnabledOverride: (key: string, value: boolean | null) => void; -}) { - return ( - - - - {feature.key} - - - - {feature.localOverride !== null ? ( - - ) : null} - - - { - const isChecked = e.currentTarget.checked; - const isOverridden = isChecked !== feature.isEnabled; - setEnabledOverride(feature.key, isOverridden ? isChecked : null); - }} - /> - - - ); -} - -export function FeatureSearch({ - onSearch, -}: { - onSearch: (val: string) => void; -}) { - return ( - onSearch(s.currentTarget.value)} - autoFocus - class="search-input" - /> - ); -} - -function Reset({ - setEnabledOverride, - featureKey, -}: { - setEnabledOverride: (key: string, value: boolean | null) => void; - featureKey: string; -}) { - return ( - { - e.preventDefault(); - setEnabledOverride(featureKey, null); - }} - > - reset - - ); -} diff --git a/packages/browser-sdk/src/toolbar/Switch.css b/packages/browser-sdk/src/toolbar/Switch.css deleted file mode 100644 index 335d1b71..00000000 --- a/packages/browser-sdk/src/toolbar/Switch.css +++ /dev/null @@ -1,22 +0,0 @@ -.switch { - cursor: pointer; -} - -.switch-track { - position: relative; - transition: background 0.1s ease; - background: #606476; -} - -.switch[data-enabled="true"] .switch-track { - background: #847cfb; -} - -.switch-dot { - background: white; - border-radius: 50%; - position: absolute; - top: 1px; - transition: transform 0.1s ease-in-out; - box-shadow: 0 0px 5px rgba(0, 0, 0, 0.2); -} diff --git a/packages/browser-sdk/src/toolbar/Switch.tsx b/packages/browser-sdk/src/toolbar/Switch.tsx deleted file mode 100644 index deb212c9..00000000 --- a/packages/browser-sdk/src/toolbar/Switch.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import { Fragment, h } from "preact"; - -interface SwitchProps extends h.JSX.HTMLAttributes { - checked: boolean; - width?: number; - height?: number; -} - -const gutter = 1; - -export function Switch({ - checked, - width = 24, - height = 14, - ...props -}: SwitchProps) { - return ( - <> -