fix(history-service): compatibility with react-router-dom (#704)

The methods of `history.History` must be bound explicitly, because
components like `Link` from the `react-router-dom` package deconstruct
methods like `push` and `replace` from the `history`.

packages/demos/package.json

@@ -34,7 +34,8 @@
     "@types/get-port": "^4.0.0",
     "@types/history": "^4.7.2",
     "@types/pino": "^5.8.5",
-    "@types/react-router": "^5.0.2",
+    "@types/react-router": "^5.1.18",
+    "@types/react-router-dom": "^5.3.3",
     "@types/styled-components": "^5.1.4",
     "@types/webpack-dev-middleware": "^4.1.0",
     "copy-webpack-plugin": "^8.0.0",
@@ -53,7 +54,8 @@
     "react-dom": "^16.8.6",
     "react-is": "^16.8.6",
     "react-media": "^1.9.2",
-    "react-router": "^5.0.1",
+    "react-router": "^5.2.1",
+    "react-router-dom": "^5.2.0",
     "style-loader": "^2.0.0",
     "styled-components": "^5.2.0",
     "ts-loader": "^8.0.17",

packages/demos/src/history-service/history-consumer/history-consumer.tsx

@@ -2,6 +2,7 @@ import {Card, H5} from '@blueprintjs/core';
 import * as React from 'react';
 import Media from 'react-media';
 import {Route} from 'react-router';
+import {Link} from 'react-router-dom';
 import {NewPathControl} from './new-path-control';
 import {PathnameLabel} from './pathname-label';
 
@@ -24,6 +25,17 @@ export function HistoryConsumer({
         )}
       </Route>
 
+      <p>
+        <Link to="/foo" id={`push-link-${specifier}`}>
+          Push /foo
+        </Link>
+      </p>
+      <p>
+        <Link to="/bar" id={`replace-link-${specifier}`} replace>
+          Replace /bar
+        </Link>
+      </p>
+
       <Media query="(max-width: 370px)">
         {(matches) => (
           <Route>

packages/demos/src/history-service/history-consumer/new-path-control.tsx

@@ -27,17 +27,17 @@ export function NewPathControl({
   return (
     <ControlGroup vertical={vertical}>
       <InputGroup
-        id={`new-path-${specifier}`}
+        id={`new-path-input-${specifier}`}
         placeholder="Enter a new path..."
         inputRef={(ref) => (inputElement.current = ref)}
       />
       <Button
-        id={`push-${specifier}`}
+        id={`push-button-${specifier}`}
         text="Push"
         onClick={() => changePath('push')}
       />
       <Button
-        id={`replace-${specifier}`}
+        id={`replace-button-${specifier}`}
         text="Replace"
         onClick={() => changePath('replace')}
       />

packages/demos/src/history-service/history-consumer/pathname-label.tsx

@@ -13,7 +13,11 @@ export function PathnameLabel({
   return (
     <Label>
       Pathname
-      <InputGroup id={`pathname-${specifier}`} value={pathname} disabled />
+      <InputGroup
+        id={`pathname-input-${specifier}`}
+        value={pathname}
+        disabled
+      />
     </Label>
   );
 }

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

@@ -19,45 +19,60 @@ class HistoryConsumerUi {
   ) {}
 
   public async getPathname(): Promise<string> {
-    const pathnameInput = await this.getPathnameInput();
-
+    const pathnameInput = await this.getElement('pathname-input');
     const value = await pathnameInput.getProperty('value');
 
     return value.jsonValue();
   }
 
   public async push(pathname: string): Promise<void> {
-    await (await this.getNewPathInput()).type(pathname);
+    await (await this.getElement('new-path-input')).type(pathname);
 
-    await this.browser.waitForNavigation((await this.getPushButton()).click());
+    await this.browser.waitForNavigation(
+      (await this.getElement('push-button')).click()
+    );
   }
 
   public async replace(pathname: string): Promise<void> {
-    await (await this.getNewPathInput()).type(pathname);
+    await (await this.getElement('new-path-input')).type(pathname);
 
     await this.browser.waitForNavigation(
-      (await this.getReplaceButton()).click()
+      (await this.getElement('replace-button')).click()
     );
   }
 
-  private async getNewPathInput(): Promise<ElementHandle<HTMLInputElement>> {
-    // tslint:disable-next-line:no-non-null-assertion
-    return (await page.$(`#new-path-${this.specifier}`))!;
-  }
-
-  private async getPathnameInput(): Promise<ElementHandle<HTMLInputElement>> {
-    // tslint:disable-next-line:no-non-null-assertion
-    return (await page.$(`#pathname-${this.specifier}`))!;
+  public async clickPushLink(): Promise<void> {
+    await this.browser.waitForNavigation(
+      (await this.getElement('push-link')).click()
+    );
   }
 
-  private async getPushButton(): Promise<ElementHandle<HTMLButtonElement>> {
-    // tslint:disable-next-line:no-non-null-assertion
-    return (await page.$(`#push-${this.specifier}`))!;
+  public async clickReplaceLink(): Promise<void> {
+    await this.browser.waitForNavigation(
+      (await this.getElement('replace-link')).click()
+    );
   }
 
-  private async getReplaceButton(): Promise<ElementHandle<HTMLButtonElement>> {
+  private async getElement(
+    prefix: 'new-path-input' | 'pathname-input'
+  ): Promise<ElementHandle<HTMLInputElement>>;
+  private async getElement(
+    prefix: 'push-button' | 'replace-button'
+  ): Promise<ElementHandle<HTMLButtonElement>>;
+  private async getElement(
+    prefix: 'push-link' | 'replace-link'
+  ): Promise<ElementHandle<HTMLAnchorElement>>;
+  private async getElement(
+    prefix:
+      | 'new-path-input'
+      | 'pathname-input'
+      | 'push-button'
+      | 'replace-button'
+      | 'push-link'
+      | 'replace-link'
+  ): Promise<ElementHandle<HTMLElement>> {
     // tslint:disable-next-line:no-non-null-assertion
-    return (await page.$(`#replace-${this.specifier}`))!;
+    return (await page.$(`#${prefix}-${this.specifier}`))!;
   }
 }
 
@@ -139,7 +154,7 @@ describe('integration test: "history-service"', () => {
     expect(await b.getPathname()).toBe('/');
   });
 
-  test('Scenario 6: Consumer A and B both push new pathnames', async () => {
+  test('Scenario 6: Consumers A and B both push new pathnames', async () => {
     await browser.goto(url);
 
     expect(await browser.getPath()).toBe('/');
@@ -211,4 +226,25 @@ describe('integration test: "history-service"', () => {
     // Re-enable JavaScript to restore the default behavior for all other tests.
     await page.setJavaScriptEnabled(true);
   });
+
+  test('Scenario 11: Consumers A and B push/replace a new pathnames using the Link component from react-router-dom', async () => {
+    await browser.goto(url);
+
+    expect(await browser.getPath()).toBe('/');
+
+    await a.clickPushLink();
+
+    expect(await browser.getPath()).toBe('/?test:history-consumer:a=/foo');
+    expect(await a.getPathname()).toBe('/foo');
+    expect(await b.getPathname()).toBe('/');
+
+    await b.clickReplaceLink();
+
+    expect(await browser.getPath()).toBe(
+      '/?test:history-consumer:a=/foo&test:history-consumer:b=/bar'
+    );
+
+    expect(await a.getPathname()).toBe('/foo');
+    expect(await b.getPathname()).toBe('/bar');
+  });
 });

packages/history-service/src/internal/browser-consumer-history.ts

@@ -21,6 +21,10 @@ export class BrowserConsumerHistory extends ConsumerHistory {
         this.handleRootLocationChange(action);
       }
     );
+
+    this.listen = this.listen.bind(this);
+    this.push = this.push.bind(this);
+    this.replace = this.replace.bind(this);
   }
 
   public destroy(): void {

packages/history-service/src/internal/consumer-history.ts

@@ -17,6 +17,19 @@ export abstract class ConsumerHistory implements history.History {
       undefined,
       history.createLocation('/')
     );
+
+    /**
+     * The methods of `history.History` must be bound explicitly, because
+     * components like `Link` from the `react-router-dom` package deconstruct
+     * methods like `push` and `replace` from the `history`.
+     */
+    this.push = this.push.bind(this);
+    this.replace = this.replace.bind(this);
+    this.go = this.go.bind(this);
+    this.goBack = this.goBack.bind(this);
+    this.goForward = this.goForward.bind(this);
+    this.block = this.block.bind(this);
+    this.createHref = this.createHref.bind(this);
   }
 
   public get length(): number {

packages/history-service/src/internal/static-consumer-history.ts

@@ -1,7 +1,19 @@
 import * as history from 'history';
 import {ConsumerHistory} from './consumer-history';
+import {HistoryMultiplexer} from './history-multiplexer';
+import {HistoryServiceContext} from './history-service-context';
 
 export class StaticConsumerHistory extends ConsumerHistory {
+  public constructor(
+    context: HistoryServiceContext,
+    historyKey: string,
+    historyMultiplexer: HistoryMultiplexer
+  ) {
+    super(context, historyKey, historyMultiplexer);
+
+    this.listen = this.listen.bind(this);
+  }
+
   public listen(): history.UnregisterCallback {
     this.context.logger.warn('history.listen() is not supported.');