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

Any plans about compatibility with @ngrx/store ? #77

Closed
mildful opened this issue Mar 23, 2016 · 19 comments
Closed

Any plans about compatibility with @ngrx/store ? #77

mildful opened this issue Mar 23, 2016 · 19 comments

Comments

@mildful
Copy link

mildful commented Mar 23, 2016

Hi.

I'm using @ngrx/store to manage my Angular 2 application which is inspired by Redux.

I would like to know if redux-devtools-extension could be compatible with it ? I'm working on it but if someone already knows something about that and can help, it would be fine.

Thanks for this awesome tool :-)

@zalmoxisus
Copy link
Owner

Hey,

As it doesn't support store enhancers, only middlewares (and those aren't compatible with redux), it wouldn't be possible to use with Redux DevTools directly.

You could create a bind as for freezer or use remotedev.

In case you release a solution, please let me know, so I will add it to the README.

@mildful
Copy link
Author

mildful commented Mar 23, 2016

@zalmoxisus
Copy link
Owner

Thanks for the info! Is there any example with Devtools (the official example seems not to include it) so I could investigate it?

@mildful
Copy link
Author

mildful commented Mar 24, 2016

You can see an implementation of it here : https://plnkr.co/edit/46Sutu999iJ9SNN9xhyC?p=preview :-)

@zalmoxisus
Copy link
Owner

@mildful, thanks for the demo. The good news is that it uses the same data structure as Redux Devtools stores in store.liftedStore (you can get it from this.store._backend.liftedState.value), but @ngrx/store API is completely different from the Redux one.

I'm not too familiar with observables, if you want to try it to implement it, all you have to do is to replicate the script the the extension uses to communicate with the page. This script can be added in the page directly, and it will send and receive the changes. All you have to do is to replace store.subscribe with observables. Instead of store.getState and store.liftedStore.getState, you could read it directly from store._value and this.store._backend.liftedState.value (as I don't see similar functions in @ngrx/store). If you could implement that, we'll have the first step done - subscribing extensions monitors to the app's store changes.

The main advantage of integrating it with the extension would be that you will be able use the awesome Redux DevTools monitors.

@mildful
Copy link
Author

mildful commented Mar 24, 2016

@zalmoxisus thanks a lot.
I'll try to do as soon as possible.

@johnhamm
Copy link

Any progress on this? Although I have ngrx devtools working with my ng2 project, I would much rather use the extension for the awesome monitor capabilities and the ability to use with production builds.

@zalmoxisus
Copy link
Owner

We should first ask @MikeRyan52 if he would be interested to support the extension there, otherwise any effort here will be broken with new API changes in ngrx/devtools.

@mildful
Copy link
Author

mildful commented Apr 14, 2016

Currently, I have a lot of work and I have not tried. Sorry !

@MikeRyanDev
Copy link

@zalmoxisus I'm certainly interested in seeing if it is possible to add @ngrx/store support, but as you've already discovered the way the store is instrumented is very different from redux. The general problem is that we expose our store as an observable, so simply lifting the store's API doesn't really work.

With that being said, since the underlying wrapped state interface is the exact same it might be feasible to cast our lifted store to the redux store API:

function castToReduxAPI(tools: StoreDevtools) {
  return {
    getState() {
      return tools.liftedState.getValue();
    },
    dispatch(action) {
      tools.dispatch(action);
    },
    subscribe(listener) {
      const sub = tools.liftedState.subscribe(listener);

      return () => sub.unsubscribe();
    },
    replaceReducer(reducer) {
      tools.replaceReducer(reducer);
    }
  }
}

@zalmoxisus
Copy link
Owner

@MikeRyan52, thanks for the details. Actually, the extension is quite agnostic about the client's store. Redux Devtools with the monitors is on the background page. We just need to modify the injected script to relay the changes from @ngrx/store (with the same structure as for the liftedState) to the background script and have the ability to dispatch actions received from the background script inside @ngrx/store.

I'll try to refactor the injected script to have an universal API.

@zalmoxisus
Copy link
Owner

The extension has now an extended API to communicate with it directly. So, it's easy to use it for any architecture. Here's an example of usage. If someone could share an example of how to integrate it with @ngrx/store it would be very welcome.

@MikeRyanDev
Copy link

I'll take a stab at it 👍

@MikeRyanDev
Copy link

Ok, I got very basic undo/redo working A few questions:

  • What and when should I be sending to devToolsExtension.send() ? Lifted actions and lifted state?
  • How do I keep the lifted state in sync with the monitors?
  • DISPATCH actions coming from the extension should be funneled through the lifted reducer, right?

zalmoxisus added a commit that referenced this issue Jun 1, 2016
@zalmoxisus
Copy link
Owner

zalmoxisus commented Jun 1, 2016

@MikeRyan52, sorry, I forgot about liftedState. Now instead of send(action, state) we can use also send(null, liftedState) (I'll publish the changes to Chrome store in few hours).

After connecting with const devTools = window.devToolsExtension.connect() better to use devTools.send() instead (it's the same, but we don't have to keep track of instance id).

Basically,

let isStarted = false;

const devTools = window.devToolsExtension.connect();
devTools.subscribe((message) => {
  if (message.type === 'START') {
    isStarted = true;
    devTools.send(null, store.liftedStore.getState());
  } else if (message.type === 'STOP') {
    isStarted = false;
  } else if (message.type === 'DISPATCH' && message.payload.type !== 'JUMP_TO_STATE') {
    store.liftedStore.dispatch(message.payload);
  } else if (message.type === 'ACTION') { // Received a store action from Dispatch monitor
    store.dispatch(message.payload);
  } 
});

store.subscribe(() => {
  if (!isStarted) return;
  const liftedState = store.liftedStore.getState();
  devTools.send(null, liftedState);
});

We could always send liftedState for any changes inside the store, and it should work, but this object can grow too big and it will affect the performance. So, it's better to use something like this:

let isStarted = false;
let isLiftedAction = false;

const devTools = window.devToolsExtension.connect();
devTools.subscribe((message) => {
  if (message.type === 'START') {
    isStarted = true;
    devTools.send(null, store.liftedStore.getState());
  } else if (message.type === 'STOP') {
    isStarted = false;
  } else if (message.type === 'DISPATCH' && message.payload.type !== 'JUMP_TO_STATE') {
    isLiftedAction = true;
    store.liftedStore.dispatch(message.payload);
  } else if (message.type === 'ACTION') { // Received a store action from Dispatch monitor
    store.dispatch(message.payload);
  } 
});

store.subscribe(() => {
  if (!isStarted) return;
  const liftedState = store.liftedStore.getState();
  if (isLiftedAction) { devTools.send(null, liftedState); isLiftedAction = false; }
  else devTools.send(liftedState.actionsById[liftedState.nextActionId - 1], store.getState());
});

I didn't try this code, will try to add a more complex example next days after the conference.

@MikeRyanDev
Copy link

Sounds great. Here's a link to our example app with experimental support for the extension: http://ngrx.github.io/example-app/

@MikeRyanDev
Copy link

@zalmoxisus Next release of ngrx/store-devtools will have full extension compatibility. Let's coordinate an announcement.

@zalmoxisus
Copy link
Owner

@MikeRyan52, that's awesome! I'll add it to the README.

@zalmoxisus
Copy link
Owner

Added also an Integrations page. Feel free to send a PR if any changes in the API occur.

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

4 participants