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

getDefaultMiddleware deprecated #1324

Closed
honzahruby opened this issue Jul 19, 2021 · 10 comments
Closed

getDefaultMiddleware deprecated #1324

honzahruby opened this issue Jul 19, 2021 · 10 comments

Comments

@honzahruby
Copy link

honzahruby commented Jul 19, 2021

What's the reason to deprecate and eventually remove this function? I was only able to find #958 (comment) as a reasoning. We're using it for dependency injection in thunks and due to code-splitting, we need to do it post store creation (using redux-dynamic-middlewares). I don't see any way to do this using the callback notation. Is there any chance of this function not being removed? Great job with the library, by the way, thank you!

@markerikson
Copy link
Collaborator

The reasons are:

  • getDefaultMiddleware was originally just a separate export - we added the callback form of the middleware option that receives getDefaultMiddleware as its argument later. That callback completely supersedes the reason to use the imported version
  • In addition, the callback form has better type behavior than the import version

Can you give a specific example of where and how you're using the export version?

@honzahruby
Copy link
Author

honzahruby commented Jul 19, 2021

To give you some more context:

  • 90% of our functionality is behind oauth login while storing the token in redux
  • we inject generated API clients as thunk extra argument

The flow goes like this:

  1. create a minimal store with a subset of reducers and redux-dynamic-middlewares as the only middleware (using configureStore)
  2. log in, lazily load inner-app components and their side-effects
  3. enhance the store with all reducers and inject more middlewares, including the ones from getDefaultMiddleware

Because of the size of the injected thunk arguments, I want to avoid doing it in step 1, if that makes sense 🙂

@markerikson
Copy link
Collaborator

Hmm. I'm not clear on why you need to re-add the default middleware if you're using something like redux-dynamic-middleware.

I would expect that those only need to be defined and added once, and then redux-dynamic-middleware would let you add and remove middleware after those without touching any of the defaults.

If you could put together some kind of a sandbox or something that demonstrates this, it would help me understand how this is supposed to behave.

@honzahruby
Copy link
Author

Because I want to set up the thunk middleware after creating the store (not immediately in configureStore).

What I can do as a workaround is to use the getDefaultMiddleware callback with thunk: false and add it later as a standalone middleware.

I'll try to prepare a sandbox.

@markerikson
Copy link
Collaborator

Yeah, that's part of what I don't understand. I can't see any reason to delay loading the thunk middleware, especially if you're modifying the list of middleware later.

Oh. You did say something about injecting an API client.

Mmm... what if you added the thunk middleware on startup, but gave it something like a getApiClient function as the extra argument that would let you set up the client later?

And yeah, skipping the thunk middleware on startup and then creating it yourself later would also be an option.

@honzahruby
Copy link
Author

Yeah, the only reason is the stuff we're injecting as extraArgument. It's quite big and there's no benefit in loading & injecting it immediately, only after the user is logged in and the app is initialized.

I agree this is not a common use case but having getDefaultMiddleware as a standalone export provides more flexibility than just the callback notation.

@markerikson
Copy link
Collaborator

Agreed on both counts.

I guess we could reconsider the deprecation. To be honest I really hadn't even thought about anyone needing or trying to create or modify middleware after store creation.

@phryneas
Copy link
Member

I agree this is not a common use case but having getDefaultMiddleware as a standalone export provides more flexibility than just the callback notation.

But also encourages a big chunk of our users to use an api that gives them less type safety and hurts them in the long run.

@markerikson
Copy link
Collaborator

Also true, and the corollary to that argument is that RTK is meant for the "80% use case" and to push people towards "correct Redux usage".

@honzahruby
Copy link
Author

I agree with that by all means, I was just wondering why is it being deprecated and removed. Type safety is a good argument if there's nothing we can do to improve the original function. I'm sure there will be workarounds for cases like mine.

Thank you for your time, I guess we can close this issue 🙌

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

No branches or pull requests

3 participants