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

API for checking whether the monitor is open #145

Closed
rybon opened this issue Jun 23, 2016 · 11 comments
Closed

API for checking whether the monitor is open #145

rybon opened this issue Jun 23, 2016 · 11 comments

Comments

@rybon
Copy link

rybon commented Jun 23, 2016

Is it possible to call a method on window.devToolsExtension to determine whether the DevTools monitor is currently open or closed? I'd like to use that information to conditionally disable side effects. As far as I know we can only check whether window.devToolsExtension is available.

@zalmoxisus
Copy link
Owner

zalmoxisus commented Jun 23, 2016

There's no such method in our API, but it wouldn't be hard to implement (the only problem is when having more instances in one tab).

Could you tell me about your user case? Maybe better to provide a way to check when a monitor action is applied (toggling, moving back and forth, importing the history...) to prevent side effects in this case?

Also related to #140.

@rybon
Copy link
Author

rybon commented Jun 23, 2016

You're right, a more granular way of disabling side effects would indeed be preferable.

Starting with opening and closing the monitor seems to be the easiest and quickest way for now however. I'm currently using redux-saga and my idea is to combine that functionality with a hook in redux-saga to conditionally disable the processing of side effects. In redux-saga, side effects are not executed immediately, instead they are declared using descriptor objects that are then processed at a later stage. Wrapping all side effects in sagas means one has more control over them, as they can now be intercepted and optionally cancelled.

@zalmoxisus
Copy link
Owner

Could you share a snippet of how you want to integrate it in sagas?

If you prevent all side effects when the extension's monitor is open, that means you'll not be able to use it for logging and remote control, only for dispatching monitors actions.

@rybon
Copy link
Author

rybon commented Jun 24, 2016

In redux-saga you have the option to add a SagaMonitor as part of initializing the saga middleware. In my local fork of redux-saga, I've just added another method to the monitor, that is subsequently invoked in runEffect. This method just returns a boolean and short-circuits the runEffect method accordingly. This means I have control over the processing of the side effects. I just need a way to dynamically return that boolean based on some state, for example the DevTools being open or not. Relevant: reduxjs/redux-devtools#167 (comment).

Of course, a better solution would be what Cerebral does and that is to keep track of which side effect caused what output, and only replay the output instead of the side effects when time travelling. A proper solution is still under discussion however, see redux-saga/redux-saga#5 and redux-saga/redux-saga#22.

For now though, a hook would do the trick until we have found a better way.

@zalmoxisus
Copy link
Owner

zalmoxisus commented Jul 6, 2016

@rybon, I was thinking about this, and adding to the API a global function like window.devToolsExtension.isMonitoring would cause conflicts in case we have multiple instances (stores). A solution would be to get this when invoking the enhancer like

const sagaMiddleware = createSagaMiddleware();
let isMonitoring;
const store = createStore(
  reducer,
  compose(
    applyMiddleware(sagaMiddleware),
    window.devToolsExtension &&  window.devToolsExtension({ isMonitoring })
  )
);
sagaMiddleware.run(rootSaga, isMonitoring);

Anyway, we want to monitor side effects as well (which I want to implement in the nearest future), so such a solution wouldn't be viable. Better to have something like isTimeTraveling or/and isMonitorAction instead of isMonitoring.

What do you think?

@rybon
Copy link
Author

rybon commented Jul 6, 2016

Looks alright to start with. I presume isMonitoring will be set to true or false by devToolsExtension? Wouldn't it be better to wrap it in a callback, so that can be invoked inside redux-saga to fetch the latest isMonitoring value? See redux-saga/redux-saga#410 for an example.

@zalmoxisus
Copy link
Owner

zalmoxisus commented Jul 6, 2016

Yes, it would be a function, which returns true or false.

Can we just use isTimeTraveling instead of isMonitoring? Are other cases when we want to disable side effects? Disabling sagas when the monitor window is opened wouldn't show what is really happen in the application.

@rybon
Copy link
Author

rybon commented Jul 6, 2016

Sure, isTimeTraveling is fine with me. Regarding other cases, I suppose I'd also want to disable side effects when replaying a Redux log in a test suite, for example based on process.env.NODE_ENV === 'test'. But that would be a static value that wouldn't change at runtime.
In my redux-saga PR, I added a hook so developers can decide for themselves when side effects should be disabled.

zalmoxisus added a commit that referenced this issue Jul 15, 2016
zalmoxisus added a commit that referenced this issue Jul 15, 2016
@zalmoxisus
Copy link
Owner

Implemented in v2.3.0. See the description and the example of usage.

A PR with a recipe or an example of usage with sagas would be much appreciated.

@rybon
Copy link
Author

rybon commented Jul 15, 2016

Thanks! An example with my forked redux-saga repo will be forthcoming.

@zalmoxisus zalmoxisus mentioned this issue Jul 18, 2016
@zalmoxisus
Copy link
Owner

There's a better way now to prevent side effects from middlewares like redux-saga. See the post for details on how to get it.

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

2 participants