troyanskiy / ngx-resource

Resource (REST) Client for Angular 2
http://troyanskiy.github.io/ngx-resource/
200 stars 46 forks source link

Switching to ResourceCRUDObservable 2x requests #153

Closed httpete closed 5 years ago

httpete commented 5 years ago

Hi,

In my early testing, if I use the CrudObservable requests get fired 2X, if I Switch back to Promises, it does it once. I switched back to CRUDPromise, and it worked as expected and fired one call.

troyanskiy commented 5 years ago

Hello @httpete I can't reproduce the issue. How to reproduce it? Did you subscribe twice to the observable?

Could you please provide more info and/or code samples?

I did some tests with ResourceCRUDObservable in the develop branch which does query and get requests.

You can clone and test it.

git clone --single-branch --branch develop https://github.com/troyanskiy/ngx-resource.git
cd ngx-resource
npm i
npm run core.build
npm run http.build
ng serve

// to run test server in other terminal
cd server
npm i
node index.js
httpete commented 5 years ago

Hi, Thank you again for this debugging - So maybe this is a part of expectation, I was hoping to simply change my code from: //from is rxjs { from } from(this.ngxResource.query(q)); and rip off the from and it would work the same way. I only subscribe once, and I verified I am only calling it once.If I convert to ResourceCRUDObservable and remove the from, I See the double request.

On Saturday, August 10, 2019, 06:13:52 AM EDT, Raman Rasliuk <notifications@github.com> wrote:  

Hello @httpete I can't reproduce the issue. How to reproduce it? Did you subscribe twice to the observable?

Could you please provide more info and/or code samples?

I did some tests with ResourceCRUDObservable in the develop branch which does query and get requests.

You can clone and test it. git clone --single-branch --branch develop https://github.com/troyanskiy/ngx-resource.git cd ngx-resource npm i npm run core.build npm run http.build ng serve

// to run test server in other terminal cd server npm i node index.js — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

httpete commented 5 years ago

I see other people experiencing something similar:

https://github.com/johannesjo/angular2-promise-buttons/issues/15

I am calling subscribe() and that is all. I expect one response upon subscribing. Maybe this has something to do with hot or cold obs as they say.

troyanskiy commented 5 years ago

Can you provide a piece of code where you use it?

httpete commented 5 years ago
import { Component, Input, OnChanges, OnInit } from '@angular/core';
import { I18NextPipe } from 'angular-i18next';
import { from, Observable, zip } from 'rxjs';
import { concatMap, filter, map, toArray } from 'rxjs/operators';
import { ActionQuery, ActionResource } from '../../services/actions.service';

@Component({
  selector: 'actions',
  templateUrl: './actions.component.html',
  styleUrls: ['./actions.component.scss'],
})
export class ActionsComponent implements OnInit, OnChanges {
  @Input() public id: string = '';

  public titles$;
  public suggested$;
  public progress$;
  public isEmpty: Observable<boolean>;

  constructor(public actionService: ActionResource) {}

  public ngOnInit() {}

  public ngOnChanges() {
    this.query();
  }

  /* Fetch the data */
  public query() {
    /* I fetch, and filter into two collections */
    let q: IManagementActionQuery = {};
    if (this.clusterId) {
      q.cluster_id = this.id;
    }
    const obs$ = from(this.service.query(q));

    let emitArray = obs$.pipe(concatMap((value) => from(value)));
    this.suggested$ = emitArray.pipe(
      filter((action) => action.event_remediation_status == 'status-1'),
      toArray(),
    );
    this.progress$ = emitArray.pipe(
      filter((action) => action.event_remediation_status == 'status-2'),
      toArray(),
    );

}
httpete commented 5 years ago

I have made this bare min stackblitz, which shows test.json getting fired twice. It is nothing but an empty ngx-resource with Observables and query(), sent to the async pipe in the template.

https://stackblitz.com/edit/angular-3u6xp5

troyanskiy commented 5 years ago

Hi. I see only one request in the stackblitz example. The other one is related to the service worker. If you set flag Bypass for network you will see only one request. image

Also in your previews example you have 2 observables finally which are subscribed by async pipe in your html template i guess.

httpete commented 5 years ago

OK you are right - this is just a matter of newness. We whipped up an example with regular angular

https://stackblitz.com/edit/angular-6ynhqw

And it shows the same behavior. We worked so hard in the past to eliminate double requests , but things have changed with Observables. Thank you Roman.

troyanskiy commented 5 years ago

@httpete

Here is a solution for you to avoid multiple calls

/* Fetch the data */
public query() {
    /* I fetch, and filter into two collections */
    let q: IManagementActionQuery = {};
    if (this.clusterId) {
      q.cluster_id = this.id;
    }
    // instead of that
    // const obs$ = from(this.service.query(q));

    // Use ReplaySubject
    const obs$ = new ReplaySubject();
    this.service.query(q).subscribe(obs$);

    let emitArray = obs$.pipe(concatMap((value) => from(value)));
    this.suggested$ = emitArray.pipe(
      filter((action) => action.event_remediation_status == 'status-1'),
      toArray(),
    );
    this.progress$ = emitArray.pipe(
      filter((action) => action.event_remediation_status == 'status-2'),
      toArray(),
    );

}
httpete commented 5 years ago

But is the second call I am seeing bad? Maybe I just need to get used to it?

troyanskiy commented 5 years ago

Well... It depends on the task. But I would not recommend to get used to extra same server requests. Better to avoid extra requests. If you get used to that at some moment your server will receive tonnes of requests.

httpete commented 5 years ago

I just don't know that it actually IS a server request - this whole notion might be new, a web worker.. We are investigating. I hope this thread will help others, who move from Promise to Observable.