-
-
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
Adding thunks and query apis dynamically #3148
Comments
I'm not sure if this is a good idea for middleware beyond the listener middleware, as that could change the For reducers, there is a proposal by me: #2776 |
Thanks, that's important context! I'll read through the linked threads and then find the best place to continue this conversation. |
@dbartholomae : can you give an example of where you need to add middleware dynamically at runtime? What specific use cases are you trying to solve? |
Mainly for adding apis from code-split modules. In a bigger app, there are often multiple independent APIs that do not depend on each other for caching and benefit from multiple teams being able to use them independently (putting them in the same API e.g. requires to ensure different names for endpoints). If apis where implemented on top of listeners, I don't think this would be necessary and they could be added on top of listeners as well. For this specifically, I would see three alternatives:
Overall, I would imagine something along the following lines (strongly inspired by redux-dynamic-modules). Examples are obviously very contrived, but I tried to covers as many parts of the api as possible while keeping the code simple. // main App
export const store = configureStore({
reducer: mainReducer
// default middleware includes a new middleware to load modules
})
export type RootState = ReturnType<typeof mainReducer>;
export interface RootExtraServices {}
type Dispatch = ThunkDispatch<RootState, RootExtraServices, AnyAction>;
export const useDispatch = untypedUseDispatch<Dispatch>;
export function useSelector<Selected>(
selector: (state: RootState) => Selected,
equalityFn?: (left: Selected, right: Selected) => boolean,
) {
return untypedUseSelector(selector, equalityFn);
} // auth module
interface AuthExtraServices {
authSDK: AuthSDK
}
const authApi = createApi({
reducerPath: 'authApi',
baseQuery: fetchBaseQuery(),
endpoints: (builder) => {
login: builder.mutation({
queryFn: ({username, password}, _queryApi, extra) => {
const { authSDK } = extra as AuthExtraServices
return authSDK.login(username, password)
}
}),
logout: builder.mutation({
queryFn: (_arg, _queryApi, extra) => {
const { authSDK } = extra as AuthExtraServices
return authSDK.logout()
}
})
}
})
const authenticatedSlice = createSlice({
name: 'authenticated',
initialState: false,
extraReducers: (builder) => {
builder.addCase(authApi.endpoints.login.fulfilled, () => true)
.addCase(authApi.endpoints.logout.pending, () => false)
}
})
export const authModule = createModule({
name: 'auth',
apis: [authApi],
slices: [authenticatedSlice],
finalActions: [logout()],
// The next line would be neat, but I'm not sure if it is achievable
extra: { authSDK },
})
export const logout = authApi.endpoints.logout.initiate
type AuthState = ReturnValue<typeof authModule.reducer>
type Dispatch = ThunkDispatch<RootState & AuthState, RootExtraServices & AuthExtraServices, AnyAction>;
// Each module can build its own correctly typed helpers so that code in each module can be sure of what types are available
export const useDispatch = untypedUseDispatch<Dispatch>;
export function useSelector<Selected>(
selector: (state: RootState & AuthState) => Selected,
equalityFn?: (left: Selected, right: Selected) => boolean,
) {
return untypedUseSelector(selector, equalityFn);
} // StopWorkingReminder that depends on the auth module
const workingSlice = createSlice({
name: 'stopWorking',
initialState: 'working',
reducers: {
notifyPause() {
return 'notifying'
}
forcePause() {
return 'forcedPause'
}
}
})
const startNotification = stopWorkingModule.createAsyncThunk(
// Users will need to manually ensure uniqueness here
'stopWorking/startNotification',
(_, { dispatch }) => {
await waitFor(20 * 60_000)
dispatch(workingSlice.actions.notifyPause())
await waitFor(5 * 60_000)
dispatch(workingSlice.actions.forcePause())
}
)
const logoutOnPauseListener = {
actionCreator: workingSlice.actions.forcePause,
effect: (_, {dispatch}) => dispatch(logout()),
})
export const stopWorkingModule = createModule({
name: "stopWorking",
dependencies: [authModule],
slices: [workingSlice],
listeners: [logoutOnPauseListener],
initialActions: [startNotification()],
}) // A component like this could be used to actually load the modules, so this is not user land anymore, but implementation inside the library
function ModuleGate({modules, children}) {
const [loading, setLoading] = useState(true)
const dispatch = useDispatch()
useEffect(() => {
for (let module of modules) {
for (let dependency of module.dependencies) {
dispatch(dependency.loadModule());
}
dispatch(module.loadModule());
}
setLoaded(false)
return // imagine similar clean up logic here
}, [modules])
if (loading) return null
return children
} I've also thought about an interface that uses the builder pattern to For implementation, the loadModule action that is dispatched would contain reducers, api middlewares, etc., and these could then be centrally added by the module middleware to the store. Btw. happy to move the discussion into one of the existing issues if you feel like it fits better there. |
Hmm. I'm not sure what you mean by "if APIs were implemented on top of listeners". Those are two very different things, implementation-wise. The listener middleware can only listen and respond to actions after they've been handled, whereas the custom middleware for RTKQ does a lot more complex work internally, including behaviors that are literally not possible for the listener middleware in any way (such as intercepting a "probe" action synchronously and returning a result). I'm also not sure what you mean by "Augment enhanceEndpoints/injectEndpoints in a way that allows codesplitting", because those are the tools that RTKQ provides specifically for code splitting. In an ideal scenario, there would still only be one single For the record I'm open to discussing the larger "general-purpose store enhancement at runtime" case, but I first want to see how feasible it is to address your team's situation with the existing capabilities. |
Actually, as a separate question, have you looked at https://github.com/fostyfost/redux-eggs ? It's the most recent "dynamic Redux modules" lib I've seen posted, but I don't know if anyone's actually using it. |
I'm not familiar with the internal implementation of RTK Query. I interpreted phryneas first comment as "it should be possible to solve all relevant behaviour with listeners and without additional middleware", so I assumed that RTK query was part of that. If it isn't, then I would go with an implementation that allows to inject middleware to have more flexibility - though that's not an interface we might want to actually expose, at least initially.
It is one of the blockers, but there are others. The biggest one I think is not being able to have custom base queries, as they might have different urls (
I ran into it multiple times, latest last month, when I tried to convert an app with multiple APIs into one. Typical examples include domain objects with different definitions per domain, e.g. the analytics service might have a different understanding of what an "account" is from the user profile service or the invoicing service. A concrete tangible example from a project in the past where I used redux dynamic modules (before there was RTK), was an application that allowed lawyers to communicate with their clients. In the beginning, we had both lawyers and clients in the same frontend but with separate APIs. While the long-term solution was to split even the frontend into two, the migration path meant to first split the api code in the frontend. The apis obviously had a lot of similar endpoints with different data.
Makes a lot of sense - let's first explore the problem space before jumping into the solution space.
I haven't used it, but from the documentation, it suffers a similar problem as enhanceApi: It requires an explicit dependency on the think it injects into. The beauty of redux-dynamic-modules and the listener implementation is that modules don't need to know about the thing they are enhancing (except for needing to find a unique name), and the only thing that is needed from the existing store is the |
Okay, those do sound like legit use cases for adding multiple RTKQ API definitions, each with their own middleware. Can you clarify what you mean by " FWIW, my main thoughts here atm are:
|
I don't have an industry overview, but I would be surprised if most of devs are working on small-size projects. There are most likely less projects that need this kind of code splitting, but they most likely have more devs working on them, so depending on how you count, I would say that it could actually affect 80-90% of devs working with Redux. But I don't have numbers on this, and the Twitter poll you did indicated only ~35% of devs using code splitting. The reasons why I am pushing for considering this, are as follows:
Instead of using a native method like dispatching actions to set up modules, you need access to the actual store. So you need to explicitly have the store available, which you might not have in all the places where you have dispatch available. I don't think it is a really big thing, but it is different from the existing method used for listeners, and it makes some things a bit more cumbersome than what they would need to be.
I would offer to take the main work of building this and contributing it to the project with nice test coverage and documentation. I tend towards more and shorter files than what I've seen in this project. You can find a small example project of mine here. I am currently using RTK at work extensively enough to justify spending even a bit of work time on it. ConclusionOverall, I see three main questions:
I'll anyways put it on my list to experiment a bit with the API and maybe create an independent package, but I still believe that it would be much more valuable if it was part of the core. |
@dbartholomae : yeah, the notion of dynamic loading and code splitting has certainly been on my mind for 2.0 as a thing to look at. I think a good first step would be doing some sketching out of the problem space and use cases:
Not saying we need to answer every single question before any code can be written, but sketching out scope and having an idea of what we want to build would be key. In the run-up to publishing RTK 1.0, I wrote up a long "vision" statement laying out what I wanted RTK to do, and the constraints and things I didn't want to do: Ironically, if you look at the things I said "I don't want to handle these"... we've ended up doing some of them ("data fetching" -> RTKQ, "encapsulated modules / plugins / code splitting" -> we're talking about this now, "React-Redux-specific functionality" -> RTKQ hooks). But saying no to those early on helped keep RTK's scope focused on what was critical for getting 1.0 out the door, and then we were able to build more features from there over time. So. If you are willing to take point on driving most of the initial work for this, I'm open to providing oversight and guidance. I know I had added a bunch of relevant libs to my "Redux Ecosystem Links" repo before I stopped updating it in 2018, and it looks like there's more libs since then that I haven't even seen before:
There's the three-ish major "Redux dynamic module loading" libs I can think of:
and then some of the major "Redux abstraction layer" libraries might have some stuff like this: and a couple articles:
and then you've got the non-Redux libs:
Heh. Okay, yeah, so just reviewing some of those, it does seem like this is a reasonably common thing people want to do with Redux :) Think you could start by filling out some answers to most of those questions, including trying to collate writeups on the ecosystem "prior art" reviews? Tweeted a link at https://twitter.com/acemarke/status/1622040111440338945 asking for more feedback. |
While I've thought about this for 5+ years, it's been a while since it's last been top of mind so I don't think I have that much to add, but I'll note one thing. The big problem I've always seen is where do you inject things? With client only this is simple (module scope), but with SSR it's trickier. I've done a writeup about this problem in this discussion (I added a new comment today) that also links to some older RFCs I wrote (first one 2018 if you want to read some ancient history). I have never considered the more advanced cases much though, like module/middleware injection and the like, since I'm not sure it belongs in core, but RTK Toolkit is different and it might very well belong there. 😃 I've always argued that it would be very nice to have better support for at least the simpler cases like reducer code splitting in core, I've seen first hand the performance impact of not code splitting reducers in even medium sized projects and I think the reason people don't do it (according to that Twitter poll for example) is that it's tricky and not built in. So I'm very happy more people are thinking about this! 😄 |
Sure, happy to do so! There are a few questions I would like to answer first, though: Should this be in RTK?What are roughly the questions that you need answered before being open to see this as part of RTK? The long list above already contains a lot of questions on the how, and there is certainly an influence (if we can't do it to a certain standard, we shouldn't include a half-baked solution), but let's not get preoccupied with whether we could and stop to think if we should just yet. For me, this comes down to the three questions from my last post, but I would like to make sure that you see it the same that answering these questions is enough to start trying to solve it as part of RTK. 1. Is dynamic modules a topic that is as needed by others as I perceive it to be, or is it less important?If there is uncertainty here, we should do more user research/outreach first. I'm happy to spread the question around a bit, but the amount of existing libraries maintained by others, we might already be convinced enough and able to skip to the next question. 2. Do you agree that this is a problem that should be solved centrally as an opinionated standard?3. Is there enough resources available to maintain an additional module like this inside redux toolkit?For both questions I made my argument and feel like it convinced you, but wanted to explicitly ask. How to run the RFC process?I don't see an existing process, but might be overlooking something. I would advise to use some kind of central document that we can comment, be it Google Doc, a PR with a markdown file against this repo, or something else. But having some kind of version control and line-by-line commentability in my experience makes the process easier. Otherwise, everyone who has input will need to read through all of the comments in this thread first, which will grow over time. If we choose a central document, I would then also share the document for feedback to all of the maintainers for existing solutions to dynamic redux modules, which might help us to avoid some of the problems they might already have stumbled onto. |
One thing to add: It would be possible to rewrite the RTK Query middleware to a point where you just Maybe that would be worth exploring, as it could already remove a lot of friction - and adding integration to inject RTKQ slices via #2776 should be perfectly possible. That would mean we had
From there, we can take a look if there is still a strong need to inject other middleware. Imho, that would split up into two categories of middlewares:
|
@dbartholomae : at this point I'm operating under the assumption this is a thing worth at least exploring, and probably adding something around dynamic loading to RTK. The list of questions above is meant to be a guide for us to figure out what we want to build. For content, maybe a shared Notion doc? Might have more flexibility in terms of what content gets added and written that way. And we can add a repo discussion thread that links that. |
I think I would not go with Notion since it requires creating an account to be able to comment, and this might limit feedback from users. I saw that Ephem created a PR with a markdown document back then, and I would propose to use the same here. |
There's a whole lot of text in this thread; apologies for not reading everything. I know @markerikson tagged me because I built this for Etsy and have previously mentioned that it is very-custom and I'd love to migrate to an RTK-managed version. @phryneas -- the system you're talking about where there's already a However, we've also adopted the same pattern for listener middleware -- during store setup, we inject a singleton middleware in every store, and each "subapp" interacts with that to load the listeners it needs. For our subapps, we have a
The idea is that anything that wants to render a particular subapp can dynamically import the correct We have a small number of subapps that need multiple top-level reducers (both for legacy reasons and for code-sharing reasons); for that, the
This ensures that any subapp which requires a shared piece of state can register it and its needs easily in a way that doesn't require an extra set of async steps (IE, we don't want to wind up with component trees that are 6 dynamic imports deep somewhere down the line). In terms of our needs / things which have been hard for us:
Things our subapps currently need to share/derive/inject:
|
This is a mile-long thread, and I don't feel like re-reading the whole thing right now: But we did just ship a |
Already scoping out the work to upgrade and then migrate over to it! Thank you so much. |
Hi there!
When writing big applications with redux, especially with multiple teams working in the same frontend on different domains, it can become complicated to colocate module code, as redux currently strongly suggests a pattern where middleware and reducers are added to the store centrally when creating the store. This also causes trouble when code-splitting as well as potentially performance degredation due to reducers that are not needed for the currently active part of the app.
There even is a solution for this from the past, redux-dynamic-modules, a Microsoft package that allows to bundle reducers, sagas, thunk,... into a module and load that module at runtime.
Unfortunately, redux-dynamic-modules uses its own
createStore
function, which makes it non-obvious to use with redux toolkit. At the same time, redux toolkit already partially supports this dynamic behavior: Listeners can be added and removed at runtime.Since redux-toolkit is supposed to be opinionated, there is an opportunity here: If we extend the pattern from listeners to other redux concepts, we could define a pattern that allows other library maintainers to follow.
What I'm proposing is to add a module middleware that listenes for certain actions, similar to the ones defined by listeners, and uses them to adjust reducers and middleware on runtime, as well as a module format that makes it easy to bundle up modules and add them to a store on runtime.
If there is interest in this, I'm happy to work on this and provide PRs, or start with an implementation external to redux-toolkit. But I do believe that in this case, an implementation outside the core will be less impactful for the community, as it is mostly about setting standards that other libraries can abide by.
The text was updated successfully, but these errors were encountered: