From c59474af9fed7277dc700a05cdceea5551b7d7ef Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Tue, 4 Feb 2025 10:45:13 +0000 Subject: [PATCH 01/15] feat(browser-sdk): enhance feature flag and configuration tracking - Add support for tracking rule evaluation results and missing context fields - Extend `CheckEvent` and `FetchedFeature` types to include additional metadata - Update client-side event sending to include more detailed feature flag information --- packages/browser-sdk/src/client.ts | 41 ++++++++++------ packages/browser-sdk/src/feature/features.ts | 49 +++++++++++++++++--- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index a3d34a19..efb73fd9 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -694,25 +694,38 @@ export class BucketClient { } : { key: undefined, payload: undefined }; - function sendCheckEvent() { - fClient - .sendCheckEvent({ - key, - version: f?.targetingVersion, - value, - }) - .catch(() => { - // ignore - }); - } - return { get isEnabled() { - sendCheckEvent(); + fClient + .sendCheckEvent({ + key, + version: f?.targetingVersion, + ruleEvaluationResults: f?.ruleEvaluationResults, + missingContextFields: f?.missingContextFields, + value, + }) + .catch(() => { + // ignore + }); return value; }, get config() { - sendCheckEvent(); + fClient + .sendCheckEvent({ + key, + version: f?.config?.version, + ruleEvaluationResults: f?.config?.ruleEvaluationResults, + missingContextFields: f?.config?.missingContextFields, + value: f?.config && { + key: f.config.key, + default: f.config.default, + payload: f.config.payload, + }, + }) + .catch(() => { + // ignore + }); + return config; }, track: () => this.track(key), diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 7c11420a..afe0a524 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -14,20 +14,30 @@ import { */ export type FetchedFeature = { /** - * Feature key + * Feature key. */ key: string; /** - * Result of feature flag evaluation + * Result of feature flag evaluation. */ isEnabled: boolean; /** - * Version of targeting rules + * Version of targeting rules. */ targetingVersion?: number; + /** + * Rule evaluation results. + */ + ruleEvaluationResults?: boolean[]; + + /** + * Missing context fields. + */ + missingContextFields?: string[]; + /** * Optional user-defined dynamic configuration. */ @@ -37,6 +47,11 @@ export type FetchedFeature = { */ key: string; + /** + * Indicated that the matched configuration value is the default. + */ + default?: boolean; + /** * The version of the matched configuration value. */ @@ -46,6 +61,16 @@ export type FetchedFeature = { * The optional user-supplied payload data. */ payload?: any; + + /** + * The rule evaluation results. + */ + ruleEvaluationResults?: boolean[]; + + /** + * The missing context fields. + */ + missingContextFields?: string[]; }; }; @@ -153,19 +178,29 @@ export function flattenJSON(obj: Record): Record { */ export interface CheckEvent { /** - * Feature key + * Feature key. */ key: string; /** - * Result of feature flag evaluation + * Result of feature flag or configuration evaluation. */ - value: boolean; + value: any; /** - * Version of targeting rules + * Version of targeting rules. */ version?: number; + + /** + * Rule evaluation results. + */ + ruleEvaluationResults?: boolean[]; + + /** + * Missing context fields. + */ + missingContextFields?: string[]; } type context = { From 5bc3816a48c17934903eef066da0e209924d82d9 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 06:52:33 +0000 Subject: [PATCH 02/15] feat(browser-sdk): improve feature flag event tracking details - Add `action` field to distinguish between feature flag and config check events - Include rule evaluation results and missing context fields in check events - Update test cases to reflect new event tracking capabilities --- packages/browser-sdk/src/client.ts | 2 ++ .../browser-sdk/src/feature/featureCache.ts | 6 +++++- packages/browser-sdk/src/feature/features.ts | 9 ++++++++- packages/browser-sdk/test/mocks/handlers.ts | 4 ++++ packages/browser-sdk/test/usage.test.ts | 18 +++++++++++++++--- 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index efb73fd9..aafeaed2 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -698,6 +698,7 @@ export class BucketClient { get isEnabled() { fClient .sendCheckEvent({ + action: "check", key, version: f?.targetingVersion, ruleEvaluationResults: f?.ruleEvaluationResults, @@ -712,6 +713,7 @@ export class BucketClient { get config() { fClient .sendCheckEvent({ + action: "check-config", key, version: f?.config?.version, ruleEvaluationResults: f?.config?.ruleEvaluationResults, diff --git a/packages/browser-sdk/src/feature/featureCache.ts b/packages/browser-sdk/src/feature/featureCache.ts index b4cb8ed8..fb19d90e 100644 --- a/packages/browser-sdk/src/feature/featureCache.ts +++ b/packages/browser-sdk/src/feature/featureCache.ts @@ -27,7 +27,9 @@ export function parseAPIFeaturesResponse( typeof feature.isEnabled !== "boolean" || feature.key !== key || typeof feature.targetingVersion !== "number" || - (feature.config && typeof feature.config !== "object") + (feature.config && typeof feature.config !== "object") || + (feature.missingContextFields && !Array.isArray(feature.missingContextFields)) || + (feature.ruleEvaluationResults && !Array.isArray(feature.ruleEvaluationResults)) ) { return; } @@ -37,6 +39,8 @@ export function parseAPIFeaturesResponse( targetingVersion: feature.targetingVersion, key, config: feature.config, + missingContextFields: feature.missingContextFields, + ruleEvaluationResults: feature.ruleEvaluationResults, }; } diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index afe0a524..4b3df73c 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -177,6 +177,11 @@ export function flattenJSON(obj: Record): Record { * Event representing checking the feature flag evaluation result */ export interface CheckEvent { + /** + * Action to perform. + */ + action: "check" | "check-config"; + /** * Feature key. */ @@ -400,11 +405,13 @@ export class FeaturesClient { await this.rateLimiter.rateLimited(rateLimitKey, async () => { const payload = { - action: "check", + action: checkEvent.action, key: checkEvent.key, targetingVersion: checkEvent.version, evalContext: this.context, evalResult: checkEvent.value, + evalRuleResults: checkEvent.ruleEvaluationResults, + evalMissingFields: checkEvent.missingContextFields, }; this.httpClient diff --git a/packages/browser-sdk/test/mocks/handlers.ts b/packages/browser-sdk/test/mocks/handlers.ts index 62fd5fe1..002e63f2 100644 --- a/packages/browser-sdk/test/mocks/handlers.ts +++ b/packages/browser-sdk/test/mocks/handlers.ts @@ -12,6 +12,8 @@ export const featureResponse = { key: "featureA", targetingVersion: 1, config: undefined, + ruleEvaluationResults: [false, true], + missingContextFields: ["field1", "field2"], }, featureB: { isEnabled: true, @@ -21,6 +23,8 @@ export const featureResponse = { version: 12, key: "gpt3", payload: { model: "gpt-something", temperature: 0.5 }, + ruleEvaluationResults: [true, false, false], + missingContextFields: ["field3"], }, }, }, diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 31399087..1b622ccc 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -441,6 +441,7 @@ describe(`sends "check" events `, () => { FeaturesClient.prototype, "sendCheckEvent", ); + const postSpy = vi.spyOn(HttpClient.prototype, "post"); const client = new BucketClient({ @@ -457,9 +458,12 @@ describe(`sends "check" events `, () => { expect(sendCheckEventSpy).toHaveBeenCalledTimes(1); expect(sendCheckEventSpy).toHaveBeenCalledWith({ + action: "check", key: "featureA", value: true, version: 1, + missingContextFields: ["field1", "field2"], + ruleEvaluationResults: [false, true], }); expect(postSpy).toHaveBeenCalledWith({ @@ -475,6 +479,8 @@ describe(`sends "check" events `, () => { }, }, evalResult: true, + evalRuleResults: [false, true], + evalMissingFields: ["field1", "field2"], key: "featureA", targetingVersion: 1, }, @@ -498,16 +504,21 @@ describe(`sends "check" events `, () => { expect(postSpy).toHaveBeenCalledWith({ body: { - action: "check", + action: "check-config", evalContext: { other: undefined, user: { id: "uid", }, }, - evalResult: true, + evalResult: { + key: "gpt3", + payload: { model: "gpt-something", temperature: 0.5 }, + }, + evalRuleResults: [true, false, false], + evalMissingFields: ["field3"], key: "featureB", - targetingVersion: 11, + targetingVersion: 12, }, path: "features/events", }); @@ -533,6 +544,7 @@ describe(`sends "check" events `, () => { expect( vi.mocked(FeaturesClient.prototype.sendCheckEvent), ).toHaveBeenCalledWith({ + action: "check", value: false, key: "non-existent", version: undefined, From d3c8ffae93d9a427407af4380806fdda54a24609 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 06:59:06 +0000 Subject: [PATCH 03/15] refactor(react-sdk): simplify feature flag retrieval and event tracking - Update `useFeature` hook to use client's `getFeature` method - Remove redundant feature tracking logic - Simplify feature flag and configuration access - Add null check for client to prevent potential errors --- packages/react-sdk/src/index.tsx | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index 835a094e..67d71fe1 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -213,7 +213,7 @@ type Feature = { */ export function useFeature(key: TKey): Feature { const { - features: { features, isLoading }, + features: { isLoading }, client, } = useContext(ProviderContext); @@ -221,7 +221,7 @@ export function useFeature(key: TKey): Feature { const requestFeedback = (opts: RequestFeedbackOptions) => client?.requestFeedback({ ...opts, featureKey: key }); - if (isLoading) { + if (isLoading || !client) { return { isLoading, isEnabled: false, @@ -231,36 +231,17 @@ export function useFeature(key: TKey): Feature { }; } - const feature = features[key]; - const enabled = feature?.isEnabledOverride ?? feature?.isEnabled ?? false; - - function sendCheckEvent() { - client - ?.sendCheckEvent({ - key, - value: enabled, - version: feature?.targetingVersion, - }) - .catch(() => { - // ignore - }); - } - - const reducedConfig = feature?.config - ? { key: feature.config.key, payload: feature.config.payload } - : { key: undefined, payload: undefined }; + const feature = client.getFeature(key); return { isLoading, track, requestFeedback, get isEnabled() { - sendCheckEvent(); - return enabled; + return feature.isEnabled ?? false; }, get config() { - sendCheckEvent(); - return reducedConfig; + return feature.config ?? { key: undefined, payload: undefined }; }, }; } From 5d62397e9910d1905df4d86c02f18e9d946f69a9 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 09:02:54 +0000 Subject: [PATCH 04/15] feat(node-sdk): enhance feature flag event tracking with config and evaluation details - Add support for new feature flag event actions: "check-config" and "evaluate-config" - Extend event tracking to include more detailed evaluation metadata - Update types to support richer feature flag and configuration event information - Improve logging and tracking of missing context fields - Enhance client-side event sending with comprehensive feature flag details --- packages/node-sdk/src/client.ts | 147 ++++++++--- packages/node-sdk/src/types.ts | 22 +- packages/node-sdk/test/client.test.ts | 343 +++++++++++++++++++++----- 3 files changed, 404 insertions(+), 108 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index dfe0278b..68fc4d2d 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -66,10 +66,13 @@ type BulkEvent = } | { type: "feature-flag-event"; - action: "check" | "evaluate"; + action: "check" | "evaluate" | "check-config" | "evaluate-config"; key: string; targetingVersion?: number; - evalResult: boolean; + evalResult: + | boolean + | { key: string; payload: any } + | { key: undefined; payload: undefined }; evalContext?: Record; evalRuleResults?: boolean[]; evalMissingFields?: string[]; @@ -477,6 +480,8 @@ export class BucketClient { isEnabled: feature?.isEnabled ?? false, targetingVersion: feature?.targetingVersion, config: feature?.config, + ruleEvaluationResults: feature?.ruleEvaluationResults, + missingContextFields: feature?.missingContextFields, }); } @@ -648,7 +653,10 @@ export class BucketClient { ok(typeof event === "object", "event must be an object"); ok( typeof event.action === "string" && - (event.action === "evaluate" || event.action === "check"), + (event.action === "evaluate" || + event.action === "evaluate-config" || + event.action === "check" || + event.action === "check-config"), "event must have an action", ); ok( @@ -661,7 +669,7 @@ export class BucketClient { "event must have a targeting version", ); ok( - typeof event.evalResult === "boolean", + typeof event.evalResult === "boolean" || isObject(event.evalResult), "event must have an evaluation result", ); ok( @@ -788,15 +796,44 @@ export class BucketClient { */ private warnMissingFeatureContextFields( options: Context, - features: { key: string; missingContextFields?: string[] }[], + features: { + key: string; + missingContextFields?: string[]; + config?: { + key: string; + missingContextFields?: string[]; + }; + }[], ) { - features.forEach(({ key, missingContextFields }) => { - if (missingContextFields?.length) { + features.forEach(({ config, ...feature }) => { + if (feature.missingContextFields?.length) { if ( !this._config.rateLimiter.isAllowed( hashObject({ - key, - missingContextFields, + key: feature.key, + missingContextFields: feature.missingContextFields, + options, + }), + ) + ) { + return; + } + + const missingFieldsStr = feature.missingContextFields + .map((field) => `"${field}"`) + .join(", "); + + this._config.logger?.warn( + `feature "${feature.key}" has targeting rules that require the following context fields: ${missingFieldsStr}`, + ); + } + + if (config?.missingContextFields?.length) { + if ( + !this._config.rateLimiter.isAllowed( + hashObject({ + key: config.key, + missingContextFields: config.missingContextFields, options, }), ) @@ -804,12 +841,12 @@ export class BucketClient { return; } - const missingFieldsStr = missingContextFields + const missingFieldsStr = config.missingContextFields .map((field) => `"${field}"`) .join(", "); this._config.logger?.warn( - `feature "${key}" has targeting rules that require the following context fields: ${missingFieldsStr}`, + `remote config "${config.key}" has targeting rules that require the following context fields: ${missingFieldsStr}`, ); } }); @@ -900,6 +937,19 @@ export class BucketClient { evalRuleResults: res.ruleEvaluationResults, evalMissingFields: res.missingContextFields, }); + + const config = evaluatedConfigs[res.featureKey]; + if (config) { + await this.sendFeatureEvent({ + action: "evaluate-config", + key: res.featureKey, + targetingVersion: config.targetingVersion, + evalResult: { key: config.key, payload: config.payload }, + evalContext: res.context, + evalRuleResults: config.ruleEvaluationResults, + evalMissingFields: config.missingContextFields, + }); + } } catch (err) { this._config.logger?.error( `failed to send evaluate event for "${res.featureKey}"`, @@ -915,8 +965,9 @@ export class BucketClient { key: res.featureKey, isEnabled: res.value ?? false, config: evaluatedConfigs[res.featureKey], - targetingVersion: featureMap[res.featureKey].targeting.version, + ruleEvaluationResults: res.ruleEvaluationResults, missingContextFields: res.missingContextFields, + targetingVersion: featureMap[res.featureKey].targeting.version, }; return acc; }, @@ -947,53 +998,69 @@ export class BucketClient { } private _wrapRawFeature( - options: { enableTracking: boolean } & Context, - { key, isEnabled, config, targetingVersion }: RawFeature, + { enableTracking, ...context }: { enableTracking: boolean } & Context, + { config, ...feature }: RawFeature, ): TypedFeatures[TKey] { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; - function sendCheckEvent() { - if (options.enableTracking) { - void client - .sendFeatureEvent({ - action: "check", - key, - targetingVersion, - evalResult: isEnabled, - }) - .catch((err) => { - client._config.logger?.error( - `failed to send check event for "${key}": ${err}`, - err, - ); - }); - } - } - const simplifiedConfig = config ? { key: config.key, payload: config.payload } : { key: undefined, payload: undefined }; return { get isEnabled() { - sendCheckEvent(); - return isEnabled; + if (enableTracking) { + void client + .sendFeatureEvent({ + action: "check", + key: feature.key, + targetingVersion: feature.targetingVersion, + evalResult: feature.isEnabled, + evalContext: context, + evalRuleResults: feature.ruleEvaluationResults, + evalMissingFields: feature.missingContextFields, + }) + .catch((err) => { + client._config.logger?.error( + `failed to send check event for "${feature.key}": ${err}`, + err, + ); + }); + } + return feature.isEnabled; }, get config() { - sendCheckEvent(); + if (enableTracking) { + void client + .sendFeatureEvent({ + action: "check-config", + key: feature.key, + targetingVersion: config?.targetingVersion, + evalResult: simplifiedConfig, + evalContext: context, + evalRuleResults: config?.ruleEvaluationResults, + evalMissingFields: config?.missingContextFields, + }) + .catch((err) => { + client._config.logger?.error( + `failed to send check event for "${feature.key}": ${err}`, + err, + ); + }); + } return simplifiedConfig as TypedFeatures[TKey]["config"]; }, - key, + key: feature.key, track: async () => { - if (typeof options.user?.id === "undefined") { + if (typeof context.user?.id === "undefined") { this._config.logger?.warn("no user set, cannot track event"); return; } - if (options.enableTracking) { - await this.track(options.user.id, key, { - companyId: options.company?.id, + if (enableTracking) { + await this.track(context.user.id, feature.key, { + companyId: context.company?.id, }); } else { this._config.logger?.debug("tracking disabled, not tracking event"); diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index 28e67598..43ed42b5 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -22,7 +22,7 @@ export type FeatureEvent = { /** * The action that was performed. **/ - action: "evaluate" | "check"; + action: "evaluate" | "evaluate-config" | "check" | "check-config"; /** * The feature key. @@ -37,7 +37,10 @@ export type FeatureEvent = { /** * The result of targeting evaluation. **/ - evalResult: boolean; + evalResult: + | boolean + | { key: string; payload: any } + | { key: undefined; payload: undefined }; /** * The context that was used for evaluation. @@ -78,6 +81,16 @@ export type RawFeatureRemoteConfig = { * The optional user-supplied payload data. */ payload: any; + + /** + * The rule results of the evaluation (optional). + */ + ruleEvaluationResults?: boolean[]; + + /** + * The missing fields in the evaluation context (optional). + */ + missingContextFields?: string[]; }; /** @@ -104,6 +117,11 @@ export interface RawFeature { */ config?: RawFeatureRemoteConfig; + /** + * The rule results of the evaluation (optional). + */ + ruleEvaluationResults?: boolean[]; + /** * The missing fields in the evaluation context (optional). */ diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index bc96e128..90c9f3d8 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1057,12 +1057,14 @@ describe("BucketClient", () => { user, other: otherContext, }; + // test that the feature is returned await client.initialize(); const feature = client.getFeature( { ...context, enableTracking: true }, "feature1", ); + await feature.track(); await client.flush(); @@ -1118,6 +1120,21 @@ describe("BucketClient", () => { userId: user.id, companyId: company.id, }, + { + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + targetingVersion: 1, + evalContext: context, + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + evalRuleResults: undefined, + evalMissingFields: undefined, + }, ], ); }); @@ -1157,10 +1174,18 @@ describe("BucketClient", () => { { type: "feature-flag-event", action: "check", - evalResult: true, - targetingVersion: 1, key: "feature1", + targetingVersion: 1, + evalResult: true, + evalContext: context, + evalRuleResults: [true], + evalMissingFields: [], }, + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + }), ], ); }); @@ -1199,11 +1224,24 @@ describe("BucketClient", () => { }), { type: "feature-flag-event", - action: "check", - evalResult: true, - targetingVersion: 1, + action: "check-config", key: "feature1", + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + targetingVersion: 1, + evalContext: context, + evalRuleResults: undefined, + evalMissingFields: undefined, }, + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + }), ], ); }); @@ -1214,6 +1252,7 @@ describe("BucketClient", () => { user, other: otherContext, }; + // test that the feature is returned await client.initialize(); const feature = client.getFeature(context, "unknown-feature"); @@ -1233,31 +1272,25 @@ describe("BucketClient", () => { expect.objectContaining({ type: "user", }), - { + expect.objectContaining({ type: "feature-flag-event", action: "evaluate", key: "feature1", - targetingVersion: 1, - evalContext: context, - evalResult: true, - evalRuleResults: [true], - evalMissingFields: [], - }, - { + }), + expect.objectContaining({ type: "feature-flag-event", action: "evaluate", key: "feature2", - targetingVersion: 2, - evalContext: context, - evalResult: false, - evalRuleResults: [false], - evalMissingFields: ["something"], - }, + }), { type: "feature-flag-event", action: "check", - evalResult: false, key: "unknown-feature", + targetingVersion: undefined, + evalContext: context, + evalResult: false, + evalRuleResults: undefined, + evalMissingFields: undefined, }, { type: "event", @@ -1265,6 +1298,11 @@ describe("BucketClient", () => { userId: user.id, companyId: company.id, }, + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + }), ], ); }); @@ -1395,19 +1433,88 @@ describe("BucketClient", () => { evalRuleResults: [false], evalMissingFields: ["something"], }, + { + action: "check-config", + evalContext: { + company, + user, + other: otherContext, + }, + evalResult: { + key: undefined, + payload: undefined, + }, + key: "feature2", + type: "feature-flag-event", + targetingVersion: undefined, + evalRuleResults: undefined, + evalMissingFields: undefined, + }, { action: "check", + evalContext: { + company, + user, + other: otherContext, + }, evalResult: false, key: "feature2", targetingVersion: 2, type: "feature-flag-event", + evalMissingFields: ["something"], + evalRuleResults: [false], + }, + { + action: "check-config", + evalContext: { + company, + user, + other: otherContext, + }, + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + key: "feature1", + targetingVersion: 1, + type: "feature-flag-event", + evalRuleResults: undefined, + evalMissingFields: undefined, }, { action: "check", + evalContext: { + company, + user, + other: otherContext, + }, evalResult: true, key: "feature1", targetingVersion: 1, type: "feature-flag-event", + evalRuleResults: [true], + evalMissingFields: [], + }, + { + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + targetingVersion: 1, + evalContext: { + company, + user, + other: otherContext, + }, + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + evalRuleResults: undefined, + evalMissingFields: undefined, }, ], ); @@ -1477,44 +1584,62 @@ describe("BucketClient", () => { expectedHeaders, [ expect.objectContaining({ type: "user" }), - { + expect.objectContaining({ type: "feature-flag-event", action: "evaluate", key: "feature1", - targetingVersion: 1, evalContext: { user, }, - evalResult: true, - evalRuleResults: [true], - evalMissingFields: [], - }, - { + }), + expect.objectContaining({ type: "feature-flag-event", action: "evaluate", key: "feature2", - targetingVersion: 2, evalContext: { user, }, - evalResult: false, - evalRuleResults: [false], - evalMissingFields: ["something"], - }, - { + }), + expect.objectContaining({ + type: "feature-flag-event", + action: "check-config", + evalContext: { + user, + }, + key: "feature2", + }), + expect.objectContaining({ + type: "feature-flag-event", action: "check", - evalResult: false, + evalContext: { + user, + }, key: "feature2", - targetingVersion: 2, + }), + expect.objectContaining({ + type: "feature-flag-event", + action: "check-config", + evalContext: { + user, + }, + key: "feature1", + }), + expect.objectContaining({ type: "feature-flag-event", - }, - { action: "check", - evalResult: true, + evalContext: { + user, + }, key: "feature1", - targetingVersion: 1, + }), + expect.objectContaining({ type: "feature-flag-event", - }, + action: "evaluate-config", + key: "feature1", + evalContext: { + user, + }, + }), ], ); }); @@ -1554,44 +1679,62 @@ describe("BucketClient", () => { expectedHeaders, [ expect.objectContaining({ type: "company" }), - { + expect.objectContaining({ type: "feature-flag-event", action: "evaluate", key: "feature1", - targetingVersion: 1, evalContext: { company, }, - evalResult: true, - evalRuleResults: [true], - evalMissingFields: [], - }, - { + }), + expect.objectContaining({ type: "feature-flag-event", action: "evaluate", key: "feature2", - targetingVersion: 2, evalContext: { company, }, - evalResult: false, - evalRuleResults: [false], - evalMissingFields: ["something"], - }, - { + }), + expect.objectContaining({ + type: "feature-flag-event", + action: "check-config", + key: "feature2", + evalContext: { + company, + }, + }), + expect.objectContaining({ + type: "feature-flag-event", action: "check", - evalResult: false, key: "feature2", - targetingVersion: 2, + evalContext: { + company, + }, + }), + expect.objectContaining({ + type: "feature-flag-event", + action: "check-config", + key: "feature1", + evalContext: { + company, + }, + }), + expect.objectContaining({ type: "feature-flag-event", - }, - { action: "check", - evalResult: true, key: "feature1", - targetingVersion: 1, + evalContext: { + company, + }, + }), + expect.objectContaining({ type: "feature-flag-event", - }, + action: "evaluate-config", + key: "feature1", + evalContext: { + company, + }, + }), ], ); }); @@ -1664,6 +1807,23 @@ describe("BucketClient", () => { evalRuleResults: [false], evalMissingFields: ["something"], }, + { + type: "feature-flag-event", + action: "evaluate-config", + evalContext: { + other: otherContext, + }, + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + key: "feature1", + targetingVersion: 1, + evalMissingFields: undefined, + evalRuleResults: undefined, + }, ], ); }); @@ -1671,12 +1831,14 @@ describe("BucketClient", () => { it("should send `track` with user and company if provided", async () => { await client.initialize(); const feature1 = client.getFeature({ company, user }, "feature1"); + await client.flush(); await feature1.track(); await client.flush(); - expect(httpClient.post).toHaveBeenCalledTimes(1); - expect(httpClient.post).toHaveBeenCalledWith( + expect(httpClient.post).toHaveBeenCalledTimes(2); + expect(httpClient.post).toHaveBeenNthCalledWith( + 1, BULK_ENDPOINT, expectedHeaders, [ @@ -1689,6 +1851,7 @@ describe("BucketClient", () => { expect.objectContaining({ type: "feature-flag-event", action: "evaluate", + key: "feature1", evalContext: { company, user, @@ -1697,7 +1860,24 @@ describe("BucketClient", () => { expect.objectContaining({ type: "feature-flag-event", action: "evaluate", + key: "feature2", }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + evalContext: { + company, + user, + }, + }), + ], + ); + expect(httpClient.post).toHaveBeenNthCalledWith( + 2, + BULK_ENDPOINT, + expectedHeaders, + [ { companyId: "company123", event: "feature1", @@ -1712,11 +1892,13 @@ describe("BucketClient", () => { await client.initialize(); const feature = client.getFeature({ user }, "feature1"); + await client.flush(); await feature.track(); await client.flush(); - expect(httpClient.post).toHaveBeenCalledTimes(1); - expect(httpClient.post).toHaveBeenCalledWith( + expect(httpClient.post).toHaveBeenCalledTimes(2); + expect(httpClient.post).toHaveBeenNthCalledWith( + 1, BULK_ENDPOINT, expectedHeaders, [ @@ -1726,14 +1908,31 @@ describe("BucketClient", () => { expect.objectContaining({ type: "feature-flag-event", action: "evaluate", + key: "feature1", evalContext: { user, }, }), expect.objectContaining({ type: "feature-flag-event", + key: "feature2", action: "evaluate", }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + evalContext: { + user, + }, + }), + ], + ); + expect(httpClient.post).toHaveBeenNthCalledWith( + 2, + BULK_ENDPOINT, + expectedHeaders, + [ { event: "feature1", type: "event", @@ -1770,6 +1969,13 @@ describe("BucketClient", () => { type: "feature-flag-event", action: "evaluate", }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + evalContext: { + company, + }, + }), ], ); }); @@ -1806,12 +2012,17 @@ describe("BucketClient", () => { expectedHeaders, [ expect.objectContaining({ type: "user" }), - { + expect.objectContaining({ + type: "feature-flag-event", + action: "check-config", + key: "key", + }), + expect.objectContaining({ type: "feature-flag-event", action: "check", key: "key", evalResult: true, - }, + }), ], ); }); From e09227db06457aabff275b116a55a96fe0540a24 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 09:09:00 +0000 Subject: [PATCH 05/15] feat(node-sdk): include rule evaluation details in feature flag response - Add `ruleEvaluationResults` and `missingContextFields` to feature flag variant details - Update client to expose more comprehensive feature flag evaluation information - Enhance test cases to reflect new response structure with evaluation metadata --- packages/node-sdk/src/client.ts | 2 ++ packages/node-sdk/test/client.test.ts | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 68fc4d2d..cf941eeb 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -909,6 +909,8 @@ export class BucketClient { acc[featureKey] = { ...variant.value, targetingVersion: feature.config.version, + ruleEvaluationResults: variant.ruleEvaluationResults, + missingContextFields: variant.missingContextFields, }; } } diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index 90c9f3d8..a5ead5ca 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1132,8 +1132,8 @@ describe("BucketClient", () => { something: "else", }, }, - evalRuleResults: undefined, - evalMissingFields: undefined, + evalRuleResults: [true], + evalMissingFields: [], }, ], ); @@ -1234,8 +1234,8 @@ describe("BucketClient", () => { }, targetingVersion: 1, evalContext: context, - evalRuleResults: undefined, - evalMissingFields: undefined, + evalRuleResults: [true], + evalMissingFields: [], }, expect.objectContaining({ type: "feature-flag-event", @@ -1480,8 +1480,8 @@ describe("BucketClient", () => { key: "feature1", targetingVersion: 1, type: "feature-flag-event", - evalRuleResults: undefined, - evalMissingFields: undefined, + evalRuleResults: [true], + evalMissingFields: [], }, { action: "check", @@ -1513,8 +1513,8 @@ describe("BucketClient", () => { something: "else", }, }, - evalRuleResults: undefined, - evalMissingFields: undefined, + evalRuleResults: [true], + evalMissingFields: [], }, ], ); @@ -1821,8 +1821,8 @@ describe("BucketClient", () => { }, key: "feature1", targetingVersion: 1, - evalMissingFields: undefined, - evalRuleResults: undefined, + evalMissingFields: [], + evalRuleResults: [true], }, ], ); From 2c3c7513c6d6d9fdc4310b182d79985f3d510b6f Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 09:32:59 +0000 Subject: [PATCH 06/15] refactor(node-sdk): improve feature flag event tracking performance and error handling - Optimize event tracking by using Promise.allSettled for concurrent event sending - Flatten and collect promises to handle multiple event tracking scenarios - Improve error logging for failed event tracking attempts - Update test cases to reflect new event tracking approach --- packages/node-sdk/src/client.ts | 65 +++++---- packages/node-sdk/test/client.test.ts | 182 +++++++++++++------------- 2 files changed, 131 insertions(+), 116 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index cf941eeb..d27bad80 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -928,35 +928,50 @@ export class BucketClient { ); if (enableTracking) { - evaluated.forEach(async (res) => { - try { - await this.sendFeatureEvent({ - action: "evaluate", - key: res.featureKey, - targetingVersion: featureMap[res.featureKey].targeting.version, - evalResult: res.value ?? false, - evalContext: res.context, - evalRuleResults: res.ruleEvaluationResults, - evalMissingFields: res.missingContextFields, - }); + const promises = evaluated + .map(async (res) => { + const promises = []; + promises.push( + this.sendFeatureEvent({ + action: "evaluate", + key: res.featureKey, + targetingVersion: featureMap[res.featureKey].targeting.version, + evalResult: res.value ?? false, + evalContext: res.context, + evalRuleResults: res.ruleEvaluationResults, + evalMissingFields: res.missingContextFields, + }), + ); const config = evaluatedConfigs[res.featureKey]; if (config) { - await this.sendFeatureEvent({ - action: "evaluate-config", - key: res.featureKey, - targetingVersion: config.targetingVersion, - evalResult: { key: config.key, payload: config.payload }, - evalContext: res.context, - evalRuleResults: config.ruleEvaluationResults, - evalMissingFields: config.missingContextFields, - }); + promises.push( + this.sendFeatureEvent({ + action: "evaluate-config", + key: res.featureKey, + targetingVersion: config.targetingVersion, + evalResult: { key: config.key, payload: config.payload }, + evalContext: res.context, + evalRuleResults: config.ruleEvaluationResults, + evalMissingFields: config.missingContextFields, + }), + ); } - } catch (err) { - this._config.logger?.error( - `failed to send evaluate event for "${res.featureKey}"`, - err, - ); + + return promises; + }) + .flat(); + + Promise.allSettled(promises).then((results) => { + const failed = results + .map((result) => + result.status === "rejected" ? result.reason : undefined, + ) + .filter(Boolean); + if (failed.length > 0) { + this._config.logger?.error(`failed to queue some evaluate events.`, { + errors: failed, + }); } }); } diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index a5ead5ca..d70f0b3b 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1104,6 +1104,21 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, + { + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + targetingVersion: 1, + evalContext: context, + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + evalRuleResults: [true], + evalMissingFields: [], + }, { type: "feature-flag-event", action: "evaluate", @@ -1120,21 +1135,6 @@ describe("BucketClient", () => { userId: user.id, companyId: company.id, }, - { - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - targetingVersion: 1, - evalContext: context, - evalResult: { - key: "config-1", - payload: { - something: "else", - }, - }, - evalRuleResults: [true], - evalMissingFields: [], - }, ], ); }); @@ -1166,6 +1166,11 @@ describe("BucketClient", () => { action: "evaluate", key: "feature1", }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate", @@ -1181,11 +1186,6 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - }), ], ); }); @@ -1217,6 +1217,11 @@ describe("BucketClient", () => { action: "evaluate", key: "feature1", }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate", @@ -1237,11 +1242,6 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - }), ], ); }); @@ -1277,6 +1277,11 @@ describe("BucketClient", () => { action: "evaluate", key: "feature1", }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate", @@ -1298,11 +1303,6 @@ describe("BucketClient", () => { userId: user.id, companyId: company.id, }, - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - }), ], ); }); @@ -1419,6 +1419,25 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, + { + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + targetingVersion: 1, + evalContext: { + company, + user, + other: otherContext, + }, + evalResult: { + key: "config-1", + payload: { + something: "else", + }, + }, + evalRuleResults: [true], + evalMissingFields: [], + }, { type: "feature-flag-event", action: "evaluate", @@ -1497,25 +1516,6 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, - { - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - targetingVersion: 1, - evalContext: { - company, - user, - other: otherContext, - }, - evalResult: { - key: "config-1", - payload: { - something: "else", - }, - }, - evalRuleResults: [true], - evalMissingFields: [], - }, ], ); }); @@ -1592,6 +1592,14 @@ describe("BucketClient", () => { user, }, }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate-config", + key: "feature1", + evalContext: { + user, + }, + }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate", @@ -1632,14 +1640,6 @@ describe("BucketClient", () => { }, key: "feature1", }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - evalContext: { - user, - }, - }), ], ); }); @@ -1689,15 +1689,15 @@ describe("BucketClient", () => { }), expect.objectContaining({ type: "feature-flag-event", - action: "evaluate", - key: "feature2", + action: "evaluate-config", + key: "feature1", evalContext: { company, }, }), expect.objectContaining({ type: "feature-flag-event", - action: "check-config", + action: "evaluate", key: "feature2", evalContext: { company, @@ -1705,7 +1705,7 @@ describe("BucketClient", () => { }), expect.objectContaining({ type: "feature-flag-event", - action: "check", + action: "check-config", key: "feature2", evalContext: { company, @@ -1713,15 +1713,15 @@ describe("BucketClient", () => { }), expect.objectContaining({ type: "feature-flag-event", - action: "check-config", - key: "feature1", + action: "check", + key: "feature2", evalContext: { company, }, }), expect.objectContaining({ type: "feature-flag-event", - action: "check", + action: "check-config", key: "feature1", evalContext: { company, @@ -1729,7 +1729,7 @@ describe("BucketClient", () => { }), expect.objectContaining({ type: "feature-flag-event", - action: "evaluate-config", + action: "check", key: "feature1", evalContext: { company, @@ -1795,18 +1795,6 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, - { - type: "feature-flag-event", - action: "evaluate", - key: "feature2", - targetingVersion: 2, - evalContext: { - other: otherContext, - }, - evalResult: false, - evalRuleResults: [false], - evalMissingFields: ["something"], - }, { type: "feature-flag-event", action: "evaluate-config", @@ -1824,6 +1812,18 @@ describe("BucketClient", () => { evalMissingFields: [], evalRuleResults: [true], }, + { + type: "feature-flag-event", + action: "evaluate", + key: "feature2", + targetingVersion: 2, + evalContext: { + other: otherContext, + }, + evalResult: false, + evalRuleResults: [false], + evalMissingFields: ["something"], + }, ], ); }); @@ -1857,11 +1857,6 @@ describe("BucketClient", () => { user, }, }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate", - key: "feature2", - }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate-config", @@ -1871,6 +1866,11 @@ describe("BucketClient", () => { user, }, }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate", + key: "feature2", + }), ], ); expect(httpClient.post).toHaveBeenNthCalledWith( @@ -1913,11 +1913,6 @@ describe("BucketClient", () => { user, }, }), - expect.objectContaining({ - type: "feature-flag-event", - key: "feature2", - action: "evaluate", - }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate-config", @@ -1926,6 +1921,11 @@ describe("BucketClient", () => { user, }, }), + expect.objectContaining({ + type: "feature-flag-event", + key: "feature2", + action: "evaluate", + }), ], ); expect(httpClient.post).toHaveBeenNthCalledWith( @@ -1965,10 +1965,6 @@ describe("BucketClient", () => { company, }, }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate", - }), expect.objectContaining({ type: "feature-flag-event", action: "evaluate-config", @@ -1976,6 +1972,10 @@ describe("BucketClient", () => { company, }, }), + expect.objectContaining({ + type: "feature-flag-event", + action: "evaluate", + }), ], ); }); From cbb7140d51118099e175331feccb1825ab80769f Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 09:54:05 +0000 Subject: [PATCH 07/15] refactor(node-sdk): improve missing context fields warning mechanism - Refactor `warnMissingFeatureContextFields` to use a more efficient reporting approach - Consolidate missing context field warnings into a single log message - Add support for tracking missing fields for both features and their configurations - Update rate limiting to prevent duplicate warnings - Enhance logging with a comprehensive report of missing context fields --- packages/node-sdk/src/client.ts | 55 ++++++++++++--------------- packages/node-sdk/test/client.test.ts | 47 +++++++++++++++++++---- 2 files changed, 64 insertions(+), 38 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index d27bad80..9fe0f207 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -791,11 +791,11 @@ export class BucketClient { /** * Warns if any features have targeting rules that require context fields that are missing. * - * @param options - The options. + * @param context - The context. * @param features - The features to check. */ private warnMissingFeatureContextFields( - options: Context, + context: Context, features: { key: string; missingContextFields?: string[]; @@ -805,51 +805,46 @@ export class BucketClient { }; }[], ) { - features.forEach(({ config, ...feature }) => { - if (feature.missingContextFields?.length) { + const report = features.reduce( + (acc, { config, ...feature }) => { if ( - !this._config.rateLimiter.isAllowed( + feature.missingContextFields?.length && + this._config.rateLimiter.isAllowed( hashObject({ - key: feature.key, + featureKey: feature.key, missingContextFields: feature.missingContextFields, - options, + context, }), ) ) { - return; + acc[feature.key] = feature.missingContextFields; } - const missingFieldsStr = feature.missingContextFields - .map((field) => `"${field}"`) - .join(", "); - - this._config.logger?.warn( - `feature "${feature.key}" has targeting rules that require the following context fields: ${missingFieldsStr}`, - ); - } - - if (config?.missingContextFields?.length) { if ( - !this._config.rateLimiter.isAllowed( + config?.missingContextFields?.length && + this._config.rateLimiter.isAllowed( hashObject({ - key: config.key, + featureKey: feature.key, + configKey: config.key, missingContextFields: config.missingContextFields, - options, + context, }), ) ) { - return; + acc[`${feature.key}.config`] = config.missingContextFields; } - const missingFieldsStr = config.missingContextFields - .map((field) => `"${field}"`) - .join(", "); + return acc; + }, + {} as Record, + ); - this._config.logger?.warn( - `remote config "${config.key}" has targeting rules that require the following context fields: ${missingFieldsStr}`, - ); - } - }); + if (Object.keys(report).length > 0) { + this._config.logger?.warn( + `feature/remote config targeting rules might not be correctly evaluated due to missing context fields.`, + report, + ); + } } private _getFeatures( diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index d70f0b3b..810c3ee5 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1531,9 +1531,10 @@ describe("BucketClient", () => { }); expect(logger.warn).toHaveBeenCalledWith( - expect.stringMatching( - 'feature "feature2" has targeting rules that require the following context fields: "something"', - ), + "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", + { + feature2: ["something"], + }, ); }); @@ -2180,13 +2181,20 @@ describe("BucketClient", () => { version: 3, default: true, payload: { something: "else" }, + missingContextFields: ["funny"], }, + missingContextFields: ["something", "funny"], }, feature2: { key: "feature2", targetingVersion: 2, isEnabled: false, - missingContextFields: ["something"], + missingContextFields: ["another"], + }, + feature3: { + key: "feature3", + targetingVersion: 5, + isEnabled: true, }, }, }, @@ -2220,6 +2228,12 @@ describe("BucketClient", () => { config: { key: undefined, payload: undefined }, track: expect.any(Function), }, + feature3: { + key: "feature3", + isEnabled: true, + config: { key: undefined, payload: undefined }, + track: expect.any(Function), + }, }); expect(httpClient.get).toHaveBeenCalledTimes(1); @@ -2244,7 +2258,12 @@ describe("BucketClient", () => { it("should warn if missing context fields", async () => { await client.getFeaturesRemote(); expect(logger.warn).toHaveBeenCalledWith( - 'feature "feature2" has targeting rules that require the following context fields: "something"', + "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", + { + feature1: ["something", "funny"], + "feature1.config": ["funny"], + feature2: ["another"], + }, ); }); }); @@ -2269,6 +2288,7 @@ describe("BucketClient", () => { version: 3, default: true, payload: { something: "else" }, + missingContextFields: ["two"], }, missingContextFields: ["one", "two"], }, @@ -2318,7 +2338,11 @@ describe("BucketClient", () => { it("should warn if missing context fields", async () => { await client.getFeatureRemote("feature1"); expect(logger.warn).toHaveBeenCalledWith( - 'feature "feature1" has targeting rules that require the following context fields: "one", "two"', + "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", + { + feature1: ["one", "two"], + "feature1.config": ["two"], + }, ); }); }); @@ -2495,6 +2519,13 @@ describe("BoundBucketClient", () => { key: "feature1", targetingVersion: 1, isEnabled: true, + config: { + key: "config-1", + version: 3, + default: true, + payload: { something: "else" }, + missingContextFields: ["else"], + }, }, feature2: { key: "feature2", @@ -2520,7 +2551,7 @@ describe("BoundBucketClient", () => { feature1: { key: "feature1", isEnabled: true, - config: { key: undefined, payload: undefined }, + config: { key: "config-1", payload: { something: "else" } }, track: expect.any(Function), }, feature2: { @@ -2551,7 +2582,7 @@ describe("BoundBucketClient", () => { expect(result).toStrictEqual({ key: "feature1", isEnabled: true, - config: { key: undefined, payload: undefined }, + config: { key: "config-1", payload: { something: "else" } }, track: expect.any(Function), }); From fb04253d9ce3f4c409119a251e685ef9dacfe487 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 10:14:28 +0000 Subject: [PATCH 08/15] feat(browser-sdk): add warning mechanism for missing feature context fields - Implement `warnMissingFeatureContextFields` method to log context field warnings - Update rate-limited logging for missing context fields - Modify `sendCheckEvent` to prefix rate limit key for better tracking - Update test logger to use vitest mock functions - Add test case for missing context fields warning mechanism --- packages/browser-sdk/src/feature/features.ts | 29 +++++++++++++++++++- packages/browser-sdk/test/features.test.ts | 27 ++++++++++++++++++ packages/browser-sdk/test/testLogger.ts | 22 +++++---------- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 4b3df73c..fbaaccdf 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -401,7 +401,7 @@ export class FeaturesClient { * @param checkEvent - The feature to send the event for. */ async sendCheckEvent(checkEvent: CheckEvent) { - const rateLimitKey = `${this.fetchParams().toString()}:${checkEvent.key}:${checkEvent.version}:${checkEvent.value}`; + const rateLimitKey = `check-event:${this.fetchParams().toString()}:${checkEvent.key}:${checkEvent.version}:${checkEvent.value}`; await this.rateLimiter.rateLimited(rateLimitKey, async () => { const payload = { @@ -476,6 +476,32 @@ export class FeaturesClient { return params; } + private warnMissingFeatureContextFields(features: FetchedFeatures) { + const report: Record = {}; + for (const featureKey in features) { + const feature = features[featureKey]; + if (feature?.missingContextFields?.length) { + report[feature.key] = feature.missingContextFields; + } + + if (feature?.config?.missingContextFields?.length) { + report[`${feature.key}.config`] = feature.config.missingContextFields; + } + } + + if (Object.keys(report).length > 0) { + this.rateLimiter.rateLimited( + `feature-missing-context-fields:${this.fetchParams().toString()}`, + () => { + this.logger.warn( + `feature/remote config targeting rules might not be correctly evaluated due to missing context fields.`, + report, + ); + }, + ); + } + } + private async maybeFetchFeatures(): Promise { const cacheKey = this.fetchParams().toString(); const cachedItem = this.cache.get(cacheKey); @@ -511,6 +537,7 @@ export class FeaturesClient { features: fetchedFeatures, }); + this.warnMissingFeatureContextFields(fetchedFeatures); return fetchedFeatures; } diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index 9213a939..7e9f98a4 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -61,6 +61,10 @@ function featuresClientFactory() { } describe("FeaturesClient", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + test("fetches features", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); const featuresClient = newFeaturesClient(); @@ -92,6 +96,29 @@ describe("FeaturesClient", () => { expect(timeoutMs).toEqual(5000); }); + test("warns about missing context fields", async () => { + const { newFeaturesClient } = featuresClientFactory(); + const featuresClient = newFeaturesClient(); + + await featuresClient.initialize(); + + expect(testLogger.warn).toHaveBeenCalledTimes(1); + expect(testLogger.warn).toHaveBeenCalledWith( + "[Features] feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", + { + featureA: ["field1", "field2"], + "featureB.config": ["field3"], + }, + ); + + vi.advanceTimersByTime(TEST_STALE_MS + 1); + + expect(testLogger.warn).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(60 * 1000); + await featuresClient.initialize(); + expect(testLogger.warn).toHaveBeenCalledTimes(2); + }); + test("ignores undefined context", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); const featuresClient = newFeaturesClient( diff --git a/packages/browser-sdk/test/testLogger.ts b/packages/browser-sdk/test/testLogger.ts index 5bf71ad8..93bf2740 100644 --- a/packages/browser-sdk/test/testLogger.ts +++ b/packages/browser-sdk/test/testLogger.ts @@ -1,17 +1,9 @@ +import { vi } from "vitest"; + export const testLogger = { - log() { - //do nothing - }, - info() { - //do nothing - }, - warn() { - //do nothing - }, - error() { - //do nothing - }, - debug() { - //do nothing - }, + log: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), }; From f1f77cd85b6dfb794dfdb6f1643e8beb35984489 Mon Sep 17 00:00:00 2001 From: Alexandru Ciobanu Date: Wed, 5 Feb 2025 10:52:20 +0000 Subject: [PATCH 09/15] docs(browser-sdk,node-sdk): enhance documentation with new features and usage patterns - Update README for browser and node SDKs with comprehensive feature documentation - Add sections on feature overrides, error handling, and advanced configuration - Improve TypeScript type examples and usage guidance - Include detailed explanations for batch operations, rate limiting, and caching - Enhance documentation formatting and readability --- packages/browser-sdk/README.md | 56 +++++-- packages/node-sdk/README.md | 265 +++++++++++++++++++++++++++------ 2 files changed, 269 insertions(+), 52 deletions(-) diff --git a/packages/browser-sdk/README.md b/packages/browser-sdk/README.md index eec910a2..1324d8be 100644 --- a/packages/browser-sdk/README.md +++ b/packages/browser-sdk/README.md @@ -101,11 +101,13 @@ type Configuration = { feedback?: undefined; // See FEEDBACK.md enableTracking?: true; // set to `false` to stop sending track events and user/company updates to Bucket servers. Useful when you're impersonating a user. featureOptions?: { - fallbackFeatures?: string[]; // Enable these features if unable to contact bucket.co - timeoutMs?: number; // Timeout for fetching features - staleWhileRevalidate?: boolean; // Revalidate in the background when cached features turn stale to avoid latency in the UI + fallbackFeatures?: + | string[] + | Record; // Enable these features if unable to contact bucket.co. Can be a list of feature keys or a record with configuration values + timeoutMs?: number; // Timeout for fetching features (default: 5000ms) + staleWhileRevalidate?: boolean; // Revalidate in the background when cached features turn stale to avoid latency in the UI (default: false) staleTimeMs?: number; // at initialization time features are loaded from the cache unless they have gone stale. Defaults to 0 which means the cache is disabled. Increase in the case of a non-SPA. - expireTimeMs?: number; // In case we're unable to fetch features from Bucket, cached/stale features will be used instead until they expire after `expireTimeMs`. + expireTimeMs?: number; // In case we're unable to fetch features from Bucket, cached/stale features will be used instead until they expire after `expireTimeMs`. Default is 30 days. }; }; ``` @@ -172,6 +174,42 @@ 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 Overrides + +You can override feature flags locally for testing purposes using `setFeatureOverride`: + +```ts +// Override a feature to be enabled +bucketClient.setFeatureOverride("huddle", true); + +// Override a feature to be disabled +bucketClient.setFeatureOverride("huddle", false); + +// Remove the override +bucketClient.setFeatureOverride("huddle", null); + +// Get current override value +const override = bucketClient.getFeatureOverride("huddle"); // returns boolean | null +``` + +Feature overrides are persisted in `localStorage` and will be restored when the page is reloaded. + +### Feature Updates + +You can listen for feature updates using `onFeaturesUpdated`: + +```ts +// Register a callback for feature updates +const unsubscribe = bucketClient.onFeaturesUpdated(() => { + console.log("Features were updated"); +}); + +// Later, stop listening for updates +unsubscribe(); +``` + +Note that the callback may be called even if features haven't actually changed. + ### Remote config Similar to `isEnabled`, each feature has a `config` property. This configuration is managed from within Bucket. @@ -262,7 +300,7 @@ bucketClient.feedback({ }); ``` -#### Bucket feedback API +### Bucket feedback API If you are not using the Bucket Browser SDK, you can still submit feedback using the HTTP API. @@ -274,9 +312,9 @@ The Bucket Browser SDK doesn't collect any metadata and HTTP IP addresses are _n For tracking individual users, we recommend using something like database ID as userId, as it's unique and doesn't include any PII (personal identifiable information). If, however, you're using e.g. email address as userId, but prefer not to send any PII to Bucket, you can hash the sensitive data before sending it to Bucket: -``` +```ts import bucket from "@bucketco/browser-sdk"; -import { sha256 } from 'crypto-hash'; +import { sha256 } from "crypto-hash"; bucket.user(await sha256("john_doe")); ``` @@ -290,7 +328,7 @@ The two cookies are: - `bucket-prompt-${userId}`: store the last automated feedback prompt message ID received to avoid repeating surveys - `bucket-token-${userId}`: caching a token used to connect to Bucket's live messaging infrastructure that is used to deliver automated feedback surveys in real time. -### Typescript +### TypeScript Types are bundled together with the library and exposed automatically when importing through a package manager. @@ -310,7 +348,7 @@ If you are including the Bucket tracking SDK with a ` + + + + + + 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 a2b8baf6..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.3", + "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 186b9eeb..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,40 +580,29 @@ 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 }; + const value = f?.isEnabled ?? false; return { get isEnabled() { fClient .sendCheckEvent({ - action: "check", - key, + key: key, version: f?.targetingVersion, - ruleEvaluationResults: f?.ruleEvaluationResults, - missingContextFields: f?.missingContextFields, value, }) .catch(() => { @@ -710,25 +610,6 @@ export class BucketClient { }); return value; }, - get config() { - fClient - .sendCheckEvent({ - action: "check-config", - key, - version: f?.config?.version, - ruleEvaluationResults: f?.config?.ruleEvaluationResults, - missingContextFields: f?.config?.missingContextFields, - value: f?.config && { - key: f.config.key, - payload: f.config.payload, - }, - }) - .catch(() => { - // ignore - }); - - return config; - }, track: () => this.track(key), requestFeedback: ( options: Omit, @@ -741,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 70f26159..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,56 +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") || - (feature.missingContextFields && - !Array.isArray(feature.missingContextFields)) || - (feature.ruleEvaluationResults && - !Array.isArray(feature.ruleEvaluationResults)) + typeof feature.targetingVersion !== "number" ) { return; } - features[key] = { isEnabled: feature.isEnabled, targetingVersion: feature.targetingVersion, key, - config: feature.config, - missingContextFields: feature.missingContextFields, - ruleEvaluationResults: feature.ruleEvaluationResults, }; } - 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, @@ -78,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 103adc69..46d02a61 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -9,129 +9,73 @@ import { parseAPIFeaturesResponse, } from "./featureCache"; -/** - * A feature fetched from the server. - */ -export type FetchedFeature = { +export type RawFeature = { /** - * Feature key. + * Feature key */ key: string; /** - * Result of feature flag evaluation. + * Result of feature flag evaluation */ isEnabled: boolean; /** - * Version of targeting rules. + * Version of targeting rules */ targetingVersion?: number; - - /** - * Rule evaluation results. - */ - ruleEvaluationResults?: boolean[]; - - /** - * Missing context fields. - */ - missingContextFields?: string[]; - - /** - * 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; - - /** - * The rule evaluation results. - */ - ruleEvaluationResults?: boolean[]; - - /** - * The missing context fields. - */ - missingContextFields?: string[]; - }; }; 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; @@ -140,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; } @@ -173,34 +115,19 @@ export function flattenJSON(obj: Record): Record { */ export interface CheckEvent { /** - * Action to perform. - */ - action: "check" | "check-config"; - - /** - * Feature key. + * Feature key */ key: string; /** - * Result of feature flag or configuration evaluation. + * Result of feature flag evaluation */ - value: any; + value: boolean; /** - * Version of targeting rules. + * Version of targeting rules */ version?: number; - - /** - * Rule evaluation results. - */ - ruleEvaluationResults?: boolean[]; - - /** - * Missing context fields. - */ - missingContextFields?: string[]; } type context = { @@ -211,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; @@ -252,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) { @@ -348,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({ @@ -376,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"); @@ -396,17 +265,15 @@ export class FeaturesClient { * @param checkEvent - The feature to send the event for. */ async sendCheckEvent(checkEvent: CheckEvent) { - const rateLimitKey = `check-event:${this.fetchParams().toString()}:${checkEvent.key}:${checkEvent.version}:${checkEvent.value}`; + const rateLimitKey = `${this.fetchParams().toString()}:${checkEvent.key}:${checkEvent.version}:${checkEvent.value}`; await this.rateLimiter.rateLimited(rateLimitKey, async () => { const payload = { - action: checkEvent.action, + action: "check", key: checkEvent.key, targetingVersion: checkEvent.version, evalContext: this.context, evalResult: checkEvent.value, - evalRuleResults: checkEvent.ruleEvaluationResults, - evalMissingFields: checkEvent.missingContextFields, }; this.httpClient @@ -424,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); @@ -471,33 +308,7 @@ export class FeaturesClient { return params; } - private warnMissingFeatureContextFields(features: FetchedFeatures) { - const report: Record = {}; - for (const featureKey in features) { - const feature = features[featureKey]; - if (feature?.missingContextFields?.length) { - report[feature.key] = feature.missingContextFields; - } - - if (feature?.config?.missingContextFields?.length) { - report[`${feature.key}.config`] = feature.config.missingContextFields; - } - } - - if (Object.keys(report).length > 0) { - this.rateLimiter.rateLimited( - `feature-missing-context-fields:${this.fetchParams().toString()}`, - () => { - this.logger.warn( - `feature/remote config targeting rules might not be correctly evaluated due to missing context fields.`, - report, - ); - }, - ); - } - } - - private async maybeFetchFeatures(): Promise { + private async maybeFetchFeatures(): Promise { const cacheKey = this.fetchParams().toString(); const cachedItem = this.cache.get(cacheKey); @@ -514,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 @@ -532,7 +343,6 @@ export class FeaturesClient { features: fetchedFeatures, }); - this.warnMissingFeatureContextFields(fetchedFeatures); return fetchedFeatures; } @@ -542,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 ( - <> -