Add type parameter for Id in createEntityAdapter

[Matt-Ord]

Fixes issue #2098 Let me know if this change also needs any docs/ I've messed up the system for creating a PR

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0e796262c93fdc6b0317a4081963776c3dddada1:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration
@examples-action-listener/counter	Configuration

[netlify[bot]]

✅ Deploy Preview for redux-starter-kit-docs ready!

Name	Link
🔨 Latest commit	0e796262c93fdc6b0317a4081963776c3dddada1
🔍 Latest deploy log	https://app.netlify.com/sites/redux-starter-kit-docs/deploys/624b2f9024112e000828c2cb
😎 Deploy Preview	https://deploy-preview-2099--redux-starter-kit-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[phryneas]

I've gone over this - from my side it's okay @markerikson . I've also roughly gone over https://github.com/reduxjs/redux-toolkit/pull/948 in the context of this - it doesn't seem like the types changes would overlap too much, probably a few types would get one new type argument from each PR, but most would be affected by only one of the two PRs.

[Matt-Ord]

Do I need to do anything to update the docs- I think currently they have still have the old types. Also is there any point keeping the DictionaryNum type around still?

[phryneas]

Yeah, docs PRs would be great where you see them necessary 👍

[Matt-Ord]

I think I've updated the docs where it's needed - it should be ready to go

[markerikson]

Hiya. I'm looking at this, and it seems like it makes some decidedly breaking changes to the types: it looks like we have to pass , Id everywhere (including existing public types), and it also rewrites the Dictionary type.

Given that, I don't think we can ship this in a 1.x release, unless I'm missing something.

Also, I'm realllly not sure about this line:

export type Dictionary<T, Id extends EntityId> = Partial<Record<Id, T>>

That looks funky to me

[Matt-Ord]

I would agree this is probably a breaking change - although I think it will be 'worth it' when you come to a 2.0 release, since it's solved a real problem on a medium to large codebase I've been working on.

Ideally I would say swap the order of Id and T (and do the same on all types) but this would make it 'more breaking' - is this the issue you have with the dictionary type?

[Matt-Ord]

@markerikson Just seen this pr was closed - is this not something you want to change anymore? I'm happy to put the work in to update it if we're coming up to a 2.0 release?

[phryneas]

I think this was just an accident. But funny enough, I don't have permissions to reopen ^^

[markerikson]

Uh. Yeah, I don't have permission to reopen it either! I think the issue is it was targeted to the v1.9 branch.

@Matt-Ord guess create a new PR? Go ahead and target master for now.