Skip to content

Action sequences return state in a wrong order. #149

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

Closed
pierre-hilt opened this issue Jul 6, 2016 · 9 comments · Fixed by #162
Closed

Action sequences return state in a wrong order. #149

pierre-hilt opened this issue Jul 6, 2016 · 9 comments · Fixed by #162
Assignees

Comments

@pierre-hilt
Copy link

Hello,
I have some problem with the call orders of Observable.

My case:
I have 2 components, Comp1 and Comp2.
Comp1 is a text search component.
Comp2 is a display of a text.

Comp1 and Comp2 share the same reducer, with the following state:
{ match: number, search_element: string }

Comp1 dispatches Action1 when user enter text in input. Payload is the text and updates search_element.

Comp2 subscribe to search_element. It dispatches Action2 with the number of match in the payload, updates math in the state.

Comp1 subscribe to match.

The problem is subscribe to march is called twice.
First with the value from Comp2 and second from the initial value coming from Action1.

Comp1 displays the wrong match.

I don't understand why subscribe to match is called first with "state2" and then with "state1".

I had to add another flag, search_text_updated, to update match only with state2 value.

Do you have any clue if this is a normal behaviour, for chaining actions?

I will try to create an example to reproduce the issue.

Thanks,

Pierre

@pierre-hilt
Copy link
Author

To reproduce the issue:
https://github.com/pierre-hilt/ng2-redux-issue149
In App.ts, if I change order of component it works.

With the current order, if you set a break point in Counter.ts you will be able to see that subscribe is called twice.

@e-schultz
Copy link
Member

Thanks for creating a repository that reproduces this issue, this is odd behavior. I can see the initial call to subscribe getting the correct value, then somehow the stale one gets pushed out again.

I'll try and investigate further and see if it's an issue on our end - but I agree with the intended expected behavior, and something weird is going on.

@e-schultz
Copy link
Member

@pierre-hilt Trying to wrap my head around the source of the issue - I did notice that moving the code that fires a dispatch into ngAfterViewInit instead of ngOnInit seemed to get things working as expected.

Going to dig a bit more and see if I can better understand the why of this though - and see if it's something that can be fixed at the library level, or if just with the way the various lifecycle hooks get fired in angular - that ngAfterViewInit is the more appropriate spot for it.

@e-schultz
Copy link
Member

 ngOnInit() {
        this.search$ = this.ngRedux.select(state => state.searchReducer.keyword);
    }    
    ngAfterViewInit() {

        this.search$.subscribe((keyword) => {
            if (keyword != '') {
                this.actions.fetchResultDispatch(keyword.length)
            }
        });
    }

@pierre-hilt
Copy link
Author

pierre-hilt commented Jul 8, 2016

Hello @e-schultz

Thanks a lot helping me on that!
Your proposition doesn't solve the problem. I found that my problem depends on the order of the subscribe methods.

Doing the subscribe in ngAfterViewInit, search$.subscribe is done after counter$.subscribe and there is no problem.

If you add a setTimeout around counter$.subscribe, it is done after search$.subscribe and so the bug happens.

so:
counter$.subscribe then search$.subscribe => Works fine
search$.subscribe then counter$.subscribe => Problem

More a problem of Observable library?

@pierre-hilt
Copy link
Author

Counter.ts

ngOnInit() {
    this.counter$ = this.ngRedux.select(state => state.searchReducer.total);
    window.setTimeout(() => {
      this.counter$.subscribe(state =>
        this.counter = state)
    });
  }

CounterInfo.ts

ngOnInit() {
        this.search$ = this.ngRedux.select(state => state.searchReducer.keyword);
    }
    ngAfterViewInit() {
        this.search$.subscribe((keyword) => {
            if (keyword != "") {
                this.actions.fetchResultDispatch(keyword.length)
            }
        });
    }

@e-schultz
Copy link
Member

Just adding this to help debug the problem. Simplifying things to just be one component.

<p>
    Counter: {{ counter }} <br/> 
    Counter$ Async: {{ counter$ | async }} <br/>
    <input id='search-input' type="text" class="search"
    [(ngModel)]="keyword" (ngModelChange)="searchKeyword()"/>
  </p>

search$ subscribed before $counter - counter in template displays correct value, counter$ displays wrong value.

this.counter$ = this.ngRedux.select(state => state.searchReducer.total);
    this.search$ = this.ngRedux.select(state => state.searchReducer.keyword);

    this.search$.subscribe((keyword) => {
      if (keyword != '') {
        this.actions.fetchResultDispatch(keyword.length)
      }
    });

    this.counter$.subscribe(state => {

      this.counter = state;
    });

search$ subscribed before $counter - counter in template displays correct value, counter$ displays wrong value.

this.counter$ = this.ngRedux.select(state => state.searchReducer.total);
this.search$ = this.ngRedux.select(state => state.searchReducer.keyword);

 this.counter$.subscribe(state => {

      this.counter = state;
    });

 this.search$.subscribe((keyword) => {
      if (keyword != '') {
        this.actions.fetchResultDispatch(keyword.length)
      }
    });

counter$ subscribed before $search - both template values display wrong value.

When using the redux dev tools, can see that the state has the correct value, also when looking at the logging - nextState is correct. However, for some reason the .select is getting re-triggered with the previous value.

.... weird.

e-schultz added a commit to e-schultz/ng2-redux that referenced this issue Jul 8, 2016
e-schultz added a commit to e-schultz/ng2-redux that referenced this issue Jul 12, 2016
e-schultz added a commit to e-schultz/ng2-redux that referenced this issue Jul 14, 2016
* fix angular-redux#149 - chained actions should not get stale state,
  changed to create an observable from redux since redux 3.4x supports
  an observable shim
* fix angular-redux#138 - ability to use select decorators in service
  Changing to use redux's observable, had to change how we created the
  initial observable. Use switchMap to switch streams once the store
  observable becomes available
@e-schultz
Copy link
Member

e-schultz commented Jul 14, 2016

@pierre-hilt I just published 3.2.1-beta.1 which should address the issue. Going to do a bit more testing before doing a non-beta release

@pierre-hilt
Copy link
Author

Hey, OK thanks a lot. I will test it on Monday and let you know if I find any bug.

e-schultz added a commit to e-schultz/ng2-redux that referenced this issue Jul 21, 2016
* fix angular-redux#149 - chained actions should not get stale state,
  changed to create an observable from redux since redux 3.4x supports
  an observable shim
* fix angular-redux#138 - ability to use select decorators in service
  Changing to use redux's observable, had to change how we created the
  initial observable. Use switchMap to switch streams once the store
  observable becomes available
e-schultz added a commit that referenced this issue Jul 21, 2016
* 3.3.0

* Features
 * DevToolsExtension - convience wrapper for dev tools (#115)
 * Select - seamless support for ImmutableJS (#160)

* Fixes
 * Able to use `@select` in services
 * Behavior of `select` with chained dispatches, (fixes #149, #153)
@e-schultz e-schultz self-assigned this Jul 21, 2016
e-schultz added a commit that referenced this issue Jul 21, 2016
* 3.3.0

* Features
 * DevToolsExtension - convience wrapper for dev tools (#115)
 * Select - seamless support for ImmutableJS (#160)

* Fixes
 * Able to use `@select` in services
 * Behavior of `select` with chained dispatches, (fixes #149, #153)
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

Successfully merging a pull request may close this issue.

2 participants