Allow the user to refine the Typescript type for the EntityId in createEntityAdapter

[lindapaiste]

Inspired by this StackOverflow question, I think it would be a good feature if it was possible for an EntityAdapter to have a generic id type which extends EntityId but could be just string or just number. This would allow the EntitySelectors to return the correct id type from selectIds and require the right id type for selectById.

Most of the types in models.ts would need a second generic type parameter which is optional like <I extends EntityId = EntityId>. I'm going to play with the code and see if I can get that working. Unless there's some reason that this isn't doable or isn't desirable. It would definitely add a level of complexity to the types.

[markerikson]

Yeah, that would definitely add complexity. Also, we're already looking at adding a second generic param to the adapter over in #948 , for defining fields in adapter.indices. That PR isn't guaranteed to make it in, but any additional changes would clash with that.

Given that in most cases people shouldn't really be needing to grab the IDs array by itself, I'm not sure there's sufficient benefit to making that overridable.

If you'd like to play around with the idea, please go ahead, but I think there'd need to be some some consideration of whether it's really useful.

[phryneas]

Ha, nice to see you from StackOverflow :wave:

I agree that it would be nice, but the timing on this is very unfortunate as Mark is really bulldozing through these files right now.

I'd suggest that we wait until that has calmed down and then revisit the whole thing? I'd like to avoid another generic argument on there, but it might be necessary... It could be kinda worth it... :thinking:

[markerikson]

(also, given that you seem to be doing a lot of answering questions regarding RTK, you ought to drop by Reactiflux and say hi to us over in the #redux channel at some point!)

[lindapaiste]

I'd suggest that we wait until that has calmed down and then revisit the whole thing?

Sounds good. I don't think it's hard to do, it's just a question of whether having an extra generic everywhere is worth it.

The only issue I'm running into is on the lines that have Object.assign.

const updated = Object.assign({}, original, update.changes)

This was before being inferred as T & Partial<T>. It is now being inferred as Dictionary<T>[I] & Partial<T>. But I must be either a string or number and Dictionary has index signatures of both string and number. Of course you could as T but my gears are turning as to why that's necessary and wasn't before. I had to go through step by step to understand how you can assign T | undefined to an empty object and get T because it's obviously not quite right.

[Shrugsy]

Additional info: This looks like it might be related to a previous experiment @phryneas tried in #841

[100terres]

Just to add to the discussion.

It feels to me that it might be useful to be able to define the EntityId in the createEntityAdapter.

In general, we probably know what's the type of the id of a specific entity. If for example it's string. It means that the selectIds would be string[] and not Array<string | number> as it currently returns. This can cause some small typing issues like this one:

Type 'EntityId' is not assignable to type 'string'. Type 'number' is not assignable to type 'string'.ts(2322)

interface UserEntity {
  id: string;
}

const selectors = usersAdapter.getSelectors();

const UsersList: React.FunctionComponent<ListProps> = () => {
  const userIds = useSelector(selectors.selectIds);

  return (
    <ul>
      {userIds.map((userId) => (
        <UserListItem userId={userId} />
{/*                   ^^^^^^
                      Type 'EntityId' is not assignable to type 'string'.
                        Type 'number' is not assignable to type 'string'.ts(2322)
*/}
      ))}
    </ul>
  );
}

interface UserListItemProps {
  userId: UserEntity["id"];
}

const UserListItem: React.FunctionComponent<UserListItemProps> = (props) => {
  const { userId } = props;
  const userEntity = useSelector((state) => selectors.selectById(state, userId));

  // ...
}

[keithfrazier98]

I am running into a similar issue as @100terres pointed out. Is there a common way to avoid this? I would really like to avoid having to coerce the EntityId to a string every time I need to use it.

[proninyaroslav]

Still needs attention. Proper TypeScript support required.

[EskiMojo14]

this was addressed in https://github.com/reduxjs/redux-toolkit/pull/3187, and is already in the v2.0 beta.

Currently it's a breaking change (due to the new type parameter required by all the types), which is why it's not in a 1.9 release.