Skip to content

Conversation

bareynol
Copy link
Contributor

When defining an RSAA action, you can define the properties of the RSAA action (body, headers, options) to be either a plain JavaScript object or a function.

If a function, the middleware executes the function to attain the result. The middleware that is executing the function is already an async function, so I added an await before the call of the above RSAA properties, so that async functions could be passed in and still evaluated.

To give a real-world example of why this is necessary: I store credentials for an API on the keychain in React-Native. I set the headers property to be a function that reads the keychain and formats the Authorization header. Accessing the keychain is done asynchronously. This change allows an async function to be used, and does not affect synchronous functions.

@coveralls
Copy link

coveralls commented Dec 11, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 775366b on echolocation:master into 6e782cd on agraboso:master.

Copy link

@jamesjara jamesjara left a comment

Choose a reason for hiding this comment

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

looks good

@nason
Copy link
Collaborator

nason commented Dec 15, 2019

Thank you for the contribution @echolocation! This looks great.

Would you mind adding at least one test case covering this behavior? Something like https://github.com/agraboso/redux-api-middleware/blob/master/src/middleware.test.js#L431 but with an async function?

@bareynol
Copy link
Contributor Author

@nason certainly! Thank you for pointing me directly at a relevant test case.

I've added async versions of the tests for the properties: headers, endpoint, body, and options.

It's the same test for each, but it uses mockResolvedValue to mock async functions

@nason
Copy link
Collaborator

nason commented Dec 18, 2019

Awesome, thanks for adding tests! This looks great. I'll publish a new release with these changes shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants