Skip to content

Commit

Permalink
feat(history-service): use Logger Feature Service instead of console (#…
Browse files Browse the repository at this point in the history 
…414)

The integrator can now provide the Logger Feature Service, which the History Service then uses for logging. The History Service declares the Logger Feature Service as an optional dependency, and uses console as a fallback logger, when the Logger Feature Service is not provided.
  • Loading branch information
@unstubbable
unstubbable authored Mar 12, 2019
1 parent 97d5f3e commit a363757
Show file tree
Hide file tree
Showing 11 changed files with 154 additions and 63 deletions.
1 change: 1 addition & 0 deletions packages/history-service/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"typings": "lib/cjs/index.d.ts",
"dependencies": {
"@feature-hub/core": "^1.1.0",
"@feature-hub/logger": "^0.0.0",
"@feature-hub/server-request": "^1.1.0",
"fast-deep-equal": "^2.0.1"
},
Expand Down
80 changes: 49 additions & 31 deletions packages/history-service/src/__tests__/define-history-service.node.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,23 @@ import {Stubbed, stubMethods} from 'jest-stub-methods';
import {
HistoryServiceDependencies,
HistoryServiceV1,
RootLocationTransformer,
defineHistoryService
} from '..';
import {testRootLocationTransformer} from '../internal/test-root-location-transformer';
import {stubbedLogger} from './stubbed-logger';

describe('HistoryServiceV1 (on Node.js)', () => {
let createHistoryServiceBinder: (
serverRequest: ServerRequestV1 | undefined,
rootLocationTransformer?: RootLocationTransformer
) => FeatureServiceBinder<HistoryServiceV1>;
let mockEnv: FeatureServiceEnvironment<undefined, HistoryServiceDependencies>;

let stubbedConsole: Stubbed<Console>;
let createHistoryServiceBinder: () => FeatureServiceBinder<HistoryServiceV1>;

beforeEach(() => {
stubbedConsole = stubMethods(console);

createHistoryServiceBinder = (
serverRequest: ServerRequestV1 | undefined
) => {
const mockEnv: FeatureServiceEnvironment<
undefined,
HistoryServiceDependencies
> = {
config: undefined,
featureServices: {
's2:server-request': serverRequest
}
};
mockEnv = {
config: undefined,
featureServices: {'s2:logger': stubbedLogger}
};

createHistoryServiceBinder = () => {
const sharedHistoryService = defineHistoryService(
testRootLocationTransformer
).create(mockEnv);
Expand All @@ -53,10 +41,6 @@ describe('HistoryServiceV1 (on Node.js)', () => {
};
});

afterEach(() => {
stubbedConsole.restore();
});

describe('#createStaticHistory()', () => {
let historyBinding1: FeatureServiceBinding<HistoryServiceV1>;
let historyBinding2: FeatureServiceBinding<HistoryServiceV1>;
Expand All @@ -66,7 +50,9 @@ describe('HistoryServiceV1 (on Node.js)', () => {
let history2: History;

const createHistories = (serverRequest: ServerRequestV1 | undefined) => {
const historyServiceBinder = createHistoryServiceBinder(serverRequest);
mockEnv.featureServices['s2:server-request'] = serverRequest;

const historyServiceBinder = createHistoryServiceBinder();

historyBinding1 = historyServiceBinder('test:1');
historyService1 = historyBinding1.featureService;
Expand Down Expand Up @@ -108,7 +94,7 @@ describe('HistoryServiceV1 (on Node.js)', () => {
history1
);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'createStaticHistory was called multiple times by consumer "test:1". Returning the same history instance as before.'
);
});
Expand Down Expand Up @@ -260,7 +246,7 @@ describe('HistoryServiceV1 (on Node.js)', () => {

expect(historyService1.staticRootLocation).toBe(rootLocation);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.go() is not supported.'
);
});
Expand All @@ -276,7 +262,7 @@ describe('HistoryServiceV1 (on Node.js)', () => {

expect(historyService1.staticRootLocation).toBe(rootLocation);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.goBack() is not supported.'
);
});
Expand All @@ -292,7 +278,7 @@ describe('HistoryServiceV1 (on Node.js)', () => {

expect(historyService1.staticRootLocation).toBe(rootLocation);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.goForward() is not supported.'
);
});
Expand All @@ -311,7 +297,7 @@ describe('HistoryServiceV1 (on Node.js)', () => {

expect(promptHookSpy).not.toHaveBeenCalled();

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.block() is not supported.'
);
});
Expand All @@ -333,7 +319,7 @@ describe('HistoryServiceV1 (on Node.js)', () => {

expect(listenSpy).not.toHaveBeenCalled();

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.listen() is not supported.'
);
});
Expand Down Expand Up @@ -366,4 +352,36 @@ describe('HistoryServiceV1 (on Node.js)', () => {
});
});
});

describe('when no Logger Feature Service is provided', () => {
let stubbedConsole: Stubbed<Console>;

beforeEach(() => {
stubbedConsole = stubMethods(console);

mockEnv.featureServices['s2:logger'] = undefined;

mockEnv.featureServices['s2:server-request'] = {
url: '/',
cookies: {},
headers: {}
};
});

afterEach(() => {
stubbedConsole.restore();
});

it('logs messages using the console', () => {
const historyServiceBinder = createHistoryServiceBinder();
const historyService = historyServiceBinder('test:1').featureService;
const staticHistory = historyService.createStaticHistory();

staticHistory.go(-1);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
'history.go() is not supported.'
);
});
});
});
56 changes: 42 additions & 14 deletions packages/history-service/src/__tests__/define-history-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
defineHistoryService
} from '..';
import {testRootLocationTransformer} from '../internal/test-root-location-transformer';
import {stubbedLogger} from './stubbed-logger';

const simulateOnPopState = (state: unknown, url: string) => {
// We need to use pushState to change to the URL that should set by the popstate event.
Expand All @@ -41,7 +42,10 @@ describe('defineHistoryService', () => {
expect(historyServiceDefinition.dependencies).toBeUndefined();

expect(historyServiceDefinition.optionalDependencies).toEqual({
featureServices: {'s2:server-request': '^1.0.0'}
featureServices: {
's2:logger': '^1.0.0',
's2:server-request': '^1.0.0'
}
});
});

Expand All @@ -57,28 +61,28 @@ describe('defineHistoryService', () => {
});

describe('HistoryServiceV1', () => {
let mockEnv: FeatureServiceEnvironment<
undefined,
HistoryServiceDependencies
>;

let createHistoryServiceBinder: () => FeatureServiceBinder<
HistoryServiceV1
>;

let pushStateSpy: jest.SpyInstance;
let replaceStateSpy: jest.SpyInstance;
let stubbedConsole: Stubbed<Console>;

beforeEach(() => {
// ensure the window.location.href is the same before each test
window.history.replaceState(null, '', 'http://example.com');

pushStateSpy = jest.spyOn(window.history, 'pushState');
replaceStateSpy = jest.spyOn(window.history, 'replaceState');
stubbedConsole = stubMethods(console);

const mockEnv: FeatureServiceEnvironment<
undefined,
HistoryServiceDependencies
> = {
mockEnv = {
config: undefined,
featureServices: {}
featureServices: {'s2:logger': stubbedLogger}
};

createHistoryServiceBinder = () => {
Expand All @@ -93,7 +97,6 @@ describe('defineHistoryService', () => {
afterEach(() => {
pushStateSpy.mockRestore();
replaceStateSpy.mockRestore();
stubbedConsole.restore();
});

describe('when the history service consumer is destroyed without having created a browser history', () => {
Expand Down Expand Up @@ -136,7 +139,7 @@ describe('defineHistoryService', () => {
history1
);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'createBrowserHistory was called multiple times by consumer "test:1". Returning the same history instance as before.'
);
});
Expand Down Expand Up @@ -297,7 +300,7 @@ describe('defineHistoryService', () => {

expect(window.location.href).toBe(href);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.go() is not supported.'
);
});
Expand All @@ -313,7 +316,7 @@ describe('defineHistoryService', () => {

expect(window.location.href).toBe(href);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.goBack() is not supported.'
);
});
Expand All @@ -329,7 +332,7 @@ describe('defineHistoryService', () => {

expect(window.location.href).toBe(href);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.goForward() is not supported.'
);
});
Expand All @@ -348,7 +351,7 @@ describe('defineHistoryService', () => {

expect(promptHookSpy).not.toHaveBeenCalled();

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
expect(stubbedLogger.warn).toHaveBeenCalledWith(
'history.block() is not supported.'
);
});
Expand Down Expand Up @@ -621,5 +624,30 @@ describe('defineHistoryService', () => {
});
});
});

describe('when no Logger Feature Service is provided', () => {
let stubbedConsole: Stubbed<Console>;

beforeEach(() => {
stubbedConsole = stubMethods(console);
mockEnv.featureServices['s2:logger'] = undefined;
});

afterEach(() => {
stubbedConsole.restore();
});

it('logs messages using the console', () => {
const historyServiceBinder = createHistoryServiceBinder();
const historyService = historyServiceBinder('test:1').featureService;
const browserHistory = historyService.createBrowserHistory();

browserHistory.go(-1);

expect(stubbedConsole.stub.warn).toHaveBeenCalledWith(
'history.go() is not supported.'
);
});
});
});
});
12 changes: 12 additions & 0 deletions packages/history-service/src/__tests__/stubbed-logger.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// tslint:disable:no-implicit-dependencies

import {Logger} from '@feature-hub/logger';
import {Stub} from 'jest-stub-methods';

export const stubbedLogger: Stub<Logger> = {
trace: jest.fn(),
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn()
};
17 changes: 12 additions & 5 deletions packages/history-service/src/define-history-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import {
FeatureServices,
SharedFeatureService
} from '@feature-hub/core';
import {Logger} from '@feature-hub/logger';
import {ServerRequestV1} from '@feature-hub/server-request';
import * as history from 'history';
import {RootLocationTransformer} from './create-root-location-transformer';
import {createHistoryMultiplexers} from './internal/create-history-multiplexers';
import {createHistoryServiceV1Binder} from './internal/create-history-service-v1-binder';
import {createHistoryServiceContext} from './internal/history-service-context';

/**
* @deprecated Use {@link HistoryServiceV1} instead.
Expand All @@ -27,6 +29,7 @@ export interface SharedHistoryService extends SharedFeatureService {
}

export interface HistoryServiceDependencies extends FeatureServices {
's2:logger'?: Logger;
's2:server-request'?: ServerRequestV1;
}

Expand All @@ -38,20 +41,24 @@ export function defineHistoryService(
> {
return {
id: 's2:history',

optionalDependencies: {
featureServices: {'s2:server-request': '^1.0.0'}
featureServices: {
's2:logger': '^1.0.0',
's2:server-request': '^1.0.0'
}
},

create: env => {
const serverRequest = env.featureServices['s2:server-request'];
const context = createHistoryServiceContext(env.featureServices);

const historyMultiplexers = createHistoryMultiplexers(
rootLocationTransformer,
serverRequest
context,
rootLocationTransformer
);

return {
'1.0.0': createHistoryServiceV1Binder(historyMultiplexers)
'1.0.0': createHistoryServiceV1Binder(context, historyMultiplexers)
};
}
};
Expand Down
4 changes: 3 additions & 1 deletion packages/history-service/src/internal/browser-consumer-history.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@ import equal from 'fast-deep-equal';
import * as history from 'history';
import {ConsumerHistory} from './consumer-history';
import {HistoryMultiplexer} from './history-multiplexer';
import {HistoryServiceContext} from './history-service-context';

export class BrowserConsumerHistory extends ConsumerHistory {
private readonly listeners = new Set<history.LocationListener>();
private readonly unregisterCallbacks: history.UnregisterCallback[] = [];
private readonly browserUnregister: () => void;

public constructor(
context: HistoryServiceContext,
consumerUid: string,
historyMultiplexer: HistoryMultiplexer
) {
super(consumerUid, historyMultiplexer);
super(context, consumerUid, historyMultiplexer);

this.browserUnregister = historyMultiplexer.listenForPop(() => {
this.handlePop();
Expand Down
Loading

0 comments on commit a363757

Please sign in to comment.