Skip to content
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.

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

Add check for undefined middleware in configureStore #959

Merged

Conversation

Shrugsy
Copy link
Collaborator

@Shrugsy Shrugsy commented Mar 29, 2021

Closes #937

As per commit message:

* Throw an error when a middleware creation callback
  is provided to configureStore that does not return
  an array of middleware

Two new tests are added:

  • explicitly passing middleware: undefined should use default middleware (same as excluding the option)
  • passing middleware: () => undefined should be considered a user error and throw an error

Note: While issue #937 arose due to a middleware creation function that returns undefined, I've considered "returning anything besides an array" as a user error in this context.

* Throw an error when a middleware creation callback
  is provided to configureStore that does not return
  an array of middleware
@netlify
Copy link

netlify bot commented Mar 29, 2021

Deploy preview for redux-starter-kit-docs ready!

Built with commit 7120607

https://deploy-preview-959--redux-starter-kit-docs.netlify.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 29, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7120607:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration

@markerikson
Copy link
Collaborator

Nice. Two requests:

  • We can make the check be only in dev mode, I think
  • We might as well also verify that all the values in the array are functions and not undefined

* Don't check middleware in production mode

* Check that each item in the middleware array is a function
@Shrugsy
Copy link
Collaborator Author

Shrugsy commented Mar 30, 2021

Nice. Two requests:

  • We can make the check be only in dev mode, I think
  • We might as well also verify that all the values in the array are functions and not undefined

I've added new commits to address these. For checking if each middleware in the array is actually a function, I've made it perform that check regardless of whether a middleware creation callback was used (i.e. passing a bad array to the middleware property directly will throw an error).

Question: Is there any kind of policy about squashing commits on pull requests, or will maintainers handle that?

@markerikson
Copy link
Collaborator

We've got "Squash and Merge" turned on for all the Redux repos. tbh I'm not exactly a fan, but eh :)

@markerikson markerikson merged commit 86f7eea into reduxjs:master Apr 1, 2021
@Shrugsy Shrugsy deleted the 937-add-check-for-undefined-middleware branch April 2, 2021 12:25
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.

Check for undefined middleware on startup
2 participants