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

4.0.x doesn't support Redux DevTools Chrome Extension #196

Open
hangzho opened this issue Mar 23, 2018 · 11 comments
Open

4.0.x doesn't support Redux DevTools Chrome Extension #196

hangzho opened this issue Mar 23, 2018 · 11 comments
Labels

Comments

@hangzho
Copy link

hangzho commented Mar 23, 2018

Redux DevTools Chrome Extension doesn't work after I upgrade the ng-redux to 4.0.x.
The console shows Error: Actions must be plain objects. Use custom middleware for async actions. even though I've already used the thunk middleware like this

$ngReduxProvider.createStoreWith(rootReducer, [thunk], [window.__REDUX_DEVTOOLS_EXTENSION__()]);

The ng-redux library works well if I remove the storeEnhancer [window.__REDUX_DEVTOOLS_EXTENSION__()].

I can reproduce the issue in the example project(https://github.com/johanzhou8/ng-redux/tree/master/examples/async-immutable) too.
Steps to reproduce the issue:

  1. download the example project
  2. install the dependencies. npm install
  3. upgrade the ng-redux. npm i [email protected]
  4. use the redux devtool storeEnhancer in the index.js file. $ngReduxProvider.createStoreWith(rootReducer, [thunk], [window.__REDUX_DEVTOOLS_EXTENSION__()]);
  5. start the server. npm start
  6. look at the error message in the console

The workaround for me is downgrading to the older version. npm i [email protected].
Thanks.

@GuyPie
Copy link

GuyPie commented Mar 23, 2018

This happens to me as well, except in my case I wasn't seeing any error, just that redux-observable actions weren't working when the devtools were included. Also, I noticed redux-immutable-state-invariant complained about how my reducer was mutating my state even though it was not, this is also not happening in version 3.5.2

@bjohnso5
Copy link

bjohnso5 commented Mar 23, 2018

I get this same error with 4.0.x and redux-thunk. 3.5.2 works as expected.
Edit: strange behaviour with 4.0.x and redux-observable epics as well.

@AntJanus
Copy link
Collaborator

AntJanus commented Mar 26, 2018

Has anyone been able to figure out where the issue happens in ng-redux?

The 4.0.X release had that whole provideStore feature. I wonder if that's causing it. I'm one of the maintainers but I barely have time. If any of you can contribute with a PR or further debugging info, that'd be immensely helpful in speeding up bugfixing.

@AntJanus AntJanus added the bug label Mar 26, 2018
@bjohnso5
Copy link

When invoking a redux-thunk action, I get the following stack trace:

Error: Actions must be plain objects. Use custom middleware for async actions.
    at Object.performAction (<anonymous>:1:40841)
    at liftAction (<anonymous>:1:34377)
    at dispatch (<anonymous>:1:38408)
    at e.changeGreeting (bindActionCreators.js:3)

You can attempt to repro by cloning this: https://github.com/bjohnso5/angularjs-redux-example/tree/feature/ng-redux-4-bug-report and npm run start:dev or yarn start:dev and browsing to http://localhost:8080/

@AntJanus
Copy link
Collaborator

Shit, I just realized what it is. Alright, I'd love some suggestions here.

@bjohnso5 the reason for the issue is the new provideStore mechanism. It wraps the provided store and subscribes to it to push changes from the AngularJS store to the provided store. This means that passing functions will cause an error.

Hmmm...

@derrickpelletier you worked a little bit on the provided store feature, do you have any ideas about how to resolve this?

@derrickpelletier
Copy link
Contributor

derrickpelletier commented Mar 30, 2018

Hiding the contents of this comment because they were misleading, see below.

Ah, i'll try to dig into this a bit later today or this weekend (away for holidays).

My gut is that the issue comes from the fact that both stores need awareness of the same middleware, and this isn't reflected in the docs right now...

For example, here's how i'm using this in production and I have both dev tools, logger, and thunk working (and a working branch of sagas).

const logger = createLogger({
  level: 'info',
  collapsed: true
})
const middleware = [reduxThunk, logger]

const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose
const enhancers = composeEnhancers(applyMiddleware(...middleware))
const store = createStore(reducers, enhancers)

angular
  // other stuff...
  .config(($ngReduxProvider) => {
    $ngReduxProvider.provideStore(store, [reduxThunk])
  })

So in this case, the ng-redux store is thunk compatible for the purpose of passing actions up to the true store, but the logger and dev tools only exist on the true store.


I'll try to make a PR to the readme to document the problem here

@derrickpelletier
Copy link
Contributor

@AntJanus My previous comment is still a relevant problem, but not the issue being discussed here.

I was digging through some commits tonight to see where this was introduced. From what I can tell, this PR introduced the bug way back in september: #166

The reason the bug never appeared until now is because ng-redux was never published to npm after the 3.5.2 release so this change didn't propagate until it ended up in 4.x.

@NadiykaS
Copy link

NadiykaS commented Apr 3, 2018

I have the same problem. Shall i use 3.5.2 version in order to get it fixed?

@derrickpelletier
Copy link
Contributor

@NadiykaS, yeah unfortunately. 4.x only introduces the provideStore functionality, so i don't think you'll miss out on too much.

@AntJanus AntJanus mentioned this issue May 2, 2018
@ProdigySim
Copy link

ProdigySim commented Jun 1, 2018

Why does the provideStore functionality have to copy the store? If it's just to add in middleware, could we use a dynamic-middleware pattern to allow ng-redux middleware to be added to the store after its creation?

@villesau
Copy link
Collaborator

villesau commented May 13, 2019

Try this approach which allows you to create the whole store in a callback: #218 I made it for different purpose but might give you enough control to sort out also this problem.

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

No branches or pull requests

8 participants