Skip to content

Suggestion: Define thunk on createSlice #1045

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

Closed
einarq opened this issue May 10, 2021 · 17 comments
Closed

Suggestion: Define thunk on createSlice #1045

einarq opened this issue May 10, 2021 · 17 comments
Labels
enhancement New feature or request TypeScript
Milestone

Comments

@einarq
Copy link

einarq commented May 10, 2021

I'm sure this has already been discussed elsewhere, but perhaps not with the following reasoning in mind, so please bear with me :)

One thing that I often come across is the desire to reuse a whole reducer across different parts of the state, so different slices would be the correct term I guess.
This can of course easily be done by creating a higher order reducer etc. What cannot so easily be done however is to reuse the async actions. You need to somehow point the action back to the different part of the state, and you also need to make sure any getState calls inside any async actions select from the correct part of the state.

Couldn't this perhaps somehow be baked into createSlice? Apologies for not having a fully fledged idea here, but I do think this could be a nice improvement to already excellent DX of RTK. Especially if those thunk actions could also somehow easily have the "local" state available, so that they don't need to know where in the global state they are located.

Some other things that would be improved by defining thunks via createSlice (which I'm sure you are of course alredy aware of) are:

  • Remove the need to define the name in a way like this:
    const addGroup = createAsyncThunk(${sliceName}/addGroup, async ({group}, {getState}) => {
    });
  • Automatically export all thunks as well via reducer.actions

Would be interested to hear your thoughts on this. And thanks again for the awesome library 👍

@markerikson
Copy link
Collaborator

We would desperately like to do this, and we even had a prototype. Unfortunately, it turned out to be basically impossible due to circular TS type definitions. Thunks require knowledge of RootState for getState, which is formed by looking at the slice reducer state type. But, if you try to define thunks in the slice, now the slice needs to know RootState, and you've got yourself a circular type definition.

See #637 for the failed PR attempt.

@phryneas
Copy link
Member

phryneas commented May 10, 2021

Adding to that, we hope that the general need for such "higher order slices", especially in combination with createAsyncThunk will be reduced quite a bit once RTK-Query is part of the next release of RTK.

@einarq
Copy link
Author

einarq commented May 10, 2021

Ok. A real shame that TS gets in the way of useful stuff like this though.

Thanks

@einarq
Copy link
Author

einarq commented May 10, 2021

Adding to that, we hope that the general need for such "higher order slices", especially in combination with createAsyncThunk will be reduced quite a bit once RTK-Query is part of the next release of RTK.

Right. Will have to start looking closer into at RTK Query at some point, but its quite a leap from basic createSlice usage I guess...

@phryneas
Copy link
Member

phryneas commented May 10, 2021

If those slices were used for "server data cache" purposes only, primarily deleting code ;)

@einarq
Copy link
Author

einarq commented May 10, 2021

It is used for displaying and updating a list of entities, so I guess so :)
Perhaps a recipe for converting a slice reducer like this to RTK query could be added to the docs when it is released?

@markerikson
Copy link
Collaborator

Probably worth showing a switch from standalone thunks + reducers to RTK Query, yeah. But as Lenz said - it's mostly "delete your existing code and just use the auto-generated hook" :)

@phryneas
Copy link
Member

phryneas commented May 10, 2021

It's literally

  • set up an api with createAPI
+export const pokemonApi = createApi({
+  reducerPath: 'pokemonApi',
+  baseQuery: fetchBaseQuery({ baseUrl: 'https://pokeapi.co/api/v2/' }),
+  endpoints: (builder) => ({
+	// endpoints here
+  }),
+});

export const store = configureStore({
  reducer: {
    // Add the generated reducer as a specific top-level slice
+   [pokemonApi.reducerPath]: pokemonApi.reducer,
  },
  // Adding the api middleware enables caching, invalidation, polling,
  // and other useful features of `rtk-query`.
+ middleware: (getDefaultMiddleware) => getDefaultMiddleware().concat(pokemonApi.middleware),
});
  • follow the RTK query docs to add an endpoint
  endpoints: (builder) => ({
-	// endpoints here
+    getPokemonByName: builder.query({
+      query: (name: string) => `pokemon/${name}`,
+    }),
  })
  • use the auto-generated hook
const { data, error, isLoading } = useGetPokemonByNameQuery('bulbasaur');
  • delete your slice

There is no real "migration" apart from "build from zero, delete" as they work differently and one just replaces the other.

@einarq
Copy link
Author

einarq commented May 10, 2021

Looks good.

This is probably described somewhere, but I assume that RTK Query allows you to plug in your own "fetcher" of sorts? We use a wrapper for native browser fetch, in order to pass some headers etc.

@phryneas
Copy link
Member

phryneas commented May 10, 2021

That is baseQuery with the fetchBaseQuery in this example. There are examples for usage with axios or graphql-request buried in the docs and/or examples.

@markerikson
Copy link
Collaborator

(and we will be adding a docs page specifically to cover using a customized base query)

@einarq
Copy link
Author

einarq commented May 10, 2021

Sounds good. Fingers crossed something like this might make it to plain RTK createSlice though. Forgot to mention that somehow involving selectors in the createSlice utility as well would be even better :)

Anyway, look forward to trying out RTK Query a bit more.

@markerikson
Copy link
Collaborator

Selectors would unfortunately suffer from the exact same typing issue. You need the RootState to type them correctly, but the RootState type is derived from the slice state type.

I believe Lenz said it's possible to break the cycle if you explicitly do export default todosSlice.reducer as Reducer<TodosState>, but that's not a pattern we've encouraged so far, and we're trying to stick with suggested patterns that "just work" as much as possible.

@phryneas
Copy link
Member

phryneas commented May 10, 2021

Isolated slice-specific selectors would be totally possible using a pattern like entityAdapter, actually I got that on my TODO list, but RTKQ is eating quite a lot time right now :D

Could potentially look like this:

const slice = createSlice({
  // ...,
  selectors: {
    foo: state => state.foo,
    bar: state => state.bar,
    fooBar: state => createSelector(slice.selectors.foo, slice.selectors.bar, (foo, bar) => foo + bar)
  }
})

export { selectFoo, selectBar, selectFooBar } = slice.bindSelectors(store => store.mySlice)

@einarq
Copy link
Author

einarq commented May 11, 2021

Isolated slice-specific selectors would be totally possible using a pattern like entityAdapter, actually I got that on my TODO list, but RTKQ is eating quite a lot time right now :D

Could potentially look like this:

const slice = createSlice({
  // ...,
  selectors: {
    foo: state => state.foo,
    bar: state => state.bar,
    fooBar: state => createSelector(slice.selectors.foo, slice.selectors.bar, (foo, bar) => foo + bar)
  }
})

export { selectFoo, selectBar, selectFooBar } = slice.bindSelectors(store => store.mySlice)

Would be awesome to have something like this built in. Looks very similar to what we are doing already in our own custom way, except the selectors are bound to the global store on the top-level instead (since the slice doesn't necessarily know or control where in the store it ends up).

@markerikson
Copy link
Collaborator

Definitely not happening any time soon, unfortunately :(

@markerikson
Copy link
Collaborator

This is now available as of 2.0-beta.0. Try it out and see what happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants