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

redux integration causes circular dependency #362

Closed
smkhalsa opened this issue Feb 16, 2017 · 21 comments
Closed

redux integration causes circular dependency #362

smkhalsa opened this issue Feb 16, 2017 · 21 comments

Comments

@smkhalsa
Copy link

I've implemented redux integration with the following reducer

export function navigation(state, action) {
  return Navigator.router.getStateForAction(action, state)
}

where Navigator is

const Navigator = StackNavigator({
  Main: {
    screen: StacksInTabs,
  },
}, {
  mode: 'modal',
  headerMode: 'none',
})

I'm trying to import my redux store within my application. However, since the StackNavigator depends upon every component in my app, importing my redux store creates a circular dependency.

This results in Navigator being undefined in the above reducer.

@ericvicenti
Copy link
Contributor

Is there a reason your store needs to be accessed by your screens? Generally you should use a <Provider store={store}> outside of your navigation hierarchy, which should avoid any circular references.

@smkhalsa
Copy link
Author

I'm creating an action sheet handler in which I need to access state and dispatch actions that are not needed on the screen that initiates the action sheet. Furthermore, this action sheet can be initiated from several different screens. I've been trying to avoid pulling in state in each screen that is not required for the screens themselves simply to pass them into the action sheet handler if the action sheet handler can access state and dispatch actions directly itself.

@elliotdickison
Copy link

I ran into this exact issue. My components import my redux store because I personally prefer an explicit import to passing the store via context/Provider. This causes a circular dependency: Components <- Store <- Reducer <- Router <- Components.

For me the most logical place to break this circle is between Router and Components. Conceptually it seems odd that the router's reducer, getStateForAction, ultimately depends upon view components (although I admit the resulting "screens" API is nice). The app view should depend on the app state, but not the other way around.

Is there any way to create a purely static route config that doesn't involve view components (i.e. screens)?

@elliotdickison
Copy link

elliotdickison commented Mar 2, 2017

@smkhalsa If this helps, what I ended up doing for now was passing my Navigator.router to my reducer via a wrapper action that looks something like this:

{ type: "NAV", payload: { action: <react-navigation action>, router: Router.router }

My reducer is then able to handle it with something like this:

case "NAV":
  const nextNavState = payload.router.getStateForAction(payload.action, state.navigation)

Not great, but it allowed me to keep the hacks localized to my routing code instead of changing my whole react/redux integration setup.

I was also able to create that wrapper action in the dispatch method that I provide to "addNavigationHelpers", so as long as I use the helpers provided by that (props.navigation.navigate, etc.) I can forget about the whole mess.

@JulianKingman
Copy link

Another use-case: I need to access state in a static property, which can't be done using @connect.

Unfortunately this is blocking me from migrating from ex-navigation, as I use Store.getState() pretty liberally throughout the app, wherever I don't need to directly depend on state changes (e.g. with functions, methods, and static attributes).

Maybe the navigator could at least ignore/strip out static properties that are not navigationOptions?

@jamsesso
Copy link

jamsesso commented Mar 17, 2017

I have also run into this problem. I ended up creating a singleton function to initialize (if needed) and return the store. It looks like this:

import {createStore} from 'redux';
import rootReducer from './reducers';

let store;

export default function getStore() {
  if(!store) {
    store = createStore(rootReducer);
  }
  return store;
}

Certainly not ideal, but a workaround.

@gsaandy
Copy link

gsaandy commented Apr 15, 2017

I created a storeRegistry and register the store while configuring it, and use this storeRegistry to access store outside react.

App.js

/* @flow */
import React from 'react';
import { Provider } from 'react-redux';
import AppNavigatorContainer from 'routes/AppNavigatorContainer';
import configureStore from 'store/configureStore';
import storeRegistry from 'store/storeRegistry';

const store = configureStore();
storeRegistry.register(store);

const App = () => (
  <Provider store={store}>
    <AppNavigatorContainer />
  </Provider>
);

export default App;

access the store outside react as below

import storeRegistry from 'store/storeRegistry';

const store = storeRegistry.getStore();

@vexsnare
Copy link

vexsnare commented Jun 8, 2017

Same issue. I need to access store when validating AuthToken.

Store -> Service -> Actions -> Component

@dwilt
Copy link

dwilt commented Jul 25, 2017

I believe we're running into this now as well too.

@dwilt
Copy link

dwilt commented Jul 26, 2017

Couldn't come up with a workable solution to this. Sadly heading back to react-native-router-flux

@maikokuppe
Copy link

maikokuppe commented Sep 3, 2017

I think this exact problem is, what the mapDispatchToProps function is meant for (http://redux.js.org/docs/basics/UsageWithReact.html#implementing-container-components).

I realized that my navigation actions do not need to import the Navigator, but just NavigationActions from react-navigation. See #840 (comment)

Further, my js module, which wants to dispatch an action to the store and therefore needs to import the store, actually does not need the store! It just needs the dispatch function. So, following the example in the first link, this is the flow:

  • Call a this.props.someFunc() callback inside the presentational component
  • Do some stuff, using the dispatch function inside the container component. If necessary, pass the dispatch function to other functions / modules

I have the feeling, that this covers most usecases.

@dwilt
Copy link

dwilt commented Sep 4, 2017

Call a this.props.someFunc() callback inside the presentational component

The whole idea in this issue is not calling it from a component

@MilllerTime
Copy link

I can't comment on react-navigation specifically. I've still been bitten by a circular dependency chain created from importing my store to use somewhere outside of my component tree, similar to the examples above. That seems to be the focus here.

I've researched this issue before and only came up with suggestions to use middleware. I may be missing something, but I fail to see how middleware is relevant to some of the problems I've encountered. I have to agree with @JulianKingman that accessing the current state at a given moment (not reactively) from within an arbitrary function (or event callback) can be very useful.

I've personally settled on @gsaandy's solution of creating a reference to the store in an isolated module which doesn't actually import the store. His example is great. At first it felt dirty to reference the store that way, but it has worked out quite nicely outside of the component tree. There's no need to think about dependencies either, since storeRegistry (or whatever) should itself be dependency free. Nice to see someone else reach the same conclusion. Of course, it should only be used when needed.

@dwilt
Copy link

dwilt commented Oct 10, 2017

@MilllerTime I use my store outside of components (in services) all the time and I don't have any circular dependency issues. I'd be interested in doing a screen share with you and looking at your code to see what exactly is happening.

@MilllerTime
Copy link

@dwilt I appreciate the offer! I think I got it though. After taking a hard look at my ad hoc usage of getState, I realized I can indeed import the store directly in each case - like you're doing. My issues with circular references have since been resolved, at least for getState.

I suspect anyone else encountering circular refs when importing their store to access state in service/helper functions can resolve them by taking a look at their module structure and extracting functions which might live in the "wrong place". That's all I did for a couple offenders.

However, I still need access to store.dispatch() in a couple of places where without the storeRegistry concept, I'd have a circular reference. A very simple example is my API helper module and the store depending on each other:

apiHelper.js <==> store.js

Here, store.js imports the API helper and passes it to ReduxThunk.withExtraArgument(). On the other end, the apiHelper.js module imports the store so it can dispatch a destroy session action if any requests fail because the current auth token is invalid. Middleware could handle this, but that feels like a layer of indirection. I already have a module dedicated to interacting with the API, and I like that it's also responsible for handling all API related errors. In this case, I do not see a way around using the storeRegistry concept to break the circle. Still, I'm always thrilled to hear new ideas 😄

@brentvatne
Copy link
Member

a PR to the documentation would be helpful for this! https://github.com/react-navigation/react-navigation.github.io

closing as we are reserving this particular issue tracker for bugs

@ifier
Copy link

ifier commented Jun 23, 2020

configureStore

Can you provide full example of Class please?

@gsaandy
Copy link

gsaandy commented Jun 23, 2020

configureStore

Can you provide full example of Class please?

@ifier see the below example from https://redux.js.org/recipes/configuring-your-store

import { applyMiddleware, compose, createStore } from 'redux'
import thunkMiddleware from 'redux-thunk'

import monitorReducersEnhancer from './enhancers/monitorReducers'
import loggerMiddleware from './middleware/logger'
import rootReducer from './reducers'

export default function configureStore(preloadedState) {
  const middlewares = [loggerMiddleware, thunkMiddleware]
  const middlewareEnhancer = applyMiddleware(...middlewares)

  const enhancers = [middlewareEnhancer, monitorReducersEnhancer]
  const composedEnhancers = compose(...enhancers)

  const store = createStore(rootReducer, preloadedState, composedEnhancers)

  return store
}

@MHasankhan
Copy link

@gsaandy can you share what exactly you have done in this files
image

@github-actions
Copy link

Hey! This issue is closed and isn't watched by the core team. You are welcome to discuss the issue with others in this thread, but if you think this issue is still valid and needs to be tracked, please open a new issue with a repro.

@gsaandy
Copy link

gsaandy commented Mar 8, 2022

@gsaandy can you share what exactly you have done in this files image

configureStore.ts

import { applyMiddleware, createStore, Middleware } from 'redux';
import thunkMiddleware from 'redux-thunk';
import defaultLogger from 'redux-logger';
import { persistReducer } from 'redux-persist';
import storage from 'redux-persist/lib/storage';
import rootReducer from './reducers/rootReducer';

const { composeWithDevTools } = __DEV__ ? { composeWithDevTools: v => v } : require('redux-devtools-extension');

const middlewares: Middleware[] = [thunkMiddleware];

if (__DEV__) {
  middlewares.push(defaultLogger);
}

const persistConfig = {
  key: 'root',
  storage,
  whitelist: ['session', 'appConfig'],
};
const persistedReducer = persistReducer(persistConfig, rootReducer);

const createStoreWithMiddleware = composeWithDevTools(applyMiddleware(...middlewares))(createStore);

export default (initialState = {}) => createStoreWithMiddleware(persistedReducer, initialState);

storeRegistry.ts

import configureStore from './configureStore';

class StoreRegistry {

  private storePersistor;
  private store = configureStore();

  getStore() {
    return this.store;
  }

  registerStorePersistor(persistor) {
    this.storePersistor = persistor;
  }

  getStorePersistor() {
    return this.storePersistor;
  }

}

export default new StoreRegistry();

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

No branches or pull requests