Skip to content

Commit 15e7566

Browse files
author
Ben Warzeski
authored
fix: remove return from useEffect call (openedx#131)
* fix: remove return from useEffect call * fix: text renderer tests
1 parent cba03d3 commit 15e7566

File tree

2 files changed

+46
-26
lines changed

2 files changed

+46
-26
lines changed

src/components/FilePreview/BaseRenderers/textHooks.js

+8-6
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ export const fetchFile = async ({
2222

2323
export const rendererHooks = ({ url, onError, onSuccess }) => {
2424
const [content, setContent] = module.state.content('');
25-
useEffect(() => module.fetchFile({
26-
setContent,
27-
url,
28-
onError,
29-
onSuccess,
30-
}), [onError, onSuccess, setContent, url]);
25+
useEffect(() => {
26+
module.fetchFile({
27+
setContent,
28+
url,
29+
onError,
30+
onSuccess,
31+
});
32+
}, [onError, onSuccess, setContent, url]);
3133
return { content };
3234
};

src/components/FilePreview/BaseRenderers/textHooks.test.js

+38-20
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,17 @@
22
import { useEffect } from 'react';
33
import * as axios from 'axios';
44

5+
import { keyStore } from 'utils';
56
import { MockUseState } from 'testUtils';
67
import * as hooks from './textHooks';
78

89
jest.mock('axios', () => ({
910
get: jest.fn(),
1011
}));
1112

13+
const hookKeys = keyStore(hooks);
1214
const state = new MockUseState(hooks);
15+
1316
let hook;
1417

1518
const testValue = 'test-value';
@@ -46,30 +49,45 @@ describe('Text file preview hooks', () => {
4649
hook = hooks.rendererHooks(props);
4750
[[cb, prereqs]] = useEffect.mock.calls;
4851
};
49-
test('useEffect, predicated on url changes', () => {
52+
it('calls fetchFile method, predicated on setContent, url, and callbacks', () => {
53+
jest.spyOn(hooks, hookKeys.fetchFile).mockImplementationOnce(() => {});
5054
loadHook();
5155
expect(useEffect).toHaveBeenCalled();
52-
expect(prereqs[3]).toEqual(props.url);
53-
});
54-
describe('onSuccess', () => {
55-
it('calls get', async () => {
56-
const testData = 'test-data';
57-
axios.get.mockReturnValueOnce(Promise.resolve({ data: testData }));
58-
loadHook();
59-
await cb(testValue);
60-
expect(props.onSuccess).toHaveBeenCalled();
61-
expect(state.setState[state.keys.content]).toHaveBeenCalledWith(testData);
56+
expect(prereqs).toEqual([
57+
props.onError,
58+
props.onSuccess,
59+
state.setState.content,
60+
props.url,
61+
]);
62+
expect(hooks.fetchFile).not.toHaveBeenCalled();
63+
cb();
64+
expect(hooks.fetchFile).toHaveBeenCalledWith({
65+
onError: props.onError,
66+
onSuccess: props.onSuccess,
67+
setContent: state.setState.content,
68+
url: props.url,
6269
});
6370
});
64-
describe('onError', () => {
65-
it('calls get on the passed url when it changes', async () => {
66-
axios.get.mockReturnValueOnce(Promise.reject(
67-
{ response: { status: testValue } },
68-
));
69-
loadHook();
70-
await cb(testValue);
71-
expect(props.onError).toHaveBeenCalledWith(testValue);
72-
});
71+
});
72+
});
73+
describe('fetchFile', () => {
74+
describe('onSuccess', () => {
75+
it('calls get', async () => {
76+
const testData = 'test-data';
77+
axios.get.mockReturnValueOnce(Promise.resolve({ data: testData }));
78+
await hooks.fetchFile({ ...props, setContent: state.setState.content });
79+
expect(props.onSuccess).toHaveBeenCalled();
80+
expect(state.setState[state.keys.content]).toHaveBeenCalledWith(testData);
81+
});
82+
});
83+
describe('onError', () => {
84+
it('calls get on the passed url when it changes', async (done) => {
85+
axios.get.mockReturnValueOnce(Promise.reject(
86+
{ response: { status: testValue } },
87+
));
88+
await hooks.fetchFile({ ...props, setContent: state.setState.content });
89+
expect(props.onError).toHaveBeenCalledWith(testValue);
90+
done();
7391
});
7492
});
7593
});

0 commit comments

Comments
 (0)