reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.64k stars 1.15k forks source link

[Feature] CRUD API Wrapper Implementation #603

Closed eslachance closed 3 years ago

eslachance commented 4 years ago

I've been working on a fresh new project and this gave me the benefit of being able to not only use redux-toolkit, but also the freedom to write a "builder" for our multiple CRUD endpoints.

After speaking with @markerikson , it became clear this might be something that's useful to a growing number of people. The basics of this is that the API endpoints we have are all similar in structure, and since CRUD operations usually have the same shape, I quickly ended up with a lot of duplicated code for each endpoint, where the only difference was the endpoint name.

So I made a function that creates all of the thunks using createAsyncThunk and the customReducer entries to make them work. This is then returned and used in configureStore or in the various components that require the data.

It's a little verbose as one might expect, but it works:

import { createSlice, createAsyncThunk } from '@reduxjs/toolkit';

export default ({
    baseUrl,
    name,
}) => {
    const fetchById = createAsyncThunk(
        `${name}/fetchByIdStatus`,
        id => fetch(`${baseUrl}/${id}`).then(r => r.json()),
    );

    const fetchAll = createAsyncThunk(
        `${name}/fetchAllStatus`,
        () =>  fetch(`${baseUrl}/`).then(r => r.json()),
    );

    const updateById = createAsyncThunk(
        `${name}/updateByIdStatus`,
        async ({id, data}) => {
            await fetch(`${baseUrl}/${id}`, {
                method: "UPDATE",
                headers: {
                    'Content-Type': 'application/json',
                },
                body: JSON.stringify(data),
            }).then(r => r.json());
            return data;
        },
    );

    const deleteById = createAsyncThunk(
        `${name}/deleteByIdStatus`,
        id =>  fetch(`${baseUrl}/${id}`, {
            method: 'DELETE',
        }).then(r => r.json()).then(() => id),
    );

    const createNew = createAsyncThunk(
        `${name}/createNewStatus`,
        data => fetch(`${baseUrl}`, {
            method: 'PUT',
            headers: {
                'Content-Type': 'application/json',
            },
            body: JSON.stringify(data),
        }),
    );

    const slice = createSlice({
        name,
        initialState: { entities: {}, loading: 'idle'},
        reducers: {},
        extraReducers: {
            [fetchById.fulfilled]: (state, action) => {
                state.entities[action.payload.id] = action.payload;
            },
            [fetchById.rejected]: (state, action) => {
                state.entities[action.payload.id] = action.payload;
            },
            [updateById.fulfilled]: (state, action) => {
                state.entities[action.payload.id] = action.payload;
            },
            [deleteById.fulfilled]: (state, action) => {
                delete state.entities[action.payload.id];
                return state;
            },
            [createNew.fulfilled]: (state, action) => {
                state.entities[action.payload.id] = action.payload;
            },
            [fetchAll.fulfilled]: (state, action) => {
                state.entities = {
                    ...state.entities,
                    ...action.payload,
                };
            },
        },
    });

    return {
        reducer: slice.reducer,
        fetchById,
        fetchAll,
        updateById,
        deleteById,
        createNew,
    };
};

This is called simply by providing the baseUrl and the name (in our case the could be different so that's why we had to split those 2 arguments):

export const cascades = builder({
    baseUrl: `${baseUrl}/cascade-blocks`,
    name: 'cascades',
});

export const groups = builder({
    baseUrl: `${baseUrl}/groups`,
    name: 'groups',
});

And then I imported those into our configureStore, combining them as a root reducer:

import { cascades, groups } from './slices';
const rootReducer = combineReducers( {
  cascades: cascades.reducer,
  groups: groups.reducer,
} );

export default store = configureStore({ reducer: rootReducer });

The only thing missing from the above is my next step, which is to provide some selector functions that can getById, getAll, getIDs, and other useful related things.

After adding the selectors I'll consider this to be a fairly self-contained, re-usable module that I'm sure we'll start using internally. Hopefully, it can be of service for Redux-Toolkit also!

dalechyn commented 4 years ago

Hey!

That definitely looks cool and useful, but what if a developer wants to have other routes URL or he handles them differently? I think that having routes property would make your idea richer, so it's like:

type CRUDReducerBuilderProps = {
  baseUrl: string
  name: string,
  routes?: {
    fetchById?: string
    fetchAll?: string
    // and so on
  }

And then fetch those routes if they are given.

Although I think CRUD operations may be very different, as you can talk not via HTTP but via other protocols (i.e. Redis), so maybe people won't use it and it wouldn't worth package space.

markerikson commented 4 years ago

Yep, thanks for filing this issue and putting that example together. This is a topic that's been on my mind for a while, and is an obvious next step after releasing createAsyncThunk and createEntityAdapter individually.

First, a couple suggestions on the code.

The reducers, as currently written, can all be replaced by use of createEntityAdapter. The example can be simplified down to:

const adapter = createEntityAdapter();

const slice = createSlice({
  name,
  initialState: adapter.getInitialState(),
  reducers: {},
  extraReducers: {
    [fetchById.fulfilled]: adapter.upsertOne,
    [fetchById.rejected]: adapter.upsertOne,
    [updateById.fulfilled]: adapter.upsertOne,
    [deleteById.fulfilled]: adapter.removeOne,
    [createNew.fulfilled]: adapter.upsertOne,
    [fetchAll.fulfilled]: adapter.upsertMany
  }
})

That said, I'm actually confused a bit as to why you're inserting an item for the fetchById.rejected case. Typo, or intentional?

Moving on and addressing the concept in general:

The biggest complications for a generic fetching + loading API like this come from the numerous various in how things could get fetched and loaded. For example, how would we want to represent loading state? For the entire slice, or on a per-item / request basis? How do we allow the user to specify the overall endpoint? Do we do the actual fetch() call ourselves, or still try to delegate and allow the user to make the call using whatever API lib they're using?

There's lots of existing other libs out there in the "data fetching wrapper" space. To point to some immediately relevant examples:

markerikson commented 4 years ago

@h0tw4t3r RTK aims to solve the most common 80% use cases, so I'm not at all worried about trying to handle anything that's not HTTP.

eslachance commented 4 years ago

Thank you Mark, I've updated my own code to use the Adapter, it's definitely cleaner that way. I was trying to get an idea of how to work the flow when something gets rejected, so yeah the upsert doesn't belong there at all, that's leftover code I hadn't handled correctly yet. There's definitely a lot more that could be done to this so it's generic enough to help anyone, but I feel it's a decent foundation to start building upon.

markerikson commented 4 years ago

So since this issue is open, I'd like to refocus it a bit to discuss the broader ideas of:

vs just the specific code snippets listed so far.

panigrah commented 4 years ago

@eslachance are you okay to share the code with the adapter please? I like this pattern and am trying to avoid duplicating code like you. I am trying to use your pattern but with the following changes:

But not sure how to work with the variable types. My code looks something like the below. Struggling with the types required for typescript and what changes I can make to the below to support a loading state per slice + a way to cancel the request if needed

@markerikson - in this example I am using loading state per entity slice type. Although my app also requires loading state per entity in a few places which I am currently handling separately. Prior to coming across this post - I had been just copying the same code multiple times for each entity.

export default ({
    baseUrl,
    name,
    initialState
}) => {

    type EntityStateType = typeof initialState & {
      loading: string,
      error?: any,
     currentRequestId?: string
    })

    const fetchById = createAsyncThunk(
        `${name}/fetchByIdStatus`,
        async ( { id }, { getState, requestId }) => {
            const { currentRequestId, loading } = getState()[name]
            if( loading !== 'pending'  || requestId !== currentRequestId ) {
              return
            }
            return axios.get(`${baseUrl}/${id}`) 
        }
    )

    const fetchAll = createAsyncThunk(
        `${name}/fetchAllStatus`,
        async( { page = 0, filters }, { getState, requestId }) => {
            const { currentRequestId, loading } = getState()[name]
            if( loading !== 'pending'  || requestId !== currentRequestId ) {
              return
            }
            return axios.get(`${baseUrl}`, { params: { query: { page, filters }}}) 
        }
    )

    const updateById = createAsyncThunk(
        `${name}/updateByIdStatus`,
        async () => {
        }
    )

    const deleteById = createAsyncThunk(
        `${name}/deleteByIdStatus`,
        async () => {
        }
    )

    const createNew = createAsyncThunk(
        `${name}/createNewStatus`,
        data => {}
    )

    const entityAdapter = createEntityAdapter({
        ...initialState,
        loading: 'idle',
        currentRequestId: undefined,
        error: undefined
    })

    const slice = createSlice({
        name,
        initialState: { entities: {}, loading: 'idle'},
        reducers: {},
        extraReducers: builder => {

            const pendingActionHandler = (state:EntityStateType, action:any) => {
                if( state.loading === 'idle') {
                    state.loading = 'pending'
                    state.error = undefined
                    state.currentRequestId = action.meta.requestId
                }
            }

            const rejectActionHandler = (state:EntityStateType, action:any) => {
                if (state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
                    state.loading = 'idle'
                    state.currentRequestId = undefined
                    state.error = action.error
                }
            }

            builder
            .addCase(fetchById.fulfilled,  (state:EntityStateType, action:any) => {
                if(state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
                    state.loading = 'idle'
                    state.currentRequestId = undefined
                    entityAdapter.upsertOne(state, action.payload.data)
                }
            })
            .addCase(fetchById.rejected, rejectActionHandler)
            .addCase(fetchById.pending, pendingActionHandler)
            .addCase(fetchAll.fulfilled,  (state:EntityStateType, action:any) => {
                if(state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
                    state.loading = 'idle'
                    state.currentRequestId = undefined
                    entityAdapter.setAll(state, action.payload.data)
                }
            })
            .addCase(fetchAll.rejected, rejectActionHandler)
            .addCase(fetchAll.pending, pendingActionHandler)
        },
    })

    return {
        reducer: slice.reducer,
        fetchById,
        fetchAll,
        updateById,
        deleteById,
        createNew,
        entityAdapter
    }
}
dalechyn commented 4 years ago

How something like Suspense might fit in

@markerikson Not a very long time I created a small fetching library for Suspense.

Basically it is a little helper for building an object with read() function for Suspense.

https://github.com/h0tw4t3r/react-redux-fetcher/blob/487332255dce77789068df059235e0c579f91af2/src/actions.js#L3-L32

Internally, dispatched actions look somewhat similar to AsyncAction actions created by async thunk.

Two main tasks that I see here are:

  1. We need to add fetching reducer, so should it be written in the library, or let users write it themselves? If first, how should we call it?
  2. Should there be a separate function or should it be a result of createAsyncThunk with .read() property?
eslachance commented 4 years ago

Here are some thoughts I have on implementation and usage.

The use case, for me, should be a relatively standard CRUD interface. If it can work out of the box with https://jsonplaceholder.typicode.com/ for example, that would make prototyping things faster. Would people build APIs around this? Yes, I think they might, so good care should be placed in the contract that we're establishing here. Perhaps even making an example OpenAPI/Swagger file might be useful to drive some standardized pattern adoption.

I'd love to see Suspense built-in, that links back to supporting query statuses I guess.

I find that there's a super common pattern when dealing with APIs, which is essentially useEffect + fetch + loading status + error handling, and all this could be handled by the toolkit with something like a hook?

eslachance commented 4 years ago

@panigrah you (and anyone else) are free to use this code any way you see fit. I'm releasing it as public domain, as far as I'm concerned I just put patterns together to push the cause slightly forward so please, feel free to whatever! <3

mbullington commented 4 years ago

So since this issue is open, I'd like to refocus it a bit to discuss the broader ideas of:

  • What some kind of an "RTK data fetching abstraction" API might look like
  • How we can take inspiration from the libs I listed above
  • What kind of constraints and use cases we would need to deal with
  • How something like Suspense might fit in

vs just the specific code snippets listed so far.

Not sure if I have enough domain knowledge in Redux to contribute much, but we recently started a React Native application using RTK and ended up with a similar pattern (focusing on the R in CRUD for now).

We have a helper at the slice level that has the following state structure, and creates a fetch thunk that will update these states accordingly:

interface AsyncData<T> {
    loading: boolean
    data: T | null | undefined
    error: any
}

fetch thunk can take up to one parameter and is protocol-agnostic, each slice is required to provide its own implementation. Right now they all use REST, but in the future I feel these options should be left open-ended.

There is also a mergePolicy function which allows data to be merged together, in cases of (for example) an infinite loading list or hashmap.

Combined with redux-persist and a tiny React hook, it feels somewhat similar to vercel/swr but in Redux-land.

Problems I can see with our approach:

  1. If a generic URL-response cache is tried with the above approach, loading and error would be very non-descriptive. Maybe having loading & error from AsyncData at the entity-level is better somehow?
Gilbert1391 commented 4 years ago

@eslachance are you okay to share the code with the adapter please? I like this pattern and am trying to avoid duplicating code like you. I am trying to use your pattern but with the following changes:

  • include a error and loading state at the entity level
  • use axios instead of fetch
  • typescript

But not sure how to work with the variable types. My code looks something like the below. Struggling with the types required for typescript and what changes I can make to the below to support a loading state per slice + a way to cancel the request if needed

@markerikson - in this example I am using loading state per entity slice type. Although my app also requires loading state per entity in a few places which I am currently handling separately. Prior to coming across this post - I had been just copying the same code multiple times for each entity.

export default ({
    baseUrl,
    name,
    initialState
}) => {

    type EntityStateType = typeof initialState & {
      loading: string,
      error?: any,
     currentRequestId?: string
    })

    const fetchById = createAsyncThunk(
        `${name}/fetchByIdStatus`,
        async ( { id }, { getState, requestId }) => {
            const { currentRequestId, loading } = getState()[name]
            if( loading !== 'pending'  || requestId !== currentRequestId ) {
              return
            }
            return axios.get(`${baseUrl}/${id}`) 
        }
    )

    const fetchAll = createAsyncThunk(
        `${name}/fetchAllStatus`,
        async( { page = 0, filters }, { getState, requestId }) => {
            const { currentRequestId, loading } = getState()[name]
            if( loading !== 'pending'  || requestId !== currentRequestId ) {
              return
            }
            return axios.get(`${baseUrl}`, { params: { query: { page, filters }}}) 
        }
    )

    const updateById = createAsyncThunk(
        `${name}/updateByIdStatus`,
        async () => {
        }
    )

    const deleteById = createAsyncThunk(
        `${name}/deleteByIdStatus`,
        async () => {
        }
    )

    const createNew = createAsyncThunk(
        `${name}/createNewStatus`,
        data => {}
    )

    const entityAdapter = createEntityAdapter({
        ...initialState,
        loading: 'idle',
        currentRequestId: undefined,
        error: undefined
    })

    const slice = createSlice({
        name,
        initialState: { entities: {}, loading: 'idle'},
        reducers: {},
        extraReducers: builder => {

            const pendingActionHandler = (state:EntityStateType, action:any) => {
                if( state.loading === 'idle') {
                    state.loading = 'pending'
                    state.error = undefined
                    state.currentRequestId = action.meta.requestId
                }
            }

            const rejectActionHandler = (state:EntityStateType, action:any) => {
                if (state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
                    state.loading = 'idle'
                    state.currentRequestId = undefined
                    state.error = action.error
                }
            }

            builder
            .addCase(fetchById.fulfilled,  (state:EntityStateType, action:any) => {
                if(state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
                    state.loading = 'idle'
                    state.currentRequestId = undefined
                    entityAdapter.upsertOne(state, action.payload.data)
                }
            })
            .addCase(fetchById.rejected, rejectActionHandler)
            .addCase(fetchById.pending, pendingActionHandler)
            .addCase(fetchAll.fulfilled,  (state:EntityStateType, action:any) => {
                if(state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
                    state.loading = 'idle'
                    state.currentRequestId = undefined
                    entityAdapter.setAll(state, action.payload.data)
                }
            })
            .addCase(fetchAll.rejected, rejectActionHandler)
            .addCase(fetchAll.pending, pendingActionHandler)
        },
    })

    return {
        reducer: slice.reducer,
        fetchById,
        fetchAll,
        updateById,
        deleteById,
        createNew,
        entityAdapter
    }
}

Hi, I am looking exactly for something like this, I tried your code but it's not working as expected. What are you passing as the initialState parameter in the builder function? i.e:

export const signIn = builder({
  baseUrl: 'https://api.deslindate.com/deslindate' + endpoints.auth.signIn,
  name: 'auth',
  initialState: { loading: '', error: false, currentRequestId: '' },
});

I keep getting the rejected type with the following error: Cannot read property 'currentRequestId' of undefined at

this is the post method:

const post = createAsyncThunk(`${name}/postStatus`, (data, { getState, requestId }) => {
    const { currentRequestId, loading } = getState()[name];
    console.log('state', getState()[name]);
    console.log('cur', currentRequestId);
    if (loading !== 'pending' || requestId !== currentRequestId) {
      return;
    }
    return axios.post(baseUrl, data);
  });

How can I make this work?

markerikson commented 4 years ago

@Gilbert1391 that getState()[name] call will only work if your reducers also happen to be storing relevant data in the store in a field matching what's in the name variable. In other words, it's up to you to write the reducer code that makes that getState() line work.

Gilbert1391 commented 4 years ago

@Gilbert1391 that getState()[name] call will only work if your reducers also happen to be storing relevant data in the store in a field matching what's in the name variable. In other words, it's up to you to write the reducer code that makes that getState() line work.

I'm not sure what you mean, I'm new to RTK so please bear with me. I followed @eslachance approach with @panigrah code.

So I have the builder file which contains the code from @panigrah , then I have a slice called authSlice which contains the following function:

export const signIn = builder({
  baseUrl: 'https://api.deslindate.com/deslindate' + endpoints.auth.signIn,
  name: 'auth',
});

The builder takes in a third parameter, I think here is where I'm at loss, I am not sure exactly what to pass here, I tried passing an empty object and some other things but no luck so far.

Then in my reducer file:

import { combineReducers } from 'redux';
import uiSlice from './uiSlice';
import { signIn } from './authSlice';

export default combineReducers({
  ui: uiSlice,
  signIn: signIn.reducer,
});

Finally, i dispatch the action in my component: dispatch(signIn.post(user));

What am i missing? I'd really appreciate some guidance here.

markerikson commented 4 years ago

You're missing a state.auth in there :) In other words, you need something like:

export default combineReducers({
  ui: uiReducer,
  signIn: signInReducer,
  auth: authReducer
})

or something along those lines.

The key point here is that the builder function is taking a name string, and then trying to look up getState()[name]. So, whatever 'someName' is being passed into the builder, must also exist as state.someName, as defined in your root reducer setup.

Gilbert1391 commented 4 years ago

You're missing a state.auth in there :) In other words, you need something like:

export default combineReducers({
  ui: uiReducer,
  signIn: signInReducer,
  auth: authReducer
})

or something along those lines.

The key point here is that the builder function is taking a name string, and then trying to look up getState()[name]. So, whatever 'someName' is being passed into the builder, must also exist as state.someName, as defined in your root reducer setup.

As we were talking I tried this and it finally returned the fulfilled action type

export default combineReducers({
  ui: uiSlice,
  auth: signIn.reducer,
});

However it's not returning any payload, I'm still at loss at what to pass as the third arg for the builder function. edit:

I had these lines commented:

.addCase(post.pending, pendingActionHandler)
        .addCase(post.fulfilled, (state: EntityStateType, action: any) => {
          if (state.loading === 'pending' && state.currentRequestId === action.meta.requestId) {
            state.loading = 'idle';
            state.currentRequestId = undefined;
            entityAdapter.upsertOne(state, action.payload.data); 
          }
        })
        .addCase(post.rejected, rejectActionHandler);

I uncommented them and now I got an error: Unhandled Rejection (TypeError): Cannot read property 'push' of undefined

I think it might be because I am not passing the third parameter

markerikson commented 4 years ago

@Gilbert1391 this issue thread is really meant for discussion, not for providing support. I'd suggest going over to the #redux channel in the Reactiflux Discord and asking there.

Tbh, it sounds like the bigger issue is that you may not yet be comfortable using Redux itself yet. I'd strongly recommend reading through the new "Redux Essentials" core docs tutorial at https://redux.js.org/tutorials/essentials/part-1-overview-concepts before you go any further. This "builder/CRUD API" is a slightly more advanced usage of the Redux Toolkit APIs, and you might be jumping a bit too far ahead here based on the questions you're asking.

eslachance commented 4 years ago

I've updated my version, might help some people. Notable additions are the use of getSelectors for the selectors, and simplifying the extraReducers (thanks to Mark for those suggestions!). Also, I added a fetchByQuery to add query param filtering to results, and optional authentication header. There's probably a better way to do the auth though.

import {
  createSlice,
  createAsyncThunk,
  createEntityAdapter
} from "@reduxjs/toolkit";

export default ({ baseUrl, name, authCode }) => {
  const fetchWithAuth = (url, { headers = {}, ...options } = {}) =>
    fetch(url, {
      ...options,
      headers: {
        ...(authCode && { Authorization: authCode }),
        ...headers
      }
    });

  const handleResponse = async r => {
    const resdata = await r.json();
    if (r.status !== 200) {
      throw new Error(resdata);
    }
    return resdata;
  };

  const fetchByQuery = createAsyncThunk(`${name}/fetchByQueryStatus`, (params) => {
    const querystring = new URLSearchParams(params);
    console.log(querystring);
    fetchWithAuth(`${baseUrl}?${querystring}`).then(handleResponse)
  });

  const fetchById = createAsyncThunk(`${name}/fetchByIdStatus`, id =>
    fetchWithAuth(`${baseUrl}/${id}`).then(handleResponse)
  );

  const fetchAll = createAsyncThunk(`${name}/fetchAllStatus`, () =>
    fetchWithAuth(`${baseUrl}`).then(handleResponse)
  );

  const updateById = createAsyncThunk(
    `${name}/updateByIdStatus`,
    ({ id, data }) =>
      fetchWithAuth(`${baseUrl}/${id}`, {
        method: "PATCH",
        headers: {
          "Content-Type": "application/json"
        },
        body: JSON.stringify(data)
      })
        .then(handleResponse)
        .then(() => data)
  );

  const deleteById = createAsyncThunk(`${name}/deleteByIdStatus`, id =>
    fetchWithAuth(`${baseUrl}/${id}`, {
      method: "DELETE"
    })
      .then(handleResponse)
      .then(() => id)
  );

  const createNew = createAsyncThunk(`${name}/createNewStatus`, data =>
    fetchWithAuth(`${baseUrl}`, {
      method: "POST",
      headers: {
        "Content-Type": "application/json"
      },
      body: JSON.stringify(data)
    })
      .then(handleResponse)
      .then(r => ({
        id: r.id,
        ...data
      }))
  );

  const adapter = createEntityAdapter();

  const slice = createSlice({
    name,
    initialState: adapter.getInitialState({
      loading: "idle"
    }),
    reducers: {},
    extraReducers: {
      [fetchById.fulfilled]: adapter.upsertOne,
      [updateById.fulfilled]: adapter.upsertOne,
      [deleteById.fulfilled]: adapter.removeOne,
      [createNew.fulfilled]: adapter.upsertOne,
      [fetchAll.fulfilled]: adapter.upsertMany,
      [fetchByQuery.fulfilled]: adapter.upsertMany,
    }
  });

  return {
    reducer: slice.reducer,
    ...adapter.getSelectors(state => state[name]),
    fetchById,
    fetchAll,
    fetchByQuery,
    updateById,
    deleteById,
    createNew
  };
};
markerikson commented 4 years ago

That example would also probably benefit from using the builder syntax for extraReducers, and adding some "matchers" for the various pending/fulfilled/rejected actions to update the loading state.

But yeah, that looks useful!

Gilbert1391 commented 4 years ago

I think for adding authentication to the header requests is better to use a middleware though. One thing to think about is that the adapter is not very useful for POST requests, because most responses from POST requests do not contain any ids properties, so the adapter can't create the look up table, result is saved as "undefined". Any thoughts on this?

eslachance commented 4 years ago

To be honest, the code was built for a specific purpose, so it's not exactly generalized. If your API isn't following the exact structure that the builder expect, it's not going to work. So, obviously this is meant as a starting point and not an end-all-be-all solution. A setup system or configuration options would be necessary to be able to use it on various back-ends. I posted this as a boilerplace/example for people to use, or possibly for someone a lot more competent than I am to integrate as an option inside of the toolkit itself.

markerikson commented 3 years ago

Here's an interesting lib based around JSON-API, which happens to have a neat-looking useAutoRequest hook:

https://github.com/rmarganti/json-api/tree/master/packages/jason-api

cjol commented 3 years ago

I've been using createAsyncThunk and createEntityAdapter together with some homebrewed status management and was thinking about abstracting it out into a createAsyncEntityAdapter myself. I came here to check whether that work has already been done by anyone so I'm glad it's at least being discussed!

In terms of tracking status per-slice or per-item, I initially went for tracking loading status at the item level, using selectors to aggregate these at the slice level. This slightly broke down when I wanted to track the status of operations like "fetch all" on page load, where there are no items yet in the state to attach the loading status to. I then evolved into a belt-and-bracers approach where I track status for both items and the slice as a whole - but this admittedly feels pretty heavy-handed. My state tree now looks something like this:

export enum LoadingStatus {
  INIT,
  PENDING,
  LOADED,
  ERROR,
}

export type LoadableEntity<
  Item,
  Error = string
> = { id: string } & (
    | {
        status: LoadingStatus.INIT | LoadingStatus.PENDING;
      }
    | {
        status: LoadingStatus.LOADED;
        item: Item;
      }
    | {
        status: LoadingStatus.ERROR;
        errors: Error[];
      }
  );

type State = {
   ids: string[];
   entities: Record<string, LoadableEntity<FooEntity>>
} & (
    | {
        status: LoadingStatus.INIT | LoadingStatus.PENDING  | LoadingStatus.LOADED;
      }
    | {
        status: LoadingStatus.ERROR;
        errors: Error[];
      }
)

The power here is all in the selectors, which is kind of nice because it's then up to the component developer to decide how precise they want to be. They can use a high-level getLoadedEntities which will just return FooEntity[] which is very easy to work with, or they can carefully discharge the conditionals to render a very precise interface.

A few other thoughts:

markerikson commented 3 years ago

@cjol : nice. Think you could share what you've implemented so far?

cjol commented 3 years ago

I'll put a gist together and share when I do!

cjol commented 3 years ago

I've pulled out the relevant code into a prototype createAsyncEntityAdapter here: https://gist.github.com/cjol/64e3cdceb642cb3b887b6c04f53abf21

N.B. I haven't actually included this version into a project, but the basic approach is very similar to what I'm currently doing.

markerikson commented 3 years ago

Someone just posted https://github.com/hubsharp-solutions/ruffle to /r/reactjs, which looks like it does some very similar AJAX/slice abstractions and actually already wraps around RTK's createSlice:

https://github.com/hubsharp-solutions/ruffle

bluedevil2k commented 3 years ago

Hi, I'm the author of the Ruffle library just posted here. I assume I've run into the same issue much of you have run into, trying to reduce the boilerplate code for an application that's using 95% REST calls. Reading through these comments, it looks like I took a different approach, separating out the API layer. However, I like the idea posted above about supporting lots of different API libraries. It's early in my project, I'm looking to build it out as I can. One thing I want to do, which my current project necessitates, is to automatically link the Store/Slices to websocket messages.

markerikson commented 3 years ago

More interesting examples: https://github.com/klis87/redux-requests , which looks like it's more of a middleware that can be configured to make requests based on dispatched actions, plus a bunch of reducer/selector logic on top.

klis87 commented 3 years ago

Hi, jumping to the discussion after invitation by @markerikson !

I am the author or redux-requests and Mark is right, I will just add that its main purpose is to allow to define all network related things (making requests, handling errors, side effects, aborts, normalisation, optimistic updates, caching, race conditions and more to come) with just Redux actions in declarative manner. You can forget about boring network stuff and focus just on the app development, having all remote state managed for you.

I will just mention that its killer features are:

Regarding this topic, personally I think that making CRUD helpers is not part of Toolkit library, from what I understood this was supposed to be a little Redux wrapper which simplifies Redux usage, I don't think this would be in scope.

I would focus on something else instead - make this library compatible will all Redux addons in a simple way. For instance, from what I can see you could use toolkit with redux-requests, the only issue is that using slices would be very inconvenient.

I will give you example based on redux-requests, but there are more libraries like that, for example redux-first-router. The problem with slices is that it always wants to create reducer for all ations, but this is not always needed. For example in redux-requests you could have an action like that:

const fetchBookDetail = id => ({
  type: 'FETCH_BOOK_DETAIL',
  request: { url: `/books/${id}` },
});

Dispatching it would make a request and save the response in a global reducer made by this library. But the point is that this reducer is global, handling all requests - not reducer per request. It is similar for redux-first-router.

Looking at the API I think I could make it work in such a way:

const booksSlice = createSlice({
  name: 'books',
  initialState: 'I dont care',
  reducers: {
    fetchBookDetail: {
      reducer: (state, action) => {
        return 'I dont care';
      },
      prepare: (id) => {
        return {
          payload: {
            request: { url: `/books/${id}` },
          },
        };
      },
    },
  },
})

I am pretty sure this would work, but see that this is very verbose. It would be cool to make initialState optional in slices (if we dont need reducers from it) and also to add new key, like actions, which would allow adding just actions without creating reducers.

I know I could just use createAction, but then I will have the possibility to risk type collisions and I would need to pass action types strings to it. @markerikson will know what I am talking about, we had discussion here . I created small helpers redux-smart-actions which thanks to Babel plugin allow to define actions like that:

import { createSmartAction } from 'redux-smart-actions';

const fetchBookDetail = createSmartAction(id => ({
  request: { url: `/books/${id}` },
}));

Compare it with createAction from the toolkit:

import { createSmartAction } from 'redux-smart-actions';

const fetchBookDetail = createAction('FETCH_BOOK_DETAIL', id => ({
  payload: {
    request: { url: `/books/${id}` },
  },
}));

I know, maybe not the biggest thing, but I don't like to pass the same name we already used for const name. Slices are great because they achieve similar thing without Babel plugin, however they suffer from this reducer problem I mentioned above.

To sum up, in my opinion toolkit should still be just a set of tools, simplifying Redux. Extending its API to cover CRUD operations etc will make its API grow and grow, possibly frightening new users. Toolkit API is already bigger than Redux itself ;)

However it should be possible to easily integrate it with the whole Redux ecosystem, just like pure Redux, generally it is, just apart from slices and the case mentioned before.

cjol commented 3 years ago

RTK slices represent a "slice" of the state tree. If you're not storing something in this part of the state tree, then you don't need a reducer and you don't need a slice. The expected flow there is just to use createAction as you later suggest. Adding a "magic" babel build step just to avoid duplication seems like the wrong tradeoff to me (especially when a similar API could be achieved using object keys without Babel).

To chime into the broader discussion around CRUD wrapper: I think there can exist a wrapper which is a natural extension of the existing createAsyncThunk and createEntityAdapter APIs, and which therefore wouldn't meaningfully increase the size of the RTK API.

jhonnymichel commented 3 years ago

What would be the best approach to implement cache invalidation and fetch policies in an asyncEntity feature in rtk?

I was wondering about this as I saw @markerikson linking to this issue on twitter.

using Apollo fetch policies as the reference (https://medium.com/@galen.corey/understanding-apollo-fetch-policies-705b5ad71980):

Let's say I setup an asyncEntity fetch action and I want to use the cache-first policy. where would be the best place to enforce the policy? if the data is in the cache (which is the store state in our context), the fetch should be skipped. Is it okay to simply getState() inside the thunk function and check if the data is present and is not stale?

or should it be a middleware that can interrupt the flow earlier?

and of course: Does fetch policies/cache invalidation strategies make sense here?

phryneas commented 3 years ago

Just to put this in here: There is experimentation on this going on in https://github.com/rtk-incubator/simple-query

Here's a Codesandbox showcasing some of it

jhonnymichel commented 3 years ago

I think that answers most of my questions. checking it out

markerikson commented 3 years ago

So FYI for folks who were following this thread: we ended up building an RTK-centric data fetching abstraction inspired by React Query and Apollo, dubbed "RTK Query":

https://rtk-query-docs.netlify.app

We're hoping to finalize work on that in the very near future, and then merge the new APIs back into RTK itself for an upcoming RTK 1.6 release:

https://github.com/rtk-incubator/rtk-query/issues/175

I think this effectively obsoletes the "CRUD Wrapper" discussions in this thread, but I'll leave this open until that release is published.

markerikson commented 3 years ago

And today we released RTK 1.6 with the new RTK Query data caching API!

https://github.com/reduxjs/redux-toolkit/releases/tag/v1.6.0