reduxjs / redux-toolkit

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

Can't use generic type with entityAdapter #663

Closed fanczy closed 3 years ago

fanczy commented 4 years ago

Hello, I'm trying to create factory for rest entity adapter, but I just cant get generic types through createEntityAdapter. Did someone stumble upon this and solve the problem? It says that type of state passed to booksAdapter.setAll is not compatible with type of state received by case reducer.

Here is snippet from documentation modified so it uses generic type.

type Book = { bookId: string; title: string }
const testFactory = <T extends Book = Book>() => {

    const booksAdapter = createEntityAdapter<T>({
      // Assume IDs are stored in a field other than `book.id`
      selectId: book => book.bookId,
      // Keep the "all IDs" array sorted based on book titles
      sortComparer: (a, b) => a.title.localeCompare(b.title)
    })

    const booksSlice = createSlice({
      name: 'books',
      initialState: booksAdapter.getInitialState(),
      reducers: {
        // Can pass adapter functions directly as case reducers.  Because we're passing this
        // as a value, `createSlice` will auto-generate the `bookAdded` action type / creator
        bookAdded: booksAdapter.addOne,
        booksReceived(state, action) {
          // Or, call them as "mutating" helpers in a case reducer
          booksAdapter.setAll(state, action.payload.books)
        }
      }
    })

    return booksSlice;
};
fanczy commented 4 years ago

Here I attach 106lines long TypescriptError error.txt

markerikson commented 4 years ago

At a minimum, you need to define the type of action in that reducer, like:

    booksReceived(state, action: PayloadAction<{books: Book[]}>) {
      booksAdapter.setAll(state, action.payload.books)
    }

Otherwise TS has no idea what is in action.

fanczy commented 4 years ago

That is not the problem, here's a piece of an actual code:

export interface RestEntityAdapterFactoryOptions<T> {
    name: string;
    selectId?: (entity: T) => EntityId;
}

interface RestEntityAdapterState<T> extends EntityState<T> {
    error?: string,
    loading: boolean
}

const restEntityAdapterFactory = <T, S>(
    options: RestEntityAdapterFactoryOptions<T>
) => {

    const { selectId } = options;
    const entityAdapter = createEntityAdapter<T>({
        selectId
    })

    const initialState: RestEntityAdapterState<T> = entityAdapter.getInitialState({
        loading: false,
    });

    const entitySlice = createSlice({
        name: options.name,
        initialState,
        reducers: {
            setAll: (state: RestEntityAdapterState<T>, action: PayloadAction<T[]>) => {
                entityAdapter.setAll(state, action.payload);
            }
        }
    });

    const restEntityAdapter: RestEntityAdapter<T, S> = {
        thunks: createAdapterThunkCreators(options)
    }

    return restEntityAdapter;
};
msutkowski commented 4 years ago

@fanczy I slightly modified your example and showed a basic functioning use case. The main cure being T extends any.

export interface RestEntityAdapterFactoryOptions<T> {
  name: string;
  selectId?: (entity: T) => string;
}

const restEntityAdapterFactory = <T extends any, S>(
  options: RestEntityAdapterFactoryOptions<T>
) => {
  const { selectId } = options;
  const entityAdapter = createEntityAdapter<T>({ selectId });

  type RestEntityAdapterState = ReturnType<
    typeof entityAdapter.getInitialState
  > & {
    loading: boolean;
    error: string;
  };
  const initialState: RestEntityAdapterState = entityAdapter.getInitialState({
    loading: false,
    error: ""
  });

  const entitySlice = createSlice({
    name: options.name,
    initialState,
    reducers: {
      setAll: (state, action: PayloadAction<T[]>) => {
        entityAdapter.setAll(state, action.payload);
      }
    }
  });

  // omitted due to not knowing implementation
  // const restEntityAdapter: RestEntityAdapter<T, S> = {
  //     thunks: createAdapterThunkCreators(options)
  // }

  return { reducer: entitySlice.reducer, actions: entitySlice.actions };
};

type Book = {
  id: number;
  title: string;
};
const { reducer: bookReducer, actions: bookActions } = restEntityAdapterFactory<
  Book,
  any
>({
  name: "books"
});

console.log(bookActions.setAll({ id: 3, party: "four" })); // type error, expects Book[]
fanczy commented 4 years ago

@msutkowski Thank you, I wonder why does T have to extend any? Anyway I wasted so much time it made me realize I was reinventing wheel and just went with React Query, seems promising.

fanczy commented 4 years ago

The idea behind this was to provide async methods to getAll, upsertOne, removeOne etc. to a factory and get reducer, selectors and thunks for data and crud operations on them, saving a ton of boilerplate.

msutkowski commented 4 years ago

Yea, @phryneas could give a much better explanation than me, but basically you were just passing a generic to a generic to a generic and TS was never able to resolve anything. Saying T extends any solves that, being that T doesn't actually matter in that code. There's another issue open about this generic CRUD factory (but not TS-focused), and an active convo in #redux on the Reactiflux discord.

I'd just point out that there's quite a difference between global state management and a network cache like react query, but you should use whatever fits your use case best either way 🎉 .

fanczy commented 4 years ago

Thanks, well I think React Query will be sufficient for my use case now. But I still might finish this as a contribution to open source community, I've made such thing in past, but it was js only and rest endpoint had to be prepared for that (for example return current state after modification...)

msutkowski commented 4 years ago

Sounds good! I'm going to look into this a little bit myself as well and see what a good pattern might be. I think this is going to create some circular imports issues, but not 100% yet. Will update this thread 👍

phryneas commented 3 years ago

I'm gonna close this one for now.