From 3d197f2ea74f00ef904fc6a6960160d0cf4ded9a Mon Sep 17 00:00:00 2001 From: Lukas Reining Date: Wed, 8 May 2024 22:41:03 +0200 Subject: [PATCH] fix: remove export of OpenFeatureClient (#794) ## This PR As discussed here https://github.com/open-feature/js-sdk/pull/750#discussion_r1450896230, we should not export `OpenFeatureClient` from the server and web SDK. The type used outside the SDK should be `Client`, which is also used in the public APIs like `OpenFeatureApi`. The question is, if we should mark this as breaking. Technically it will break code that imports `OpenFeatureClient` instead of `Client`, but as @toddbaert said code using it could also be seen as "used wrong" while being technically fine. I am still leaning towards marking it as breaking, to be sure we are not breaking something that is technically fine, just because it is unintended use. But I could also live with non-beaking. --------- Signed-off-by: Lukas Reining --- .../{ => internal}/open-feature-client.ts | 19 ++++++++++------ packages/client/src/open-feature.ts | 2 +- packages/client/test/client.spec.ts | 14 +++++++----- packages/client/test/open-feature.spec.ts | 2 +- .../{ => internal}/open-feature-client.ts | 22 +++++++++++-------- packages/server/src/open-feature.ts | 4 ++-- packages/server/test/client.spec.ts | 2 +- packages/server/test/open-feature.spec.ts | 2 +- 8 files changed, 39 insertions(+), 28 deletions(-) rename packages/client/src/client/{ => internal}/open-feature-client.ts (94%) rename packages/server/src/client/{ => internal}/open-feature-client.ts (94%) diff --git a/packages/client/src/client/open-feature-client.ts b/packages/client/src/client/internal/open-feature-client.ts similarity index 94% rename from packages/client/src/client/open-feature-client.ts rename to packages/client/src/client/internal/open-feature-client.ts index 9fb23383..7e649e39 100644 --- a/packages/client/src/client/open-feature-client.ts +++ b/packages/client/src/client/internal/open-feature-client.ts @@ -18,13 +18,13 @@ import { instantiateErrorByErrorCode, statusMatchesEvent, } from '@openfeature/core'; -import { FlagEvaluationOptions } from '../evaluation'; -import { ProviderEvents } from '../events'; -import { InternalEventEmitter } from '../events/internal/internal-event-emitter'; -import { Hook } from '../hooks'; -import { OpenFeature } from '../open-feature'; -import { Provider, ProviderStatus } from '../provider'; -import { Client } from './client'; +import { FlagEvaluationOptions } from '../../evaluation'; +import { ProviderEvents } from '../../events'; +import { InternalEventEmitter } from '../../events/internal/internal-event-emitter'; +import { Hook } from '../../hooks'; +import { OpenFeature } from '../../open-feature'; +import { Provider, ProviderStatus } from '../../provider'; +import { Client } from './../client'; type OpenFeatureClientOptions = { /** @@ -35,6 +35,11 @@ type OpenFeatureClientOptions = { version?: string; }; +/** + * This implementation of the {@link Client} is meant to only be instantiated by the SDK. + * It should not be used outside the SDK and so should not be exported. + * @internal + */ export class OpenFeatureClient implements Client { private _hooks: Hook[] = []; private _clientLogger?: Logger; diff --git a/packages/client/src/open-feature.ts b/packages/client/src/open-feature.ts index cae2d2fa..d6fc0793 100644 --- a/packages/client/src/open-feature.ts +++ b/packages/client/src/open-feature.ts @@ -9,10 +9,10 @@ import { stringOrUndefined, } from '@openfeature/core'; import { Client } from './client'; +import { OpenFeatureClient } from './client/internal/open-feature-client'; import { OpenFeatureEventEmitter, ProviderEvents } from './events'; import { Hook } from './hooks'; import { NOOP_PROVIDER, Provider, ProviderStatus } from './provider'; -import { OpenFeatureClient } from './client/open-feature-client'; // use a symbol as a key for the global singleton const GLOBAL_OPENFEATURE_API_KEY = Symbol.for('@openfeature/web-sdk/api'); diff --git a/packages/client/test/client.spec.ts b/packages/client/test/client.spec.ts index ee06bae1..bb776ba3 100644 --- a/packages/client/test/client.spec.ts +++ b/packages/client/test/client.spec.ts @@ -14,7 +14,7 @@ import { ResolutionDetails, StandardResolutionReasons, } from '../src'; -import { OpenFeatureClient } from '../src/client/open-feature-client'; +import { OpenFeatureClient } from '../src/client/internal/open-feature-client'; const BOOLEAN_VALUE = true; const STRING_VALUE = 'val'; @@ -130,12 +130,15 @@ describe('OpenFeatureClient', () => { resolveBooleanEvaluation(): ResolutionDetails { throw new Error('Method not implemented.'); } + resolveStringEvaluation(): ResolutionDetails { throw new Error('Method not implemented.'); } + resolveNumberEvaluation(): ResolutionDetails { throw new Error('Method not implemented.'); } + resolveObjectEvaluation(): ResolutionDetails { throw new Error('Method not implemented.'); } @@ -236,7 +239,7 @@ describe('OpenFeatureClient', () => { numberFlag, defaultNumberValue, {}, - {} + {}, ); }); }); @@ -248,7 +251,7 @@ describe('OpenFeatureClient', () => { const defaultNumberValue = 4096; const value: MyRestrictedNumber = client.getNumberValue( numberFlag, - defaultNumberValue + defaultNumberValue, ); expect(value).toEqual(NUMBER_VALUE); @@ -256,7 +259,7 @@ describe('OpenFeatureClient', () => { numberFlag, defaultNumberValue, {}, - {} + {}, ); }); }); @@ -333,7 +336,7 @@ describe('OpenFeatureClient', () => { booleanFlag, defaultBooleanValue, {}, - {} + {}, ); }); }); @@ -634,4 +637,3 @@ describe('OpenFeatureClient', () => { }); }); }); - diff --git a/packages/client/test/open-feature.spec.ts b/packages/client/test/open-feature.spec.ts index 51ac2088..75364b5d 100644 --- a/packages/client/test/open-feature.spec.ts +++ b/packages/client/test/open-feature.spec.ts @@ -1,6 +1,6 @@ import { Paradigm } from '@openfeature/core'; import { OpenFeature, OpenFeatureAPI, Provider, ProviderStatus } from '../src'; -import { OpenFeatureClient } from '../src/client/open-feature-client'; +import { OpenFeatureClient } from '../src/client/internal/open-feature-client'; const mockProvider = (config?: { initialStatus?: ProviderStatus; runsOn?: Paradigm }) => { return { diff --git a/packages/server/src/client/open-feature-client.ts b/packages/server/src/client/internal/open-feature-client.ts similarity index 94% rename from packages/server/src/client/open-feature-client.ts rename to packages/server/src/client/internal/open-feature-client.ts index fb37b73f..0e3cd1bb 100644 --- a/packages/server/src/client/open-feature-client.ts +++ b/packages/server/src/client/internal/open-feature-client.ts @@ -9,7 +9,6 @@ import { HookContext, JsonValue, Logger, - ManageContext, OpenFeatureError, ProviderFatalError, ProviderNotReadyError, @@ -19,13 +18,13 @@ import { instantiateErrorByErrorCode, statusMatchesEvent, } from '@openfeature/core'; -import { FlagEvaluationOptions } from '../evaluation'; -import { ProviderEvents } from '../events'; -import { InternalEventEmitter } from '../events/internal/internal-event-emitter'; -import { Hook } from '../hooks'; -import { OpenFeature } from '../open-feature'; -import { Provider, ProviderStatus } from '../provider'; -import { Client } from './client'; +import { FlagEvaluationOptions } from '../../evaluation'; +import { ProviderEvents } from '../../events'; +import { InternalEventEmitter } from '../../events/internal/internal-event-emitter'; +import { Hook } from '../../hooks'; +import { OpenFeature } from '../../open-feature'; +import { Provider, ProviderStatus } from '../../provider'; +import { Client } from './../client'; type OpenFeatureClientOptions = { /** @@ -36,7 +35,12 @@ type OpenFeatureClientOptions = { version?: string; }; -export class OpenFeatureClient implements Client, ManageContext { +/** + * This implementation of the {@link Client} is meant to only be instantiated by the SDK. + * It should not be used outside the SDK and so should not be exported. + * @internal + */ +export class OpenFeatureClient implements Client { private _context: EvaluationContext; private _hooks: Hook[] = []; private _clientLogger?: Logger; diff --git a/packages/server/src/open-feature.ts b/packages/server/src/open-feature.ts index f0313fd8..03d4b330 100644 --- a/packages/server/src/open-feature.ts +++ b/packages/server/src/open-feature.ts @@ -3,10 +3,12 @@ import { ManageContext, OpenFeatureCommonAPI, ProviderWrapper, + ServerProviderStatus, objectOrUndefined, stringOrUndefined, } from '@openfeature/core'; import { Client } from './client'; +import { OpenFeatureClient } from './client/internal/open-feature-client'; import { OpenFeatureEventEmitter } from './events'; import { Hook } from './hooks'; import { NOOP_PROVIDER, Provider, ProviderStatus } from './provider'; @@ -16,8 +18,6 @@ import { TransactionContext, TransactionContextPropagator, } from './transaction-context'; -import { ServerProviderStatus } from '@openfeature/core'; -import { OpenFeatureClient } from './client/open-feature-client'; // use a symbol as a key for the global singleton const GLOBAL_OPENFEATURE_API_KEY = Symbol.for('@openfeature/js-sdk/api'); diff --git a/packages/server/test/client.spec.ts b/packages/server/test/client.spec.ts index fb163511..368c3bb5 100644 --- a/packages/server/test/client.spec.ts +++ b/packages/server/test/client.spec.ts @@ -17,7 +17,7 @@ import { TransactionContext, TransactionContextPropagator, } from '../src'; -import { OpenFeatureClient } from '../src/client/open-feature-client'; +import { OpenFeatureClient } from '../src/client/internal/open-feature-client'; const BOOLEAN_VALUE = true; const STRING_VALUE = 'val'; diff --git a/packages/server/test/open-feature.spec.ts b/packages/server/test/open-feature.spec.ts index 7c5943a8..18b6bfd4 100644 --- a/packages/server/test/open-feature.spec.ts +++ b/packages/server/test/open-feature.spec.ts @@ -1,6 +1,6 @@ import { Paradigm } from '@openfeature/core'; import { OpenFeature, OpenFeatureAPI, Provider, ProviderStatus } from '../src'; -import { OpenFeatureClient } from '../src/client/open-feature-client'; +import { OpenFeatureClient } from '../src/client/internal/open-feature-client'; const mockProvider = (config?: { initialStatus?: ProviderStatus; runsOn?: Paradigm }) => { return {