-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[RFC] combineSlices
helper with injectable slice/reducer support and selector wrapping
#2776
Comments
I rewrote all of Etsy's seller-facing webapp to be powered by similar sorts of architecture. A lot of that was based on earlier thinking/work that went into the code-splitting stuff for Twitter's early PWA, which is talked a bit about here. Our Islands architecture, written about here, took that even further out of necessity -- one redux store, any number of islands present on a page. Mostly just including this for context that I've spent 4+ years thinking about this sort of thing in one form or another. If this had existed, it would have made a lot of our extremely-custom code completely unnecessary. You've identified and either avoided or handled the vast majority of the edge cases which were extremely tricky for us. If/when this gets released, I'd probably immediately push for us to start migrating our custom-built architecture over to using this instead. I think we would likely need the "next step" version of the selectors to fully migrate, though. A few thoughts:
|
Thanks for the great feedback! I'm generally curious how this might overlap with the much more complex "Redux modules" concepts from libraries like the ones I linked at #1326 (comment) . I'm most definitely not saying we should have absolute full-blown "dynamic modules" support in RTK. But, I will say that it seems like a noticeable selling point for Vuex/Pinia, and that use case is a weak spot for Redux in general, so it seems worth discussing as a topic for RTK 2.0 and figuring out what is worth adding if anything. @mq2thez could you paste in some samples of what your current setup and usage patterns look like, for reference? |
This will of course also be possible. It is just a bit more circular, so if we introduce a new documented pattern here, I'd rather document both the slice and the types to be injected - but that won't mean you can just import the types to create that interface right there.
That is a good point and certainly needs some investigation - but I think it shouldn't really be the case.
Generally this should not be too much of a problem (just nest a reducer created by
Per default, these "most simple" selectors would not even be memoized by default. That part still needs to be fleshed out and I'm pretty sure we will end up with something that will allow all three variants: "non-memoized", "memoized" and "memoized with options". |
In terms of code samples, this is a version of one of our subapps, with names altered. This sort of example lives in a // SomeSubapp/Loader.ts
import reducerRegistry from "path/to/reducerRegistry";
import reducer from "./reducer";
import OtherReducer from "../OtherSubapp/reducer";
// path to the main component
import App from "./App";
reducerRegistry.register({
subapp1Settings: reducer,
otherSharedCode: OtherReducer,
});
export const SomeSubapp = App; In this example, our main Most subapps define only a single field here (IE, the reducer which handles their slice of the top-level store state,
In a perfect world, that Instead, we typically define / export the relevant types in our import type { Action } from "redux";
import type { ThunkDispatch } from "redux-thunk";
import { useSelector, useDispatch, type TypedUseSelectorHook } from "react-redux";
import type { SubappStateType } from "path/to/subappStateTypeHelper";
import type { SomeSubappSliceType } from "./reducer";
export type SubappStateType = SubappStateType<{
someSubapp: SomeSubappSliceType;
}>;
export type SubappDispatchType = ThunkDispatch<
SubappStateType,
unknown,
Action
>;
export const useSomeSubappSelector: TypedUseSelectorHook<SubappStateType> =
useSelector;
export const useSomeSubappDispatch: () => SubappDispatchType =
useDispatch;
function getSlice(state: SubappStateType) {
return state.someSubapp;
}
// various selector functions follow In this case, These samples all come from our seller app -- our buyer-facing Islands code does similar stuff, though with abstractions in some different places. |
As I mentioned above, we've got home-grown utilities that mostly-solve a lot of these problems, but it's not hard to see how we could replace what we've got with what you're looking at. Our solutions have a number of limitations, such as that all "shared" components have to add new top-level state fields. It seems like your solution would remove that limitation and make a number of things more flexible, with a style that feels like hacked-on and more organized. |
Hi! This API sounds like it could be really useful and could probably even handle the "dynamic modules" case that Mark was mentioning. I've definitely been in situations where I had a complicated micro-frontend / sub-app / widget that used redux, but in order to render multiple different versions of it and have nice DX, I opted to give each sub-app its own redux store & provider. Being able to easily inject / clean-up individual slices could enable that kind of architecture to tap directly into the app-wide store and respond to app-level events more easily. Reading this got my gears turning, so I threw together a proof-of-concept, trying to follow Lenz's proposed API closely. I didn't implement any of the selector stuff though -- just the loading / unloading part. It looks like this could make lazy loading & dynamic modules much easier to implement. https://codesandbox.io/s/gifted-galois-9er3id?file=/app/combineSlices/combineSlices.ts |
By the way, for anyone subscribed here: @EskiMojo14 is currently working on this over in #3297 |
WOOOOO just saw this pop up in 2.0.0-alpha.5! |
@mq2thez @agusterodin yeah! try it out and let us know what you think? |
It looks like we've got a few more steps worth of tech debt to eliminate before we can adopt something like this, but I can see a decently clear path. It seems like the |
it's worth noting one difference between |
@EskiMojo14 awesome, thank you for the clarification (and all of the hard work)! I'll add that to my notes. At the moment, I think our biggest blocker / thing I'll have to figure out is whether |
Going to close this since it's available in 2.0-beta.0, but we'd love more feedback on how it works out in practice! Very much open to tweaking the API design before any final release. |
I'm just writing this down so I get my head emptied out and see if what I'm thinking makes any sense
reducer.ts
the "naive" approach
fileC.ts
the next step (maybe in a later release?)
"integrated" approach - adding a
selectors
field tocreateSlice
fileD.ts
general interface
Open questions:
combineSlice
calls?The text was updated successfully, but these errors were encountered: