rtk-incubator / rtk-query

Data fetching and caching addon for Redux Toolkit
https://redux-toolkit.js.org/rtk-query/overview
MIT License
626 stars 31 forks source link

fix getState type #144

Closed kahirokunn closed 3 years ago

codesandbox-ci[bot] commented 3 years ago

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 4c7a79e6eaf7337ac84b6c11f36378f9f3fe30fd:

Sandbox Source
React Configuration
React Typescript Configuration
rtk-query-demo Configuration
svelte-app-rtk-simplequery-demo Configuration
phryneas commented 3 years ago

Hmm, this would add a dependency to react-redux from the (in the next release, see #141) framework-agnostic entry point /, causing build failures for many people.

So we won't be able to add that one, I'm sorry. Guess it'll have to be a manual cast for now.

All that aside:

DefaultRootState is not anything really official - that's just something someone at DefinitelyTyped added to the types.
I mean, the pattern itself is a cool idea, but it's also pretty dangerous as just about any other module can now accidentally "poison" your global state structure just by being installed as a dependency.
We're still not sure if we want to embrace that pattern of module augmentation or not - in any case it's not part of any official documentation or types of any officially maintained redux module.

Kinda the downside of having the types externally is that people always add new stuff without any real sense of ownership or communication. :/

kahirokunn commented 3 years ago

I see. sorry :pray: Thx for your explanation!

phryneas commented 3 years ago

No need to be sorry, if we wouldn't have the need to keep / clean and DefaultRootState would be a bit more official, this would have been a great addition.

Thanks for that!