Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'RootState' circularly references itself #2770

Closed
tsarapke opened this issue Oct 12, 2022 · 32 comments
Closed

'RootState' circularly references itself #2770

tsarapke opened this issue Oct 12, 2022 · 32 comments

Comments

@tsarapke
Copy link

Hello there!

We are migrating our applications to use @reduxjs/toolkit and faced with issue of RootState circularly references itself, and store implicitly has type any when we are trying to specify state type of createAsyncThunk<Blob, { height: number, width: number }, { state: RootState }>.

Reproduction

Screenshot:
image

We've already read some of existing topics here, like:
#2462
#2237
#1126
#324

But don't found any solution to handle it.

In addition, it would be nice to have exposing of AsyncThunkConfig from @reduxjs/toolkit, as for now there is no way infer that type in slice/reducer custom factory (only by specifying any)
image

@markerikson
Copy link
Collaborator

The main fallback option for this sort of issue is to infer the RootState type from the root reducer separately, instead of from store.getState:

const rootReducer = combineReducers({
  a: sliceA.reducer
})

type RootState = ReturnType<typeof rootReducer>;

Does that help?

@tsarapke
Copy link
Author

@markerikson Unfortunately no, it doesn't.
It solves the issue with type of const store, so it doesn't of any type anymore.
But RootState still circularly references itself.
image

@phryneas
Copy link
Member

phryneas commented Oct 13, 2022

One of your slices references RootState. In that case, you have to type that slice reducer on export.

So

export default slice.reducer

becomes

export default (slice.reducer as Reducer<typeof initialState>)

@tsarapke
Copy link
Author

@phryneas
It also doesn't help. At reproduction link, I've already specify return type of slice.

V1:
image

V2
image

@phryneas
Copy link
Member

Well, it is one of your slices that causes it - you might have gone with the wrong one here. Comment them out until the message disappears, then you know which one to do this on. All I can say is that that reducer somewhere references RootState while being part of it. And then you need that assertion I showed.

@tsarapke
Copy link
Author

tsarapke commented Oct 13, 2022

@phryneas
There are only 2 slices, created by the same creator
image

Slice at row 6 is fine.
Slice at row 7 is not.
But they are created by the same generator. It trows an error, until you will remove { state: RootState } from createAsyncThunk generic type. But because of we want to know getState, we can't remove it.

@phryneas
Copy link
Member

Do fetchImage2 or ImageState rely on the type of RootState?

@tsarapke
Copy link
Author

fetchImage2 rely on RootState. fetchImage2 is async thunk where we want to use getState.
image

ImageState doesn't rely on RootState. ImageState is a part of RootState.
image

@phryneas
Copy link
Member

Well, there you have your circular type reference.

Honestly, I can't do much with screenshots. If you give me a TypeScript playground that is complete enough to replicate your problem I can take a quick look and give a recommendation on how to solve it in your case.

@tsarapke
Copy link
Author

@phryneas, actually I left link to playground at the topic-starting message (here is duplicate)

image

If passing state to generic type of async thunk makes circular reference, why do we need it at all? From another side, how we will know about type returned by getState? Don't you find it strange?

@phryneas
Copy link
Member

phryneas commented Oct 14, 2022

I'll take a look later, missed that part. Thanks.

Generally: something like this is usually not a problem if you just write normal logic. But if you add wrapping logic like you are doing here, it gets problematic. That's just TS being TS. (without the createImageSlice you would not have any problem)

@phryneas
Copy link
Member

So, after taking a look, you can interrupt that circle by doing something like

const image1Slice = createImageSlice("image1", fetchImage1) as Reducer<ImageState, AnyAction>;
const image2Slice = createImageSlice("image2", fetchImage2) as Reducer<ImageState, AnyAction>;

The problem is really that the fetchImage2 needs the type of RootState, which depends on rootReducer, which depends on imageReducer, which depends on image2Slice - which then depends on fetchImage2 - a circle. And you have to break the circle at one point.

Generally, if I may ask:

  • why are you writing this by hand instead of using RTK Query, which would do that job for you? You are essentially recreating that here.
  • are you really really sure you want to store images in the Redux store? Those should usually not be in JavaScript memory at all.

@tsarapke
Copy link
Author

tsarapke commented Oct 14, 2022

Answering questions:

  • we are doing smooth migration step-by-step. We can't do it at one time. It very long process that must be done carefully. We have some middlewares that rely on current api wrapper. So we can't use RTK for now (but have plans in future). We have micro frontend, about 8 apps with huge business logic. So.... yeah.
  • we don't store images in Redux store, it would be a really bad idea. These reducers stores requestId, but not the images themselfs. In our app it also could store some meta-info (object) along to requestId.

Regarding to recursion. It's sad that we forced to use type casting (not real fan of it). BTW, createImageSlice already returns Reducer<ImageState, AnyAction>. Why it doesn't help?
image

It seems to me like the issue is not in asyncAction usage by the slice, but in arguments types of factory. When you will remove generic type and make asyncAction to be any - it starts work fine. But usage of any is not safe though.
image

@phryneas
Copy link
Member

Regarding to recursion. It's sad that we forced to use type casting (not real fan of it). BTW, createImageSlice already returns Reducer<ImageState, AnyAction>. Why it doesn't help?

Honestly, I do not know. This is a TypeScript quirk - we cannot do a lot about it.

And yes, changing your factory function in some way could also do that, but as you noticed it's not the best idea either.

In the next TS version, it will be possible to do

const image1Slice = createImageSlice("image1", fetchImage1) satisfies Reducer<ImageState, AnyAction> as Reducer<ImageState, AnyAction>;
const image2Slice = createImageSlice("image2", fetchImage2) satisfies Reducer<ImageState, AnyAction> as Reducer<ImageState, AnyAction>;

which is also not more beautiful but will at least be typesafe despite being a cast.

@tsarapke
Copy link
Author

Yeah.
Anyway, for now we decided to use type casting as temp workaround until we get close to RTK. Good thing there aren't many places like this. Don't really like to break the code of conduct 🙂

Another question - why do you not expose AsyncThunkConfig from @reduxjs/toolkit, so it may help to create some predefined configs types by extending this one?

@phryneas
Copy link
Member

Don't really like to break the code of conduct slightly_smiling_face

You really should not have a code of conduct that disallows the as keyword. It should not be overused, but there will always be edge cases where the programmer knows something the compiler does not and in those cases it is perfectly fine to use it. Pretty much every TypeScript teacher/book author I know agrees on this.

Another question - why do you not expose AsyncThunkConfig from @reduxjs/toolkit, so it may help to create some predefined configs types by extending this one?

Because you can just as well extend from an empty object - that base object has no value or meaning.

You will be thrilled about #2604 though - it is available in our 1.9 alphas.

@Emiya0306
Copy link

@tsarapke You can resolve the problem by this workaround. Enjoy it.

import { AsyncThunk, AsyncThunkPayloadCreator, Dispatch } from '@reduxjs/toolkit';

import { RootState } from './store';

declare module '@reduxjs/toolkit' {
  // https://stackoverflow.com/questions/64793504/cannot-set-getstate-type-to-rootstate-in-createasyncthunk
  type AsyncThunkConfig = {
    state?: unknown;
    dispatch?: Dispatch;
    extra?: unknown;
    rejectValue?: unknown;
    serializedErrorType?: unknown;
  };

  function createAsyncThunk<
    Returned,
    ThunkArg = void,
    ThunkApiConfig extends AsyncThunkConfig = {
      state: RootState;
    },
  >(
    typePrefix: string,
    payloadCreator: AsyncThunkPayloadCreator<
    Returned,
    ThunkArg,
    ThunkApiConfig
    >,
    options?: any
  ): AsyncThunk<Returned, ThunkArg, ThunkApiConfig>;
}

@tsarapke
Copy link
Author

@Emiya0306 Many thanks! I would prefer yours one. Looks more reasonable than continuous type-casting. Love it.

Probably one note. Maybe it would be better to describe options property too? Like:

options?: AsyncThunkOptions<ThunkArg, ThunkApiConfig>

@yegor-klymenchuk
Copy link

yegor-klymenchuk commented Mar 10, 2023

@tsarapke Hi, I just try to make same implementation , but any way I have getState: () => unknown. What I made wrong ?

Screenshot 2023-03-10 at 12 04 44

Screenshot 2023-03-10 at 12 06 02

Screenshot 2023-03-10 at 12 06 24

@tsarapke
Copy link
Author

Hi @EgorKlimenchuk. What version of toolkit do you use?
BTW, I think you didn't make anything wrong. It just because that workaround is doesn't work for toolkit > 1.8.6.

In 1.8.6 they was export creator like this:
image

But in 1.9.0 in was changed. "creator" is generated in IIEF (Immediately Invoked Function Expression)
image

So it does not allows to overload module type export correctly.

@yegor-klymenchuk
Copy link

yegor-klymenchuk commented Mar 10, 2023

I have updated redux/toolkit to 1.9.0 , and unfortunally it did't help me((

Screenshot 2023-03-10 at 15 38 47 1

Screenshot 2023-03-10 at 15 42 36

@tsarapke
Copy link
Author

@EgorKlimenchuk it doesn't work for 1.9+. Only for 1.8.6 or below

@yegor-klymenchuk
Copy link

@tsarapke aaa okey, but before update I was on 1.8.5

@tsarapke
Copy link
Author

@EgorKlimenchuk if you are using 1.8.6 or below, it depends on env. But you should check if your typescript takes into account your custom.d.ts file

@yegor-klymenchuk
Copy link

yegor-klymenchuk commented Mar 10, 2023

@tsarapke Yes if I declare the type in custom.d.ts , I can see difference. For example I can directly change AsyncThunkConfig . But anyway getState return unknown

Screenshot 2023-03-10 at 15 53 25 1

Screenshot 2023-03-10 at 15 53 50

@phryneas
Copy link
Member

createAsyncThunk.withTypes has shipped by now, maybe it would be better to use that instead of trying to overwrite our internal types.

@yegor-klymenchuk
Copy link

@phryneas Can I wrap createAsyncThunk or do I have to use withTypes every time?

@yegor-klymenchuk
Copy link

@phryneas when I use createAsyncThunk.withTypes and import RootState into slice I have this circular error

Screenshot 2023-03-10 at 20 24 16 1

@phryneas
Copy link
Member

Can I wrap createAsyncThunk or do I have to use withTypes every time?

You do that once per project.

This is not about importing it into the same file, it is because you create an actual type circle of calls. Usually, that happens because you are omitting the {} of an extraReducers function, but it's impossible to tell without access to your code. I'd suggest you try to boil it down to a minimal reproduction that you can show.

@yegor-klymenchuk
Copy link

yegor-klymenchuk commented Mar 11, 2023

@phryneas
The first of my problem, I have this error

Screenshot 2023-03-11 at 11 09 30

I found one solution in tsconfig.json add "declaration": false. But honestly I don't want to do it, cause I might create another problem

@yegor-klymenchuk
Copy link

yegor-klymenchuk commented Mar 11, 2023

@phryneas
The second problem even I enable "declaration": false and try to use the CreateAppAsyncThunk I have 2 problems:

  1. RootSate has circularly references itself .
  2. CreateAppAsyncThunk return any, but I think it happens because of 1 point

I have tried to split into different files RootState and setupStore, but it was the same problem

store/index.ts
Screenshot 2023-03-11 at 11 19 10
Screenshot 2023-03-11 at 11 26 21

store/reducers/EditorDataSliceReducer.ts
Screenshot 2023-03-11 at 11 18 00

@phryneas
Copy link
Member

Comment out all of your reducers.
Comment in reducers until the problem comes up.
Boil it down to a minimal reproduction.
Show it here.

Please don't give incomplete screenshots - nobody can help you with that.
I have yet to see a single car mechanic that could tell you what's wrong on the inside of the motor when showing them a photo of the left back corner of a car.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants