Skip to content

Commit

Permalink
feat(react): improve logging (#226)
Browse files Browse the repository at this point in the history
- The messages/errors logged by the React `FeatureAppContainer` have been improved.
- The messages/errors logged by the React `FeatureAppLoader` have been improved.
  • Loading branch information
clebert authored Jan 4, 2019
1 parent 0bfbe24 commit 534f9f9
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('FeatureAppContainer (on Node.js)', () => {
{attachTo: 'foo'},
{render: 'foo'}
]) {
describe(`when an invalid feature app (${JSON.stringify(
describe(`when an invalid Feature App (${JSON.stringify(
invalidFeatureApp
)}) is created`, () => {
beforeEach(() => {
Expand All @@ -59,7 +59,7 @@ describe('FeatureAppContainer (on Node.js)', () => {

it('logs and throws an error', () => {
const expectedError = new Error(
'Invalid feature app found. The feature app must be an object with either 1) a `render` method that returns a react element, or 2) an `attachTo` method that accepts a container DOM element.'
'Invalid Feature App found. The Feature App must be an object with either 1) a `render` method that returns a React element, or 2) an `attachTo` method that accepts a container DOM element.'
);

expect(() =>
Expand All @@ -76,11 +76,11 @@ describe('FeatureAppContainer (on Node.js)', () => {
});
}

describe('when a feature app scope fails to be created', () => {
describe('when a Feature App scope fails to be created', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to create feature app scope.');
mockError = new Error('Failed to create Feature App scope.');

mockGetFeatureAppScope.mockImplementation(() => {
throw mockError;
Expand Down
38 changes: 19 additions & 19 deletions packages/react/src/__tests__/feature-app-container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('FeatureAppContainer', () => {
spyConsoleError.mockRestore();
});

it('calls the manager with the given feature app definition and id specifier', () => {
it('calls the manager with the given Feature App definition and id specifier', () => {
shallow(
<FeatureAppContainer
manager={mockManager}
Expand All @@ -50,17 +50,17 @@ describe('FeatureAppContainer', () => {
]);
});

describe('with a react feature app', () => {
describe('with a React Feature App', () => {
beforeEach(() => {
mockFeatureAppScope = {
featureApp: {
render: () => <div>This is the react feature app.</div>
render: () => <div>This is the React Feature App.</div>
},
destroy: jest.fn()
};
});

it('renders the react element', () => {
it('renders the React element', () => {
const wrapper = shallow(
<FeatureAppContainer
manager={mockManager}
Expand All @@ -70,13 +70,13 @@ describe('FeatureAppContainer', () => {

expect(wrapper).toMatchInlineSnapshot(`
<div>
This is the react feature app.
This is the React Feature App.
</div>
`);
});

describe('when unmounted', () => {
it('calls destroy() on the feature app scope', () => {
it('calls destroy() on the Feature App scope', () => {
const wrapper = shallow(
<FeatureAppContainer
manager={mockManager}
Expand All @@ -91,11 +91,11 @@ describe('FeatureAppContainer', () => {
expect(mockFeatureAppScope.destroy).toHaveBeenCalledTimes(1);
});

describe('when the feature app scope throws an error while being destroyed', () => {
describe('when the Feature App scope throws an error while being destroyed', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to destroy feature app scope');
mockError = new Error('Failed to destroy Feature App scope');

mockFeatureAppScope.destroy = () => {
throw mockError;
Expand All @@ -118,19 +118,19 @@ describe('FeatureAppContainer', () => {
});
});

describe('with a dom feature app', () => {
describe('with a DOM Feature App', () => {
beforeEach(() => {
mockFeatureAppScope = {
featureApp: {
attachTo(container: HTMLElement): void {
container.innerHTML = 'This is the dom feature app.';
container.innerHTML = 'This is the DOM Feature App.';
}
},
destroy: jest.fn()
};
});

it("renders a container and passes it to the feature app's render method", () => {
it("renders a container and passes it to the Feature App's render method", () => {
const wrapper = mount(
<FeatureAppContainer
manager={mockManager}
Expand All @@ -139,12 +139,12 @@ describe('FeatureAppContainer', () => {
);

expect(wrapper.html()).toMatchInlineSnapshot(
'"<div>This is the dom feature app.</div>"'
'"<div>This is the DOM Feature App.</div>"'
);
});

describe('when unmounted', () => {
it('calls destroy() on the feature app scope', () => {
it('calls destroy() on the Feature App scope', () => {
const wrapper = shallow(
<FeatureAppContainer
manager={mockManager}
Expand All @@ -159,11 +159,11 @@ describe('FeatureAppContainer', () => {
expect(mockFeatureAppScope.destroy).toHaveBeenCalledTimes(1);
});

describe('when the feature app scope throws an error while being destroyed', () => {
describe('when the Feature App scope throws an error while being destroyed', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to destroy feature app scope');
mockError = new Error('Failed to destroy Feature App scope');

mockFeatureAppScope.destroy = () => {
throw mockError;
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('FeatureAppContainer', () => {
{attachTo: 'foo'},
{render: 'foo'}
]) {
describe(`when an invalid feature app (${JSON.stringify(
describe(`when an invalid Feature App (${JSON.stringify(
invalidFeatureApp
)}) is created`, () => {
beforeEach(() => {
Expand All @@ -214,19 +214,19 @@ describe('FeatureAppContainer', () => {
expect(wrapper.isEmptyRender()).toBe(true);

const expectedError = new Error(
'Invalid feature app found. The feature app must be an object with either 1) a `render` method that returns a react element, or 2) an `attachTo` method that accepts a container DOM element.'
'Invalid Feature App found. The Feature App must be an object with either 1) a `render` method that returns a React element, or 2) an `attachTo` method that accepts a container DOM element.'
);

expect(spyConsoleError.mock.calls).toEqual([[expectedError]]);
});
});
}

describe('when a feature app scope fails to be created', () => {
describe('when a Feature App scope fails to be created', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to create feature app scope.');
mockError = new Error('Failed to create Feature App scope.');

mockGetFeatureAppScope.mockImplementation(() => {
throw mockError;
Expand Down
10 changes: 5 additions & 5 deletions packages/react/src/__tests__/feature-app-loader.node.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('FeatureAppLoader (on Node.js)', () => {
});

describe('without a nodeSrc', () => {
it('does not try to load a feature app definition', () => {
it('does not try to load a Feature App definition', () => {
shallow(<FeatureAppLoader manager={mockManager} src="example.js" />, {
disableLifecycleMethods: true
});
Expand All @@ -54,7 +54,7 @@ describe('FeatureAppLoader (on Node.js)', () => {
});

describe('with a nodeSrc', () => {
it('loads a feature app definition for the nodeSrc', () => {
it('loads a Feature App definition for the nodeSrc', () => {
shallow(
<FeatureAppLoader
manager={mockManager}
Expand All @@ -71,11 +71,11 @@ describe('FeatureAppLoader (on Node.js)', () => {
]);
});

describe('when the async feature app definition synchronously has an error', () => {
describe('when the async Feature App definition synchronously has an error', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to load feature app module.');
mockError = new Error('Failed to load Feature App module.');
mockAsyncFeatureAppDefinition.error = mockError;
});

Expand All @@ -96,7 +96,7 @@ describe('FeatureAppLoader (on Node.js)', () => {

expect(spyConsoleError.mock.calls).toEqual([
[
'The feature app for the src "example-node.js" and the ID specifier "testIdSpecifier" could not be rendered.',
'The Feature App for the src "example-node.js" and the ID specifier "testIdSpecifier" could not be rendered.',
mockError
]
]);
Expand Down
18 changes: 9 additions & 9 deletions packages/react/src/__tests__/feature-app-loader.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('FeatureAppLoader', () => {
});
});

describe('when a feature app definition is synchronously available', () => {
describe('when a Feature App definition is synchronously available', () => {
let mockFeatureAppDefinition: FeatureAppDefinition<unknown>;

beforeEach(() => {
Expand Down Expand Up @@ -134,11 +134,11 @@ describe('FeatureAppLoader', () => {
});
});

describe('when the async feature app definition synchronously has an error', () => {
describe('when the async Feature App definition synchronously has an error', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to load feature app module.');
mockError = new Error('Failed to load Feature App module.');

mockAsyncFeatureAppDefinition = new AsyncValue(
Promise.reject(mockError),
Expand All @@ -160,14 +160,14 @@ describe('FeatureAppLoader', () => {

expect(spyConsoleError.mock.calls).toEqual([
[
'The feature app for the src "/test.js" and the ID specifier "testIdSpecifier" could not be rendered.',
'The Feature App for the src "/test.js" and the ID specifier "testIdSpecifier" could not be rendered.',
mockError
]
]);
});
});

describe('when a feature app definition is loaded asynchronously', () => {
describe('when a Feature App definition is loaded asynchronously', () => {
let mockFeatureAppDefinition: FeatureAppDefinition<unknown>;

beforeEach(() => {
Expand Down Expand Up @@ -227,11 +227,11 @@ describe('FeatureAppLoader', () => {
});
});

describe('when a feature app definition fails to load asynchronously', () => {
describe('when a Feature App definition fails to load asynchronously', () => {
let mockError: Error;

beforeEach(() => {
mockError = new Error('Failed to load feature app module.');
mockError = new Error('Failed to load Feature App module.');

mockAsyncFeatureAppDefinition = new AsyncValue(
new Promise<FeatureAppDefinition<unknown>>((_, reject) =>
Expand Down Expand Up @@ -262,7 +262,7 @@ describe('FeatureAppLoader', () => {

expect(spyConsoleError.mock.calls).toEqual([
[
'The feature app for the src "/test.js" and the ID specifier "testIdSpecifier" could not be rendered.',
'The Feature App for the src "/test.js" and the ID specifier "testIdSpecifier" could not be rendered.',
mockError
]
]);
Expand All @@ -286,7 +286,7 @@ describe('FeatureAppLoader', () => {

expect(spyConsoleError.mock.calls).toEqual([
[
'The feature app for the src "/test.js" could not be rendered.',
'The Feature App for the src "/test.js" could not be rendered.',
mockError
]
]);
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/feature-app-container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class FeatureAppContainer extends React.PureComponent<

if (!isFeatureApp(this.featureAppScope.featureApp)) {
throw new Error(
'Invalid feature app found. The feature app must be an object with either 1) a `render` method that returns a react element, or 2) an `attachTo` method that accepts a container DOM element.'
'Invalid Feature App found. The Feature App must be an object with either 1) a `render` method that returns a React element, or 2) an `attachTo` method that accepts a container DOM element.'
);
}

Expand All @@ -56,7 +56,7 @@ export class FeatureAppContainer extends React.PureComponent<
console.error(error);

if (!inBrowser) {
// TODO: we should only throw for "mission critical" feature apps ...
// TODO: we should only throw for "mission critical" Feature Apps ...
throw error;
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/react/src/feature-app-loader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class FeatureAppLoader extends React.PureComponent<
this.reportError(asyncFeatureAppDefinition.error);

if (!inBrowser) {
// TODO: we should only throw for "mission critical" feature apps ...
// TODO: we should only throw for "mission critical" Feature Apps ...
throw asyncFeatureAppDefinition.error;
}

Expand Down Expand Up @@ -148,12 +148,12 @@ export class FeatureAppLoader extends React.PureComponent<

console.error(
idSpecifier
? `The feature app for the src ${JSON.stringify(
? `The Feature App for the src ${JSON.stringify(
src
)} and the ID specifier ${JSON.stringify(
idSpecifier
)} could not be rendered.`
: `The feature app for the src ${JSON.stringify(
: `The Feature App for the src ${JSON.stringify(
src
)} could not be rendered.`,
error
Expand Down

0 comments on commit 534f9f9

Please sign in to comment.