feat(server-renderer): replace register & rerender with rerenderAfter (…

…#241)
feature-hub · Jan 9, 2019 · 5fbfcc6 · 5fbfcc6
1 parent b33f744
commit 5fbfcc6
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 218 deletions.
diff --git a/packages/server-renderer/package.json b/packages/server-renderer/package.json
@@ -22,6 +22,9 @@
   "dependencies": {
     "@feature-hub/core": "^0.11.0"
   },
+  "devDependencies": {
+    "jest-mock-console": "^0.4.2"
+  },
   "publishConfig": {
     "access": "public"
   }

diff --git a/packages/server-renderer/src/__tests__/index.test.ts b/packages/server-renderer/src/__tests__/index.test.ts
@@ -1,20 +1,26 @@
+// tslint:disable:no-implicit-dependencies
+
 import {
   FeatureAppEnvironment,
   FeatureServiceBinder,
   FeatureServiceProviderDefinition
 } from '@feature-hub/core';
+import mockConsole from 'jest-mock-console';
 import {ServerRendererV1, ServerRequest, defineServerRenderer} from '..';
+import {ServerRendererConfig} from '../config';
 import {useFakeTimers} from './use-fake-timers';
 
 describe('defineServerRenderer', () => {
-  let mockEnv: FeatureAppEnvironment<undefined, {}>;
+  let mockEnv: FeatureAppEnvironment<ServerRendererConfig, {}>;
   let serverRendererDefinition: FeatureServiceProviderDefinition;
   let serverRequest: ServerRequest;
 
   beforeEach(() => {
-    jest.useFakeTimers();
-
-    mockEnv = {config: undefined, featureServices: {}, idSpecifier: undefined};
+    mockEnv = {
+      config: {timeout: 5},
+      featureServices: {},
+      idSpecifier: undefined
+    };
 
     serverRequest = {
       path: '/app',
@@ -25,10 +31,6 @@ describe('defineServerRenderer', () => {
     serverRendererDefinition = defineServerRenderer(serverRequest);
   });
 
-  afterEach(() => {
-    jest.useRealTimers();
-  });
-
   it('creates a server renderer definition', () => {
     expect(serverRendererDefinition.id).toBe('s2:server-renderer');
     expect(serverRendererDefinition.dependencies).toBeUndefined();
@@ -41,7 +43,7 @@ describe('defineServerRenderer', () => {
       expect(sharedServerRenderer['1.0']).toBeDefined();
     });
 
-    for (const invalidConfig of [null, {rerenderWait: false}]) {
+    for (const invalidConfig of [null, {timeout: false}]) {
       describe(`with an invalid config ${JSON.stringify(
         invalidConfig
       )}`, () => {
@@ -73,25 +75,15 @@ describe('defineServerRenderer', () => {
     });
 
     describe('rendering', () => {
-      const createServerRendererConsumer = (
-        consumerUid: string,
-        rerenderWait = 0
-      ) => {
+      const createServerRendererConsumer = (consumerUid: string) => {
         const serverRenderer = serverRendererBinder(consumerUid).featureService;
 
         let firstRender = true;
-        let completed = false;
 
         const render = () => {
           if (firstRender) {
             firstRender = false;
-            serverRenderer.register(() => completed);
-
-            setTimeout(async () => {
-              completed = true;
-
-              await serverRenderer.rerender();
-            }, rerenderWait);
+            serverRenderer.rerenderAfter(Promise.resolve());
           }
         };
 
@@ -109,31 +101,7 @@ describe('defineServerRenderer', () => {
         });
       });
 
-      describe('with an integrator, and a consumer that is completed in the first render pass', () => {
-        it('resolves with an html string after the first render pass', async () => {
-          const serverRendererIntegrator = serverRendererBinder(
-            'test:integrator'
-          ).featureService;
-
-          const serverRendererConsumer = serverRendererBinder('test:consumer')
-            .featureService;
-
-          const mockRender = jest.fn(() => {
-            serverRendererConsumer.register(() => true);
-
-            return 'testHtml';
-          });
-
-          const html = await useFakeTimers(async () =>
-            serverRendererIntegrator.renderUntilCompleted(mockRender)
-          );
-
-          expect(html).toEqual('testHtml');
-          expect(mockRender).toHaveBeenCalledTimes(1);
-        });
-      });
-
-      describe('with an integrator, and a consumer that is completed after triggering a rerender', () => {
+      describe('with an integrator, and a consumer that triggers a rerender', () => {
         it('resolves with an html string after the second render pass', async () => {
           const serverRendererIntegrator = serverRendererBinder(
             'test:integrator'
@@ -149,29 +117,27 @@ describe('defineServerRenderer', () => {
             return 'testHtml';
           });
 
-          const html = await useFakeTimers(async () =>
-            serverRendererIntegrator.renderUntilCompleted(mockRender)
+          const html = await serverRendererIntegrator.renderUntilCompleted(
+            mockRender
           );
 
           expect(html).toEqual('testHtml');
           expect(mockRender).toHaveBeenCalledTimes(2);
         });
       });
 
-      describe('with an integrator, and two consumers that are completed after both triggered a rerender within "rerenderWait" milliseconds', () => {
+      describe('with an integrator, and two consumers that both trigger a rerender in the first render pass', () => {
         it('resolves with an html string after the second render pass', async () => {
           const serverRendererIntegrator = serverRendererBinder(
             'test:integrator'
           ).featureService;
 
           const serverRendererConsumer1 = createServerRendererConsumer(
-            'test:consumer:1',
-            50
+            'test:consumer:1'
           );
 
           const serverRendererConsumer2 = createServerRendererConsumer(
-            'test:consumer:2',
-            100
+            'test:consumer:2'
           );
 
           const mockRender = jest.fn(() => {
@@ -181,94 +147,15 @@ describe('defineServerRenderer', () => {
             return 'testHtml';
           });
 
-          const html = await useFakeTimers(
-            async () =>
-              serverRendererIntegrator.renderUntilCompleted(mockRender),
-            150
+          const html = await serverRendererIntegrator.renderUntilCompleted(
+            mockRender
           );
 
           expect(html).toEqual('testHtml');
           expect(mockRender).toHaveBeenCalledTimes(2);
         });
       });
 
-      describe('with an integrator, and two consumers that are completed after triggering a rerender more than "rerenderWait" milliseconds apart from one another', () => {
-        describe('and the default "rerenderWait"', () => {
-          it('resolves with an html string after the third render pass', async () => {
-            const serverRendererIntegrator = serverRendererBinder(
-              'test:integrator'
-            ).featureService;
-
-            const serverRendererConsumer1 = createServerRendererConsumer(
-              'test:consumer:1',
-              50
-            );
-
-            const serverRendererConsumer2 = createServerRendererConsumer(
-              'test:consumer:2',
-              101
-            );
-
-            const mockRender = jest.fn(() => {
-              serverRendererConsumer1.render();
-              serverRendererConsumer2.render();
-
-              return 'testHtml';
-            });
-
-            const html = await useFakeTimers(
-              async () =>
-                serverRendererIntegrator.renderUntilCompleted(mockRender),
-              151
-            );
-
-            expect(html).toEqual('testHtml');
-            expect(mockRender).toHaveBeenCalledTimes(3);
-          });
-        });
-
-        describe('and a custom, higher rerenderWait', () => {
-          beforeEach(() => {
-            serverRendererBinder = serverRendererDefinition.create({
-              config: {rerenderWait: 51},
-              featureServices: {}
-            })['1.0'] as FeatureServiceBinder<ServerRendererV1>;
-          });
-
-          it('resolves with an html string after the second render pass', async () => {
-            const serverRendererIntegrator = serverRendererBinder(
-              'test:integrator'
-            ).featureService;
-
-            const serverRendererConsumer1 = createServerRendererConsumer(
-              'test:consumer:1',
-              50
-            );
-
-            const serverRendererConsumer2 = createServerRendererConsumer(
-              'test:consumer:2',
-              101
-            );
-
-            const mockRender = jest.fn(() => {
-              serverRendererConsumer1.render();
-              serverRendererConsumer2.render();
-
-              return 'testHtml';
-            });
-
-            const html = await useFakeTimers(
-              async () =>
-                serverRendererIntegrator.renderUntilCompleted(mockRender),
-              152
-            );
-
-            expect(html).toEqual('testHtml');
-            expect(mockRender).toHaveBeenCalledTimes(2);
-          });
-        });
-      });
-
       describe('when the given render function throws an error', () => {
         it('rejects with the error', async () => {
           const serverRenderer = serverRendererBinder('test').featureService;
@@ -284,16 +171,46 @@ describe('defineServerRenderer', () => {
         });
       });
 
-      describe('when renderUntilCompleted is called multiple times', () => {
-        it('rejects with an error', async () => {
+      describe('when rendering takes longer than the configured timeout', () => {
+        it('rejects with an error after the configured timeout', async () => {
           const serverRenderer = serverRendererBinder('test').featureService;
-          const mockRender = jest.fn(() => 'testHtml');
+          const mockRender = jest.fn(() => {
+            serverRenderer.rerenderAfter(new Promise<void>(() => undefined));
 
-          await serverRenderer.renderUntilCompleted(mockRender);
+            return 'testHtml';
+          });
 
           return expect(
+            useFakeTimers(
+              async () => serverRenderer.renderUntilCompleted(mockRender),
+              5
+            )
+          ).rejects.toEqual(new Error('Got rendering timeout after 5 ms.'));
+        });
+      });
+
+      describe('when no timeout is configured', () => {
+        beforeEach(() => {
+          serverRendererBinder = serverRendererDefinition.create({
+            config: undefined,
+            featureServices: {}
+          })['1.0'] as FeatureServiceBinder<ServerRendererV1>;
+        });
+
+        it('logs a warning', async () => {
+          const serverRenderer = serverRendererBinder('test').featureService;
+          const mockRender = jest.fn(() => 'testHtml');
+          const restoreConsole = mockConsole();
+
+          await useFakeTimers(async () =>
             serverRenderer.renderUntilCompleted(mockRender)
-          ).rejects.toEqual(new Error('Rendering has already been started.'));
+          );
+
+          expect(console.warn).toHaveBeenCalledWith(
+            'No timeout is configured for the server renderer. This could lead to unexpectedly long render times or, in the worst case, never resolving render calls!'
+          );
+
+          restoreConsole();
         });
       });
     });

diff --git a/packages/server-renderer/src/__tests__/use-fake-timers.ts b/packages/server-renderer/src/__tests__/use-fake-timers.ts
@@ -43,7 +43,7 @@ export async function useFakeTimers<TResult>(
       actualTimeoutInMilliseconds += 1;
     }
 
-    if (expectedTimeoutInMilliseconds !== undefined) {
+    if (typeof expectedTimeoutInMilliseconds === 'number') {
       expect(actualTimeoutInMilliseconds).toBe(expectedTimeoutInMilliseconds);
     }
 

diff --git a/packages/server-renderer/src/config.ts b/packages/server-renderer/src/config.ts
@@ -1,5 +1,5 @@
 export interface ServerRendererConfig {
-  rerenderWait: number;
+  timeout?: number;
 }
 
 function isValidConfig(
@@ -13,11 +13,9 @@ function isValidConfig(
     return false;
   }
 
-  const {rerenderWait} = config as ServerRendererConfig;
+  const {timeout} = config as ServerRendererConfig;
 
-  return (
-    typeof rerenderWait === 'undefined' || typeof rerenderWait === 'number'
-  );
+  return typeof timeout === 'undefined' || typeof timeout === 'number';
 }
 
 export function validateConfig(

diff --git a/packages/server-renderer/src/define-server-renderer.ts b/packages/server-renderer/src/define-server-renderer.ts
@@ -3,7 +3,7 @@ import {
   FeatureServiceProviderDefinition,
   SharedFeatureService
 } from '@feature-hub/core';
-import {validateConfig} from './config';
+import {ServerRendererConfig, validateConfig} from './config';
 import {
   ServerRenderer,
   ServerRendererV1,
@@ -21,8 +21,10 @@ export function defineServerRenderer(
     id: 's2:server-renderer',
 
     create: (env): SharedServerRenderer => {
-      const {rerenderWait = 50} = validateConfig(env.config) || {};
-      const serverRenderer = new ServerRenderer(serverRequest, rerenderWait);
+      const {timeout} =
+        validateConfig(env.config) || ({} as ServerRendererConfig);
+
+      const serverRenderer = new ServerRenderer(serverRequest, timeout);
 
       return {
         '1.0': () => ({featureService: serverRenderer})

diff --git a/packages/server-renderer/src/internal/debounce-async.ts b/packages/server-renderer/src/internal/debounce-async.ts
diff --git a/packages/server-renderer/src/internal/set-timeout-async.ts b/packages/server-renderer/src/internal/set-timeout-async.ts
@@ -0,0 +1,5 @@
+export async function setTimeoutAsync(duration: number): Promise<void> {
+  return new Promise<void>((resolve: () => void) =>
+    setTimeout(resolve, duration)
+  );
+}