Skip to content

Commit

Permalink
feat(core): make ExternalsValidator optional for the FeatureServiceRe…
Browse files Browse the repository at this point in the history
…gistry (#343)
  • Loading branch information
unstubbable authored and clebert committed Feb 7, 2019
1 parent 18fba0d commit 9860ff5
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 96 deletions.
187 changes: 121 additions & 66 deletions packages/core/src/__tests__/feature-service-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('FeatureServiceRegistry', () => {
beforeEach(() => {
mockExternalsValidator = {validate: jest.fn()};

featureServiceRegistry = new FeatureServiceRegistry(mockExternalsValidator);
featureServiceRegistry = new FeatureServiceRegistry();

featureServiceA = {kind: 'featureServiceA'};
bindingA = {featureService: featureServiceA};
Expand Down Expand Up @@ -131,10 +131,7 @@ describe('FeatureServiceRegistry', () => {
}
beforeEach(() => {
configs = {a: {kind: 'a'}, c: {kind: 'c'}};
featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator,
{configs}
);
featureServiceRegistry = new FeatureServiceRegistry({configs});
});

it('registers the Feature Services "a", "b", "c" one after the other', () => {
Expand Down Expand Up @@ -339,65 +336,133 @@ describe('FeatureServiceRegistry', () => {
);
});

it('fails to register a Feature Service due to failing externals validation', () => {
const mockError = new Error('mockError');
describe('without an ExternalsValidator provided to the FeatureServiceRegistry', () => {
describe('with a Feature Service definition that is declaring external dependencies', () => {
beforeEach(() => {
providerDefinitionA = {
...providerDefinitionA,
dependencies: {
externals: {
react: '^16.0.0'
}
}
};
});

mockExternalsValidator.validate = jest.fn(() => {
throw mockError;
it("doesn't throw an error", () => {
expect(() => {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);
}).not.toThrowError();
});
});
});

featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
describe('with an ExternalsValidator provided to the FeatureServiceRegistry', () => {
beforeEach(() => {
featureServiceRegistry = new FeatureServiceRegistry({
externalsValidator: mockExternalsValidator
});
});

expect(() => {
featureServiceRegistry.registerFeatureServices(
[
{
id: 'failing-externals',
create: jest.fn(),
dependencies: {
externals: {
react: '^16.0.0'
}
describe('with a Feature Service definition that is failing the externals validation', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('mockError');

mockExternalsValidator.validate = jest.fn(() => {
throw mockError;
});

providerDefinitionA = {
...providerDefinitionA,
dependencies: {
externals: {
react: '^16.0.0'
}
}
],
'test'
);
}).toThrowError(mockError);
};
});

expect(mockExternalsValidator.validate).toHaveBeenCalledWith({
react: '^16.0.0'
});
});
it('calls the provided ExternalsValidator with the defined externals', () => {
try {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);
} catch {}

it('does not fail to register a Feature Service with valid externals', () => {
mockExternalsValidator.validate = jest.fn();
expect(mockExternalsValidator.validate).toHaveBeenCalledWith({
react: '^16.0.0'
});
});

featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
it('throws the validation error', () => {
expect(() => {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);
}).toThrowError(mockError);
});
});

expect(() => {
featureServiceRegistry.registerFeatureServices(
[
{
id: 'with-externals',
create: jest.fn(() => ({})),
dependencies: {
externals: {
react: '^16.0.0'
}
describe('with a Feature Service definition that is not failing the externals validation', () => {
beforeEach(() => {
providerDefinitionA = {
...providerDefinitionA,
dependencies: {
externals: {
react: '^16.0.0'
}
}
],
'test'
);
}).not.toThrowError();
};
});

it('calls the provided ExternalsValidator with the defined externals', () => {
try {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);
} catch {}

expect(mockExternalsValidator.validate).toHaveBeenCalledWith({
react: '^16.0.0'
});
});

it("doesn't throw an error", () => {
expect(() => {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);
}).not.toThrowError();
});
});

expect(mockExternalsValidator.validate).toHaveBeenCalledWith({
react: '^16.0.0'
describe('with a Feature Service definition that declares no externals', () => {
it('does not call the provided ExternalsValidator', () => {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);

expect(mockExternalsValidator.validate).not.toHaveBeenCalled();
});

it("doesn't throw an error", () => {
expect(() => {
featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);
}).not.toThrowError();
});
});
});
});
Expand All @@ -416,9 +481,7 @@ describe('FeatureServiceRegistry', () => {

describe('for a Feature Service consumer with an id specifier and dependencies', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
featureServiceRegistry = new FeatureServiceRegistry();

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
Expand All @@ -443,9 +506,7 @@ describe('FeatureServiceRegistry', () => {

describe('for a Feature Service consumer without an id specifier and dependencies', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
featureServiceRegistry = new FeatureServiceRegistry();

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
Expand All @@ -471,9 +532,7 @@ describe('FeatureServiceRegistry', () => {
describe('for a Feature Service consumer and two optional dependencies', () => {
describe('with the first dependency missing', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
featureServiceRegistry = new FeatureServiceRegistry();

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
Expand All @@ -498,9 +557,7 @@ describe('FeatureServiceRegistry', () => {

describe('with the second dependency missing', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
featureServiceRegistry = new FeatureServiceRegistry();

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
Expand All @@ -525,9 +582,7 @@ describe('FeatureServiceRegistry', () => {

describe('with no dependency missing', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry(
mockExternalsValidator
);
featureServiceRegistry = new FeatureServiceRegistry();

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA, providerDefinitionB],
Expand Down
27 changes: 23 additions & 4 deletions packages/core/src/feature-service-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ export interface FeatureServiceRegistryOptions {
* registered.
*/
readonly configs?: FeatureServiceConfigs;

/**
* If the {@link FeatureAppManager} is configured with a
* {@link FeatureAppManager#moduleLoader}, to load Feature Apps from a remote
* location that also provide their own Feature Services, i.e. the Feature
* Services are included in a different bundle than the integrator bundle, it
* might make sense to validate external dependencies that are required by
* those Feature Services against the shared dependencies that are provided by
* the integrator. This makes it possible that an error is already thrown when
* registering a Feature Service with incompatible external dependencies, and
* thus enables early feedback as to whether a Feature Service is compatible
* with the integration environment.
*/
readonly externalsValidator?: ExternalsValidatorLike;
}

type ProviderId = string;
Expand Down Expand Up @@ -200,7 +214,6 @@ export class FeatureServiceRegistry implements FeatureServiceRegistryLike {
private readonly consumerUids = new Set<string>();

public constructor(
private readonly externalsValidator: ExternalsValidatorLike,
private readonly options: FeatureServiceRegistryOptions = {}
) {}

Expand Down Expand Up @@ -429,12 +442,18 @@ export class FeatureServiceRegistry implements FeatureServiceRegistryLike {
}

private validateExternals(
consumerDefinition: FeatureServiceConsumerDefinition
featureAppDefinition: FeatureServiceConsumerDefinition
): void {
const {dependencies} = consumerDefinition;
const {externalsValidator} = this.options;

if (!externalsValidator) {
return;
}

const {dependencies} = featureAppDefinition;

if (dependencies && dependencies.externals) {
this.externalsValidator.validate(dependencies.externals);
externalsValidator.validate(dependencies.externals);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ export default async function renderApp({
});

const featureAppNodeUrl = `http://localhost:${port}/feature-app.commonjs.js`;
const featureServiceRegistry = new FeatureServiceRegistry(externalsValidator);

const featureServiceRegistry = new FeatureServiceRegistry({
externalsValidator
});

const featureAppManager = new FeatureAppManager(featureServiceRegistry, {
moduleLoader: loadCommonJsModule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const externalsValidator = new ExternalsValidator({
'@feature-hub/react': '0.12.0'
});

const featureServiceRegistry = new FeatureServiceRegistry(externalsValidator);
const featureServiceRegistry = new FeatureServiceRegistry({externalsValidator});

const featureAppManager = new FeatureAppManager(featureServiceRegistry, {
moduleLoader: loadAmdModule,
Expand Down
9 changes: 2 additions & 7 deletions packages/demos/src/history-service/integrator.node.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
ExternalsValidator,
FeatureAppManager,
FeatureServiceRegistry
} from '@feature-hub/core';
import {FeatureAppManager, FeatureServiceRegistry} from '@feature-hub/core';
import {defineHistoryService} from '@feature-hub/history-service';
import {defineServerRequest} from '@feature-hub/server-request';
import * as React from 'react';
Expand All @@ -14,8 +10,7 @@ import {rootLocationTransformer} from './root-location-transformer';
export default async function renderApp({
req
}: AppRendererOptions): Promise<AppRendererResult> {
const externalsValidator = new ExternalsValidator({});
const featureServiceRegistry = new FeatureServiceRegistry(externalsValidator);
const featureServiceRegistry = new FeatureServiceRegistry();

const featureServiceDefinitions = [
defineServerRequest(req),
Expand Down
9 changes: 2 additions & 7 deletions packages/demos/src/history-service/integrator.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
import {
ExternalsValidator,
FeatureAppManager,
FeatureServiceRegistry
} from '@feature-hub/core';
import {FeatureAppManager, FeatureServiceRegistry} from '@feature-hub/core';
import {defineHistoryService} from '@feature-hub/history-service';
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import '../blueprint-css';
import {App} from './app';
import {rootLocationTransformer} from './root-location-transformer';

const externalsValidator = new ExternalsValidator({});
const featureServiceRegistry = new FeatureServiceRegistry(externalsValidator);
const featureServiceRegistry = new FeatureServiceRegistry();

featureServiceRegistry.registerFeatureServices(
[defineHistoryService(rootLocationTransformer)],
Expand Down
2 changes: 1 addition & 1 deletion packages/demos/src/module-loader-amd/integrator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const externalsValidator = new ExternalsValidator({
react: '16.7.0'
});

const featureServiceRegistry = new FeatureServiceRegistry(externalsValidator);
const featureServiceRegistry = new FeatureServiceRegistry({externalsValidator});

const featureAppManager = new FeatureAppManager(featureServiceRegistry, {
moduleLoader: loadAmdModule,
Expand Down
9 changes: 2 additions & 7 deletions packages/demos/src/module-loader-commonjs/integrator.node.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
ExternalsValidator,
FeatureAppManager,
FeatureServiceRegistry
} from '@feature-hub/core';
import {FeatureAppManager, FeatureServiceRegistry} from '@feature-hub/core';
import {loadCommonJsModule} from '@feature-hub/module-loader-commonjs';
import {FeatureAppLoader, FeatureHubContextProvider} from '@feature-hub/react';
import * as React from 'react';
Expand All @@ -13,8 +9,7 @@ export default async function renderApp({
port
}: AppRendererOptions): Promise<AppRendererResult> {
const featureAppNodeUrl = `http://localhost:${port}/feature-app.commonjs.js`;
const externalsValidator = new ExternalsValidator({});
const featureServiceRegistry = new FeatureServiceRegistry(externalsValidator);
const featureServiceRegistry = new FeatureServiceRegistry();

const featureAppManager = new FeatureAppManager(featureServiceRegistry, {
moduleLoader: loadCommonJsModule
Expand Down
Loading

0 comments on commit 9860ff5

Please sign in to comment.