Skip to content

Handle sync requests without await #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 24, 2018
Merged

Handle sync requests without await #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,14 @@ function apiMiddleware({ getState }) {
}

// We can now dispatch the request FSA
next(await actionWith(requestType, [action, getState()]));
if (
typeof requestType.payload === 'function' ||
typeof requestType.meta === 'function'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the other requestType properties that can be passed as functions? ie, headers, options or bailout

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headers, options or bailout aren't properties of requestType, they are properties of [RSAA], thus this change doesn't apply to them.

) {
next(await actionWith(requestType, [action, getState()]));
} else {
next(requestType);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible/worth it to add tests that verify this does happen synchronously, as expected?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's neither worth nor possible to test that this happens synchronously. Provided there is no mistake in if condition, and taking into account there is no await in else branch, this should work as expected. I have added 2 tests that cover first branch of this if statement, and they ensure that meta or payload functions are called, that they are called with correct arguments, and that their result is inserted to meta or payload property of request FSA. A lot of existing tests cover the second branch.

}

try {
// Make the API call
Expand Down
118 changes: 118 additions & 0 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,124 @@ test('apiMiddleware must use an [RSAA].options function when present', t => {
actionHandler(anAction);
});

test('apiMiddleware must use payload property of request type descriptor when it is a function', t => {
const api = nock('http://127.0.0.1')
.get('/api/users/1')
.reply(200);
const expectedState = { foo: 'bar' };
const anAction = {
[RSAA]: {
endpoint: 'http://127.0.0.1/api/users/1',
method: 'GET',
types: [
{
type: 'REQUEST',
meta: 'requestMeta',
payload: (action, state) => {
t.pass('request type descriptor payload function has been called');
t.equal(
action,
anAction,
'request type descriptor payload function has been called with correct action parameter'
);
t.equal(
state,
expectedState,
'request type descriptor payload function has been called with correct state parameter'
);
return 'requestPayload';
}
},
'SUCCESS',
'FAILURE'
]
}
};
const doGetState = () => {
return expectedState;
};
const nextHandler = apiMiddleware({ getState: doGetState });
const doNext = action => {
if (action.type === 'REQUEST') {
t.pass('request FSA passed to the next handler');
t.equal(
action.meta,
'requestMeta',
'request FSA has correct meta property'
);
t.equal(
action.payload,
'requestPayload',
'request FSA has correct payload property'
);
t.notOk(action.error, 'request FSA has correct error property');
}
};
const actionHandler = nextHandler(doNext);

t.plan(7);
actionHandler(anAction);
});

test('apiMiddleware must use meta property of request type descriptor when it is a function', t => {
const api = nock('http://127.0.0.1')
.get('/api/users/1')
.reply(200);
const expectedState = { foo: 'bar' };
const anAction = {
[RSAA]: {
endpoint: 'http://127.0.0.1/api/users/1',
method: 'GET',
types: [
{
type: 'REQUEST',
meta: (action, state) => {
t.pass('request type descriptor meta function has been called');
t.equal(
action,
anAction,
'request type descriptor meta function has been called with correct action parameter'
);
t.equal(
state,
expectedState,
'request type descriptor meta function has been called with correct state parameter'
);
return 'requestMeta';
},
payload: 'requestPayload'
},
'SUCCESS',
'FAILURE'
]
}
};
const doGetState = () => {
return expectedState;
};
const nextHandler = apiMiddleware({ getState: doGetState });
const doNext = action => {
if (action.type === 'REQUEST') {
t.pass('request FSA passed to the next handler');
t.equal(
action.meta,
'requestMeta',
'request FSA has correct meta property'
);
t.equal(
action.payload,
'requestPayload',
'request FSA has correct payload property'
);
t.notOk(action.error, 'request FSA has correct error property');
}
};
const actionHandler = nextHandler(doNext);

t.plan(7);
actionHandler(anAction);
});

test('apiMiddleware must dispatch a success FSA on a successful API call with a non-empty JSON response', t => {
const api = nock('http://127.0.0.1')
.get('/api/users/1')
Expand Down