Repository: open-feature/js-sdk
Pull request #794 (2024-01-27 23:48:25)

fix: remove export of OpenFeatureClient
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## 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.

toddbaert: I think we can (debatabley) consider this non-breaking, since it's an export we didn't expect people to use... it really depends how strict we want to be.

In any case, I think it might be a good idea to discuss rolling together some upcoming breaking changes. I'd like to also include:

- making it impossible to return an error resolution from providers (force them to throw if something goes wrong)
- removing `setProviderAndWait` and just making `setProvider` async

What do you guys think?

cc @lukas-reining @beeme1mr @luizgribeiro @Kavindu-Dodan 
Kavindu-Dodan: > I think we can (debatabley) consider this non-breaking, since it's an export we didn't expect people to use... it really depends how strict we want to be.
> 
> In any case, I think it might be a good idea to discuss rolling together some upcoming breaking changes. I'd like to also include:
> 
> * making it impossible to return an error resolution from providers (force them to throw if something goes wrong)
> * removing `setProviderAndWait` and just making `setProvider` async
> 
> What do you guys think?
> 
> cc @lukas-reining @beeme1mr @luizgribeiro @Kavindu-Dodan

+1 for combining multiple breaking changes for the next release 
toddbaert: > I'd say it's a breaking change because if it's shipped as is since it would break minor upgrades in projects that are already using `OpenFeatureClient`. I also think bundling more breaking changes in one new major is a good alternative.
> 
> Maybe another way that might not be a breaking change is to expose `OpenFeatureClient` as an alias type to `Client`. I'm not a big fan of using `Client` as a type. Many different packages have a `Client` exported type and it would end up with an alias anyway.

I think it's OK to require people to alias things, but I would be up for changing the name to something more specific in a 2.0. I definitely don't want to alias the `OpenFeatureClient` though since it's a class. It's a good idea to export and reference interfaces instead of classes where-ever possible, or you can run into nasty things [like this](https://cloud-native.slack.com/archives/C06E4DE6S07/p1706650042057369?thread_ts=1706562067.358389&cid=C06E4DE6S07) which can happen if people end up with multiple copies of a dep (we do our best to avoid that but it can happen). 
toddbaert: I've opened [this](https://github.com/open-feature/js-sdk/issues/799) to record (small) proposed breaking changes in 2.0
toddbaert: @lukas-reining @luizgribeiro @beeme1mr I've rebased and resolved conflcits on this PR.

I also made the title non-breaking as we decided (though it's technically a break but not on an export we indented people to use).

Patch:
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/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<OpenFeatureClient> {
+/**
+ * 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');


Test patch:
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<boolean> {
         throw new Error('Method not implemented.');
       }
+
       resolveStringEvaluation(): ResolutionDetails<string> {
         throw new Error('Method not implemented.');
       }
+
       resolveNumberEvaluation(): ResolutionDetails<number> {
         throw new Error('Method not implemented.');
       }
+
       resolveObjectEvaluation<T extends JsonValue>(): ResolutionDetails<T> {
         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<MyRestrictedNumber>(
                 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/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 {