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

Tips on how to avoid circular dependencies with the module pattern and inferring RootState from rootReducer? #154

Closed
thomasvanlankveld opened this issue Apr 9, 2019 · 11 comments

Comments

@thomasvanlankveld
Copy link

I keep running into circular dependency issues, which mostly seem to stem from inferring RootState from the rootReducer.

What happens in many of my tests is that they import things from modules via index.ts files, which list a bunch of files. In many cases, this triggers the loading of files that depend on RootState, which needs rootReducer, which needs all the other reducers, all of which are loaded from modules via index.ts files. At this point, often one of these index files is hit for the second time and the loader halts, leaving several variables undefined and resulting in cryptic error messages in the test output. If this doesn't go wrong, there is still a possibility for things to go wrong in the dependencies of these reducers.

What is the recommended pattern here? Some approaches I can think of are:

  • Move away from the module pattern and to put all reducers (and their dependencies) in a single place.
  • Move away from RootState inference and just define it explicitly.
  • Put reducers in a separate place inside each module, and don't expose them via the module's index.ts. Do something similar with all of the reducer dependencies.

I'm currently trying out something similar to the third option. Any thoughts?

@chawax
Copy link
Contributor

chawax commented Apr 9, 2019

You can define RootState in an ambient module as explained here : #85

Something like that in a .d.ts file :

import { StateType } from 'typesafe-actions';
import rootReducer from '../redux/reducers';

declare module 'Types' {
  export type RootState = StateType<typeof rootReducer>;
}

Then for example when you define a selector :

import Types from 'Types';
const mySelector = (state: Types.RootState): string = ....

There is an example of that in playground project.

https://github.com/piotrwitek/react-redux-typescript-guide/blob/master/playground/src/store/types.d.ts

I use that in my project and it works well.

Hope it will help !

@chawax
Copy link
Contributor

chawax commented Apr 13, 2019

@thomasvanlankveld Did it help you ?

@thomasvanlankveld
Copy link
Author

thomasvanlankveld commented Apr 13, 2019

@chawax Thanks for the swift reply!

I tried the suggestion in two different ways. When I try it with import statements outside the declare block (as in your example above), other files are not able to find the ambient module:

// src/App/core/RootTypes.d.ts
import { StateType } from 'typesafe-actions';
import { rootReducer } from '../store/rootReducer';
import { PostActions } from '../../Posts/core';
import { ServerAPI } from '../../ServerAPI';

declare module "RootTypes" {
  export type RootAction = PostActions;
  export type RootState = StateType<typeof rootReducer>;
  export type Services = { serverAPI: ServerAPI };
}

The following then yields Cannot find module 'RootTypes'.:

// src/App/store/setupStore.ts
import { Services, RootState, RootAction } from 'RootTypes';

When I move the imports into the declare 'block', then the types RootAction, RootState and Services resolve to any. I should mention that I don't have much experience with declaration files, so I'm probably doing something wrong.

For now I just made RootState an explicit combination of the slice states, since the guide recommends making all of these explicit anyway. My slice state types are in various src/SomeModule/core folders, along with the action creators. Reducers, selectors and epics go in src/SomeModule/data. Both these folders have their own index files, and none of their contents are exported from src/SomeModule (although some components from src/SomeModule/components are).

// src/App/core/types.ts
import { PostActions, PostsState, AuthorsState } from "../../Posts/core";
import { UsersState } from "../../Users/core";
import { ServerAPI } from "../../ServerAPI";

export type RootAction = PostActions;
export type RootState = Readonly<{
  posts: PostsState;
  authors: AuthorsState;
  users: UsersState;
}>;
export type Services = { serverAPI: ServerAPI };

// src/App/data/rootReducer.ts
import { combineReducers, Reducer } from 'redux';
import { RootAction, RootState } from '../core';
import { postsReducer, authorsReducer } from '../../Posts/data';
import { usersReducer } from '../../Users/data';

export const rootReducer: Reducer<RootState, RootAction> = combineReducers({
  posts: postsReducer,
  authors: authorsReducer,
  users: usersReducer
});

This setup works well so far. It requires very little extra (one might say 'redundant') typing, with the added benefits of making sure the state tree is very easy to explore with VSC, and that it is Readonly all the way up.

Edit: I guess I could maybe still use ambient modules to get absolute imports for all of these things...

@piotrwitek
Copy link
Owner

piotrwitek commented Apr 13, 2019

I'm closing this as it is an old and solved issue and the working example solution was also linked by @chawax: https://github.com/piotrwitek/react-redux-typescript-guide/blob/master/playground/src/store/types.d.ts

Btw. the correct solution was producing any for you because you were not following the example precisely, just use the exact same syntax as in the example and you'll be good.

@thomasvanlankveld
Copy link
Author

Ahhhh thanks! The following does work:

declare module 'RootTypes' {
  import { StateType } from "typesafe-actions";
  export type RootAction = import('../../Posts/core').PostActions;
  export type RootState = StateType<typeof import('../store/rootReducer').rootReducer>;
  export type Services = { serverAPI: import('../../ServerAPI').ServerAPI };
}

@piotrwitek In this setup, RootState is no longer readonly, do you happen to have any thoughts on this?

@piotrwitek
Copy link
Owner

piotrwitek commented Apr 13, 2019

Glad it worked!

Readonly doesn't change anything right now because it's not recursive anyway, now from 3.4 we have new syntax for type assertion as const and it'll make things change.
I just need to update the type helper StateType and that's all, you don't have to worry about it.

Perhaps you could even create a ticket for it in "typesafe-actions" repo if you want.

@thomasvanlankveld
Copy link
Author

That sounds promising! In the meantime, the following seems to do the trick:

import { DeepReadonly } from "utility-types";
export type RootState = DeepReadonly<
  StateType<typeof import('../store/rootReducer').rootReducer>
>;

@piotrwitek
Copy link
Owner

@thomasvanlankveld yeah definitely, that's another option!

@tomspeak
Copy link

tomspeak commented Jun 23, 2019

@piotrwitek @thomasvanlankveld @chawax

Sorry for reviving this thread, but I had to make sure it's abundantly clear how grateful I am for this thread.

You've resolved two days of absolute misery for me working through this circular dependency issue.

Thank you 🙏

@ethanneff
Copy link

Follow up to confirm this worked for me as well. Additional thank you!

@Chetan11-dev
Copy link

If you use reactjs then
declare the types in react-app-env.d.ts

//react-app-env.d.ts
declare module 'Types' {
    import { StateType } from "typesafe-actions";
    export type RootState =  StateType<typeof import('src/redux/rootReducer').default>;
}

then import so

import Types from 'Types';

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

6 participants