Bug: Observable returned from AngularFireFunctions does not trigger change detection

heddendorp commented 4 years ago

Version info

Angular CLI: 9.1.1
Node: 12.12.0
OS: win32 x64

Angular: 9.1.2
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.7
@angular-devkit/build-angular     0.901.1
@angular-devkit/build-optimizer   0.901.1
@angular-devkit/build-webpack     0.901.1
@angular-devkit/core              9.1.1
@angular-devkit/schematics        9.1.1
@angular/cdk                      9.2.1
@angular/cli                      9.1.1
@angular/fire                     6.0.0
@angular/flex-layout              9.0.0-beta.29
@angular/material                 9.2.1
@ngtools/webpack                  9.1.1
@schematics/angular               9.1.1
@schematics/update                0.901.1
rxjs                              6.5.5
typescript                        3.8.3
webpack                           4.42.0

How to reproduce these conditions

Failing test unit, Plunkr, or JSFiddle demonstrating the problem I deployed the issue here https://mc-changelogger.web.app/

Steps to set up and reproduce Continue using a functions observable like here https://github.com/Isigiel/changelogger2/blob/master/src/app/services/technic-api.service.ts#L29

To reproduce: Open https://mc-changelogger.web.app/ Enter http://api.technicpack.net/modpack/shivaxi-rlcraft and wait for 10 seconds Observe how the progress bar keeps going and there is no error displayed If you now trigger change detection by clicking on the button or on another step, you will see that the UI updates suddenly happen

Sample data and security rules

Debug output

Errors in the JavaScript console

Output from firebase.database().enableLogging(true);

No relevant output

Expected behavior

I expect any observable to trigger angular change detection.

Actual behavior

I'm assuming due to https://github.com/angular/angularfire/blob/master/src/functions/functions.ts#L43 the observables returned by the functions are running outside of the angular zone and thus not triggering changeDetcteion properly.

Suggested Fix

Make sure the Observable actually run inside angular or at least state very explicitly that they have very unexpected behavior and provide a workaround for people who want to use them.

Hint

Although I'm not sure about the current situation I have seen similar behavior with the authentication part of this library

mmorandini commented 4 years ago

Hi, I am having the exact same issue here. Having to call changeDetectorRef.detectChanges() to force change detection;

Surely this cannot be the expected behavior?

39ro commented 4 years ago

Defining the handler of the callable as asynchronous at the moment doesn't trigger the change detection, removing the async keyword resolve the issue in the following scenario:

// not working
exports.hello = functions.https.onCall(async (data, context) => {
    return 'hello world';
});
// working
exports.hello = functions.https.onCall((data, context) => {
    return 'hello world';
});

// component.ts
  constructor(private fns: AngularFireFunctions) {
  }

  doSomething() {
    this.loading = true;
    const callable = this.fns.httpsCallable('hello');
    callable({name: 'some-data'})
      .pipe(finalize(() => {
        console.log('finalize');
        this.loading = false;
      }))
      .subscribe(
        res => console.log(res),
        error => console.log(error)
      );
  }

k- commented 4 years ago

As someone who's new to Angular, I'm not really sure how to work around this bug elegantly. I'm trying to avoid the subscribe(val=> this.val = val) pattern - is there a way to use changeDetectorRef.detectChanges() to cause the Observable from httpsCallable to trigger changes when using | async in the view?

mmorandini commented 4 years ago

I am not a pro myself, but I think that calling changeDetectorRef.detectChanges() inside the subscribe() should work in your case.

E.G.:

export class MyComponent() {
 foo$: Observable<any>;

  constructor(aff: AngularFireFunctions, cdr: ChangeDetectorRef) {
    const myRequest = this.aff.httpsCallable('foo-bar-function');
    this.foo$ = myRequest({someData})
    this.foo$.subscribe(() => this.cdr.detectChanges())
  }
}

jcpham81 commented 4 years ago

Same issue here

k- commented 4 years ago

I've settled on a simple enough workaround based on the comment from @mmorandini manually call detectChanges in a finalize.

someObservable: Observable<Type> = 
    this.fireFunctions.httpsCallable('funcName')
        ({..})
            .pipe(finalize(() => 
                    this.changeDetectorRef.detectChanges()));

Then, you can use {someObservable | async} in your view.

Hopefully the root issue will be fixed, and to revert the workaround, simply remove the finalize pipe.