feat(core): allow returning undefined in feature service's create met…

…hod (#587)

closes #582

docs/guides/custom-logging.md

@@ -71,10 +71,9 @@ defineFeatureAppLoader(featureAppManager, {logger});
 To provide a custom logger to Feature Apps and Feature Services, the integrator
 can provide the Logger Feature Service from the
 [`@feature-hub/logger`][logger-api] package. It must be defined with a function
-that takes the `consumerUid` of the consuming Feature App or Feature Service,
-and returns a [`Logger`][core-logger-interface] instance. This makes it possible
-to augment the consumer logs with the `consumerUid`, e.g. using pino's child
-logger:
+that takes the `consumerId` of the consuming Feature App or Feature Service, and
+returns a [`Logger`][core-logger-interface] instance. This makes it possible to
+augment the consumer logs with the `consumerId`, e.g. using pino's child logger:
 
 ```js
 import {defineLogger} from '@feature-hub/logger';
@@ -84,7 +83,7 @@ import {defineLogger} from '@feature-hub/logger';
 const {featureAppManager} = createFeatureHub('acme:integrator', {
   logger,
   featureServiceDefinitions: [
-    defineLogger(consumerUid => logger.child({consumerUid}))
+    defineLogger(consumerId => logger.child({consumerId}))
   ]
 });
 ```

docs/guides/writing-a-feature-service.md

@@ -114,17 +114,20 @@ following properties:
    };
    ```
 
-## Feature Service Binder
-
 The `create` method must return an object that provides an implementation of the
 `FeatureServiceBinder` type of the [`@feature-hub/core`][core-api] package for
 each supported major version. The Feature Service binder is a function that is
-called for each consumer with its `consumerUid` string as the first argument. It
+called for each consumer with its `consumerId` string as the first argument. It
 returns a Feature Service binding with a consumer-bound `featureService` and an
 optional `unbind` method. The `FeatureServiceRegistry` passes this
 consumer-bound `featureService` to the consumer's `create` method via the
 `env.featureServices` argument.
 
+If within its `create` method a Feature Service concludes that it can not be
+created, e.g. because of a certain configuration or missing requirement, it
+might also return `undefined`. Such a Feature Service should document for its
+consumers that they must declare the dependency in `optionalDependencies`.
+
 ## Providing a Versioned API
 
 A Feature Service provider can support multiple major versions at the same time
@@ -138,7 +141,7 @@ const myFeatureServiceDefinition = {
   create(env) {
     let count = 0;
 
-    const v1 = consumerUid => ({
+    const v1 = consumerId => ({
       featureService: {
         plus() {
           count += 1;
@@ -166,7 +169,7 @@ const myFeatureServiceDefinition = {
   create(env) {
     let count = 0;
 
-    const v1 = consumerUid => ({
+    const v1 = consumerId => ({
       featureService: {
         plus() {
           count += 1;
@@ -213,7 +216,7 @@ const myFeatureServiceDefinition = {
     const decrement = () => void --count;
     const increment = () => void ++count;
 
-    const v1 = consumerUid => ({
+    const v1 = consumerId => ({
       featureService: {
         getCount,
 
@@ -229,7 +232,7 @@ const myFeatureServiceDefinition = {
       }
     });
 
-    const v2 = consumerUid => ({
+    const v2 = consumerId => ({
       featureService: {getCount, increment, decrement}
     });
 
@@ -266,21 +269,21 @@ const myFeatureServiceDefinition = {
     // Shared state lives here.
     let consumerCounts = {};
 
-    const v1 = consumerUid => {
+    const v1 = consumerId => {
       // Consumer-specific state lives here.
-      consumerCounts[consumerUid] = 0;
+      consumerCounts[consumerId] = 0;
 
       const featureService = {
         increment() {
-          consumerCounts[consumerUid] += 1;
+          consumerCounts[consumerId] += 1;
         },
 
         decrement() {
-          consumerCounts[consumerUid] -= 1;
+          consumerCounts[consumerId] -= 1;
         },
 
         getCount() {
-          return consumerCounts[consumerUid];
+          return consumerCounts[consumerId];
         },
 
         getTotalCount() {
@@ -293,7 +296,7 @@ const myFeatureServiceDefinition = {
 
       const unbind = () => {
         // Cleaning up the consumer-specific state.
-        delete consumerCounts[consumerUid];
+        delete consumerCounts[consumerId];
       };
 
       return {featureService, unbind};

packages/async-ssr-manager/src/__tests__/index.test.ts

@@ -1,4 +1,5 @@
 // tslint:disable:no-implicit-dependencies
+// tslint:disable:no-non-null-assertion
 
 import {
   FeatureServiceBinder,
@@ -49,15 +50,15 @@ describe('defineAsyncSsrManager', () => {
     it('creates a shared Feature Service containing version 1.0.0', () => {
       const sharedAsyncSsrManager = asyncSsrManagerDefinition.create(mockEnv);
 
-      expect(sharedAsyncSsrManager['1.0.0']).toBeDefined();
+      expect(sharedAsyncSsrManager!['1.0.0']).toBeDefined();
     });
   });
 
   describe('AsyncSsrManagerV1', () => {
     let asyncSsrManagerBinder: FeatureServiceBinder<AsyncSsrManagerV1>;
 
     beforeEach(() => {
-      asyncSsrManagerBinder = asyncSsrManagerDefinition.create(mockEnv)[
+      asyncSsrManagerBinder = asyncSsrManagerDefinition.create(mockEnv)![
         '1.0.0'
       ];
     });
@@ -284,7 +285,7 @@ describe('defineAsyncSsrManager', () => {
         beforeEach(() => {
           asyncSsrManagerDefinition = defineAsyncSsrManager();
 
-          asyncSsrManagerBinder = asyncSsrManagerDefinition.create(mockEnv)[
+          asyncSsrManagerBinder = asyncSsrManagerDefinition.create(mockEnv)![
             '1.0.0'
           ];
         });
@@ -316,7 +317,7 @@ describe('defineAsyncSsrManager', () => {
 
         asyncSsrManagerBinder = asyncSsrManagerDefinition.create({
           featureServices: {}
-        })['1.0.0'];
+        })!['1.0.0'];
       });
 
       afterEach(() => {

packages/core/src/__tests__/feature-service-registry.test.ts

@@ -126,10 +126,12 @@ describe('FeatureServiceRegistry', () => {
         [providerDefinitionA],
         'test'
       );
+
       featureServiceRegistry.registerFeatureServices(
         [providerDefinitionB],
         'test'
       );
+
       featureServiceRegistry.registerFeatureServices(
         [providerDefinitionC],
         'test'
@@ -152,6 +154,7 @@ describe('FeatureServiceRegistry', () => {
         [providerDefinitionA, providerDefinitionB],
         'test'
       );
+
       featureServiceRegistry.registerFeatureServices(
         [providerDefinitionB],
         'test'
@@ -183,6 +186,25 @@ describe('FeatureServiceRegistry', () => {
       ]);
     });
 
+    it('does not register a Feature Service that returns undefined from create', () => {
+      providerDefinitionA.create.mockReturnValue(undefined);
+
+      featureServiceRegistry.registerFeatureServices(
+        [providerDefinitionA],
+        'test'
+      );
+
+      expect(providerDefinitionA.create.mock.calls).toEqual([
+        [{featureServices: {}}]
+      ]);
+
+      expect(logger.info.mock.calls).toEqual([
+        [
+          'The Feature Service "a" could not be registered by registrant "test" because it returned undefined.'
+        ]
+      ]);
+    });
+
     it('fails to register the Feature Service "c" due to the lack of dependency "a"', () => {
       expect(() =>
         featureServiceRegistry.registerFeatureServices(
@@ -196,7 +218,7 @@ describe('FeatureServiceRegistry', () => {
       );
     });
 
-    it('doesnt fail to register the Feature Service "b" due to the lack of optional dependency "a"', () => {
+    it('does not fail to register the Feature Service "b" due to the lack of optional dependency "a"', () => {
       providerDefinitionB = {
         id: 'b',
         optionalDependencies: {featureServices: {a: '^1.0.0'}},
@@ -547,6 +569,30 @@ describe('FeatureServiceRegistry', () => {
       });
     });
 
+    describe('for a Feature Service consumer and an optional dependency to a Feature Service that returned undefined in create', () => {
+      it('creates a bindings object without Feature Services', () => {
+        featureServiceRegistry = new FeatureServiceRegistry({logger});
+
+        providerDefinitionA.create.mockReturnValue(undefined);
+
+        featureServiceRegistry.registerFeatureServices(
+          [providerDefinitionA],
+          'test'
+        );
+
+        expect(binderA.mock.calls).toEqual([]);
+
+        expect(
+          featureServiceRegistry.bindFeatureServices(
+            {optionalDependencies: {featureServices: {a: '1.1.0'}}},
+            'foo'
+          )
+        ).toEqual({featureServices: {}, unbind: expect.any(Function)});
+
+        expect(binderA.mock.calls).toEqual([]);
+      });
+    });
+
     describe('for a Feature Service consumer and two optional dependencies', () => {
       describe('with the first dependency missing', () => {
         it('creates a bindings object with Feature Services', () => {

packages/core/src/feature-service-registry.ts

@@ -56,7 +56,7 @@ export interface FeatureServiceProviderDefinition<
 
   create(
     env: FeatureServiceEnvironment<TFeatureServices>
-  ): TSharedFeatureService;
+  ): TSharedFeatureService | undefined;
 }
 
 export interface FeatureServiceBinding<TFeatureService> {
@@ -329,17 +329,23 @@ export class FeatureServiceRegistry {
 
     const sharedFeatureService = providerDefinition.create({featureServices});
 
-    this.validateFeatureServiceVersions(
-      sharedFeatureService,
-      providerId,
-      registrantId
-    );
+    if (sharedFeatureService) {
+      this.validateFeatureServiceVersions(
+        sharedFeatureService,
+        providerId,
+        registrantId
+      );
 
-    this.sharedFeatureServices.set(providerId, sharedFeatureService);
+      this.sharedFeatureServices.set(providerId, sharedFeatureService);
 
-    this.logger.info(
-      Messages.featureServiceSuccessfullyRegistered(providerId, registrantId)
-    );
+      this.logger.info(
+        Messages.featureServiceSuccessfullyRegistered(providerId, registrantId)
+      );
+    } else {
+      this.logger.info(
+        Messages.featureServiceReturnedUndefined(providerId, registrantId)
+      );
+    }
   }
 
   private bindFeatureService(

packages/core/src/internal/feature-service-registry-messages.ts

@@ -56,6 +56,17 @@ export function featureServiceNotRegistered(
   )}.`;
 }
 
+export function featureServiceReturnedUndefined(
+  providerId: string,
+  registrantId: string
+): string {
+  return `The Feature Service ${JSON.stringify(
+    providerId
+  )} could not be registered by registrant ${JSON.stringify(
+    registrantId
+  )} because it returned undefined.`;
+}
+
 export function featureServiceSuccessfullyRegistered(
   providerId: string,
   registrantId: string

packages/history-service/src/__tests__/index.node.test.ts

@@ -46,7 +46,7 @@ describe('defineHistoryService', () => {
           {mode: 'static'}
         ).create(mockEnv);
 
-        return sharedHistoryService['1.0.0'];
+        return sharedHistoryService!['1.0.0'];
       };
     });
 
@@ -445,7 +445,7 @@ describe('defineHistoryService', () => {
           {mode: 'static'}
         ).create(mockEnv);
 
-        return sharedHistoryService['2.0.0'];
+        return sharedHistoryService!['2.0.0'];
       };
 
       createHistories({url: '/example', cookies: {}, headers: {}});

packages/history-service/src/__tests__/index.test.ts

@@ -76,8 +76,8 @@ describe('defineHistoryService', () => {
         featureServices: {}
       });
 
-      expect(sharedHistoryService['1.0.0']).toBeDefined();
-      expect(sharedHistoryService['2.0.0']).toBeDefined();
+      expect(sharedHistoryService!['1.0.0']).toBeDefined();
+      expect(sharedHistoryService!['2.0.0']).toBeDefined();
     });
   });
 
@@ -98,7 +98,7 @@ describe('defineHistoryService', () => {
           createRootLocationTransformer({consumerPathsQueryParamName})
         ).create(mockEnv);
 
-        return sharedHistoryService['1.0.0'];
+        return sharedHistoryService!['1.0.0'];
       };
     });
 
@@ -648,7 +648,7 @@ describe('defineHistoryService', () => {
           createRootLocationTransformer({consumerPathsQueryParamName})
         ).create(mockEnv);
 
-        return sharedHistoryService['2.0.0'];
+        return sharedHistoryService!['2.0.0'];
       };
     });
 
@@ -1494,7 +1494,7 @@ describe('defineHistoryService', () => {
               createNewRootLocationForMultipleConsumers: createNewRootLocationForMultipleConsumersMock
             }).create(mockEnv);
 
-            return sharedHistoryService['2.0.0'];
+            return sharedHistoryService!['2.0.0'];
           };
 
           const historyServiceBinder = createHistoryServiceBinder();

packages/logger/src/__tests__/index.test.ts

@@ -1,4 +1,5 @@
 // tslint:disable:no-implicit-dependencies
+// tslint:disable:no-non-null-assertion
 
 import {
   FeatureServiceEnvironment,
@@ -25,7 +26,7 @@ describe('defineLogger', () => {
     it('creates a shared Feature Service containing version 1.0.0', () => {
       const sharedLogger = loggerDefinition.create(mockEnv);
 
-      expect(sharedLogger['1.0.0']).toBeDefined();
+      expect(sharedLogger!['1.0.0']).toBeDefined();
     });
   });
 
@@ -34,7 +35,7 @@ describe('defineLogger', () => {
 
     describe('with the default createConsumerLogger function', () => {
       beforeEach(() => {
-        logger = loggerDefinition.create(mockEnv)['1.0.0']('test:id')
+        logger = loggerDefinition.create(mockEnv)!['1.0.0']('test:id')
           .featureService;
       });
 
@@ -96,7 +97,7 @@ describe('defineLogger', () => {
 
         loggerDefinition = defineLogger(mockCreateConsumerLogger);
 
-        logger = loggerDefinition.create(mockEnv)['1.0.0']('test:id')
+        logger = loggerDefinition.create(mockEnv)!['1.0.0']('test:id')
           .featureService;
       });