angular-redux / store

Angular 2+ bindings for Redux
MIT License
1.34k stars 202 forks source link

Observable doesn't update view after ng2-redux update #259

Closed AntJanus closed 7 years ago

AntJanus commented 8 years ago

I have a pretty simple app that I'm running and whenever the state updates, the view doesn't reflect that. Using Angular 2.2.0 and ng2-redux 4.1.0

Here's what I'm doing:

I have a select in my app.component.ts file

I'm using the async pipe in my template to display the raw data in app.html and every single time it comes back as empty.

I'm intercepting the actions and the reducers to console log wtf is going on. The actions run, the new state gets returned but the view doesn't update. I originally had this project running 4.0.0-beta.0 with angular 2.0.0 and that worked flawlessly. After update, doesn't work.

SethDavenport commented 8 years ago

A few people have had issues when they updated Angular 2 but didn't update zonejs as well. What version of zonejs are you using?

AntJanus commented 8 years ago

I'm using 0.6.26. Maybe I'm initializing it wrong. In my main.ts (app entry), I'm doing the following:

require('core-js/shim')
require('zone.js')

(my local machine now uses imports).

I copied the vendor.ts file from the counter project but that didn't help anything.

SethDavenport commented 8 years ago

let me see if I can repro ... I haven't gotten around to trying ng 2.2 yet

e-schultz commented 8 years ago

@AntJanus tried running your project to see if I could find out what's going on, but when running npm start I'm getting an error with electron.

eateCompilerHostFromConfigFileSync (/Users/evan/Github/e-schultz/omen/node_modules/electron-compile/lib/config-parser.js:361:19)
    at createCompilerHostFromProjectRootSync (/Users/evan/Github/e-schultz/omen/node_modules/electron-compile/lib/config-parser.js:381:12)
    at init (/Users/evan/Github/e-schultz/omen/node_modules/electron-compile/lib/config-parser.js:274:22)
    at main (/Users/evan/Github/e-schultz/omen/node_modules/electron-prebuilt-compile/lib/es6-init.js:38:29)
    at Object.<anonymous> (/Users/evan/Github/e-schultz/omen/node_modules/electron-prebuilt-compile/lib/es6-init.js:41:1)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)

any ideas?

scarrier commented 8 years ago

I'm having the same issue with the view not updating. I tried updating the counter project to 2.2.0 and it still was working fine, so I don't think it's just angular. I did get it working with the following style workaround. It just subscribes to the @select observable and updates a local object. Not an ideal solution, but it works.

@select('user') user: Observable<User>;
myUser: User;

ngOnInit() {
    this.user.subscribe(u => { this.myUser = u; } );
}
AntJanus commented 8 years ago

@e-schultz I'll look into that. I always use my globally installed electron package so I probably messed up there. It should run with npm run watch and electron . but obviously, I'll fix it.

@scarrier I'll try that out and let you know if that works!

AntJanus commented 8 years ago

@e-schultz fixed it. npm start will run the app (after npm install)

AntJanus commented 8 years ago

@scarrier I've no idea what I'm doing wrong but that workaround doesn't fix it. When I log out the contents of the subscribe function, I get the stream updates as they come in but the view just won't update.

scarrier commented 8 years ago

Did you update your template to use the local copy of the object? In my case the template looks like

<div> Welcome, {{myUser.username}} <div>

Just doing standard template binding, not using the async pipe on the Observable. If you see the updates coming in the subscribe, and you are updating your local object with the new value, then that should be reflected in your view by standard angular binding. If not, there might be something else going on.

SethDavenport commented 8 years ago

@AntJanus are you using regular or on_push change detection?

SethDavenport commented 8 years ago

What you describe is that the actions and reducers are firing and the state is updated, but Angular doesn't refresh its view right?

If so that sounds like a classic ZoneJS or change detection related issue.

If you're willing to share your repo I can take a look this weekend.

In the meantime some more questions:

SethDavenport commented 8 years ago

Ok cool - just saw the link above, and that it's Electron. Will take a look soon.

AntJanus commented 8 years ago

I just wanted to quickly mention that I'm really appreciative for all the help everyone is providing! :)

@scarrier updated it. Using | async throws an error so I just did a direct binding. Still didn't work.

@SethDavenport thanks for taking a look! I didn't change the change detection strategy but yes: actions + reducers run, state is updated, even .subscribe gets run but the template doesn't update. It could definitely be a ZoneJS error but I'm not sure how to troubleshoot that. I'm still a n00b :)

As far as your questions, I'm not using any 3rd-party libraries to interact with NgRedux, there's no middleware, and I created the app from scratch.

Thanks for taking a look!

You can easily recreate the issue by trying to open one of the files from the sidebar (directories don't work yet!) once the electron app loads up. If you check the console, you'll see the output of the IPC connection (you'll get a response from the main thread with all the data). I'm not sure if I pushed this but on the main content area (so, not the sidebar) you should see an empty {} which is supposed to be a loaded file. Once you click on a file in the sidebar (like CHANGELOG.md), the empty curly braces should change and the entire page should display an editor but it doesn't.

SethDavenport commented 8 years ago

By the way, very cool to see someone using ng2-redux with Electron. Haven't played with Electron much but it seems pretty awesome.

I cloned your repo and did some experimenting. You can see the results here: https://github.com/AntJanus/omen/pull/4

As I suspected, it is related to dispatches happening outside the Angular zone. Basically what's happening is that a lot of your calls to dispatch are called from callbacks to Electron's ipcRenderer.

The way Angular2 works is that ZoneJS basically monkeypatches as many async APIs as it knows about in order to bring them into a setup where Angular knows that async stuff has 'settled'. At this point Angular2 fires its change detector and the UI updates.

In your case, the asynchrony is coming from an Electron API that Angular doesn't know about. This means that whatever dispatches are called from electron callbacks will fire, and run reducers, etc; but Angular2 doesn't know it has to update its UI.

My pr basically wraps ngRedux.dispatch in a function that checks whether it's in-zone or not; and if not it calls a special hook to do the dispatch in-zone instead.

SethDavenport commented 8 years ago

This is actually a general with problem integrating callback-driven third-party APIs with Angular2 - think of it as analogous to the old ng1 digest cycle stuff: a cleaner, better implementation, but the same fundamental issue when faced with callbacks.

https://github.com/angular/angular/issues/7154

@e-schultz this has come up a few times with different libraries - Electron, SignalR, etc. I'm wondering if we should take my wrapped dispatch function in AntJanus/omen#4 and make NgRedux.dispatch work that way by default? It's definitely part of 'bridging the gap' between 3rd-parties and Angular2, which is part of our mission.

AntJanus commented 8 years ago

@SethDavenport I had no idea this could be a problem! It sounds exactly like a $digest problem but I see the difference. Thanks for making the fix. I'll have to look more into how Zone works because I'll be migrating the IPC API to use the electron remote API but that wraps everything in Promises so I wonder if that'll change things.

The weird thing is that Angular 2.0 and an older version of ng2-redux worked flawlessly. I'm using that editor to write NaNoWriMo so I got good 20K words written in it and 20-30 files created/saved using this app.

Thanks for all the help again! Definitely makes me understand Angular 2 much more :)

e-schultz commented 8 years ago

@SethDavenport I've thought about doing a check to see if in the angular zone or not - and doing a zone.run.

I'm just thinking of times of which people might be explicitly not wanting to run things in a zone, basically if someone has intentionally done a ngZone. runOutsideAngular(()=>{}) because they want finer control over when CD kicks in - would this be a problem?

It seems like it'd be the less common scenario - but if someone is wanting that level of control over when to trigger CD/etc, wouldn't want to take that away.

The more common scenario though is, 'something I don't know is kicking me out of the zone for some reason' - and would be nice to have a better buffer against that.

AntJanus commented 8 years ago

@e-schultz I think it makes sense to not patch it. If Zone.js or Angular 2 add support for it, that's great; however, they might have a reason against it. It doesn't make sense to override it in this one very specific instance.

SethDavenport commented 8 years ago

@e-schultz @AntJanus I guess what I'm saying is that it's not that specific: it comes up pretty frequently any time someone uses a third party lib with a callback.

I think people expect it to just work (TM), and there's a pretty simple solution. Also, we compare unfavourably to ngrx/store here (to be verified); my bet is that ngrx's dispatch automatically ends up in-zone because they implement dispatch as an Observable operator (I'm guessing Observable is one of the things zonejs knows to monkeypatch).

For the case of 'out of zone' updates @mhazy and I were arguing about that the other day. Personally I think it's "non-redux" in the sense that now your UI is no longer a pure function of your action stream over time.

However we don't want to restrict people if they really want to do it. I had suggested at the time making zone enforcement the default, but also providing an 'escape hatch':

dispatch(action: Action, runOutsideAngular = false)

or similar. Of course this messes with the Redux API in a way that's not friendly to middlewares. Maybe instead

dispatch(action: Action) { ... }

// Are you sure you need this?
dispatchOutsideAngular(action: Action) { ... }
SethDavenport commented 7 years ago

Fix coming in 5.1

anton-107 commented 7 years ago

hi guys, i'm having this issues (observables not emitting new events) with the following dependencies:

"dependencies": {
    "@angular/common": "^2.3.1",
    "@angular/compiler": "^2.3.1",
    "@angular/core": "^2.3.1",
    "@angular/forms": "^2.3.1",
    "@angular/http": "^2.3.1",
    "@angular/material": "^2.0.0-beta.1",
    "@angular/platform-browser": "^2.3.1",
    "@angular/platform-browser-dynamic": "^2.3.1",
    "@angular/router": "^3.3.1",
    "core-js": "^2.4.1",
    "ng2-redux": "^5.1.2",
    "redux": "^3.6.0",
    "redux-logger": "^2.8.1",
    "rxjs": "^5.0.1",
    "ts-helpers": "^1.1.1",
    "zone.js": "^0.7.2"
  }