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 middlewareBuilder notation with type-curried arguments to co… #549

Merged
merged 5 commits into from
Jun 13, 2020

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented May 10, 2020

This adds the option to pass a function instead of the middleware array as middleware option on configureStore.

This function receives a middlewareBuilderApi argument of the shape

type MiddlewareBuilderApi<S> = {
  defaultMiddleware: ThunkMiddlewareFor<S>[]
  getDefaultMiddleware: CurriedGetDefaultMiddleware<S>
}

Essentially that state-type-currying allows to do something like

const store = configureStore({
      reducer: reducerA,
      middleware: ({ getDefaultMiddleware }) =>
        [
          otherMiddleware,
          ...getDefaultMiddleware({ thunk: false })
        ] as const
    })

which would before have been

const store = configureStore({
      reducer: reducerA,
      middleware: [
          otherMiddleware,
          ...getDefaultMiddleware<RootState, { thunk: false }>({ thunk: false })
        ] as const
    })

Also, it is now not necessary to know the RootState type, which makes the use of custom middlewares possible in combination with the object-type reducer notation in TS.

(Also, while touching those files anyway, I moved the separated "3.4 tests" back into normal test suite, as we're not supporting pre-3.4 TS any more)

@netlify
Copy link

netlify bot commented May 10, 2020

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

Built with commit 206a378

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 10, 2020

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 206a378:

Sandbox Source
wonderful-herschel-894wp Configuration
unruffled-forest-x0osh Configuration
zealous-firefly-65l1w Configuration

@markerikson
Copy link
Collaborator

What's the reasoning for passing in both getDefaultMiddleware and middleware?

@phryneas
Copy link
Member Author

phryneas commented May 10, 2020

I figured it might be the most common use case to just use the default middleware and extend it like

middleware: ({defaultMiddleware}) => [ ...defaultMiddleware, logger ], but that some people also might want to change the options, so I added both.

But we can go the "only getDefaultMiddleware" (that one is definitely required while the other is more convenience) route as well.

@markerikson
Copy link
Collaborator

markerikson commented May 10, 2020

Yeah, I get the thought of just handing them the array, but let's assume that if they're providing a callback here that they're probably going to want to customize something, or at least that it's not too much work to have them call it here.

@phryneas
Copy link
Member Author

Okay, I cut it down to just the curriedGetDefaultMiddleware.

@markerikson markerikson changed the title add middlewareBuilder notation with type-curried arguments to configureStore add middlewareBuilder notation with type-curried arguments to co… Jun 13, 2020
@markerikson markerikson merged commit bb9ea1e into reduxjs:master Jun 13, 2020
markerikson added a commit that referenced this pull request Jun 13, 2020
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.

2 participants