Skip to content

Version 5.4.2 breaks Angular Universal server rendering #2341

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
jeffwhelpley opened this issue Mar 2, 2020 · 5 comments
Closed

Version 5.4.2 breaks Angular Universal server rendering #2341

jeffwhelpley opened this issue Mar 2, 2020 · 5 comments

Comments

@jeffwhelpley
Copy link

jeffwhelpley commented Mar 2, 2020

Version info

Angular:
9.0.4

Firebase:
7.9.3

AngularFire:
5.4.2

Other (e.g. Ionic/Cordova, Node, browser, operating system):

How to reproduce these conditions

Have an Angular Universal app that does an AngularFire query on page load. When you do this, you will see that isStable from the Angular ApplicationRef is immediately set to true at the moment of the database call.

This is a problem because it means the server response is sent back to the client BEFORE the data from the database is actually used to update the page (i.e. the server view does not contain the data even though it is returned).

This issue does not exist with Firebase 7.6.2 and AngularFire 5.3.0. It only exists with Fireabase 7.9.3 and AngularFire 5.4.2.

Failing test unit, Plunkr, or JSFiddle demonstrating the problem

N/A - I have this issue on a private repo, but have tested enough to confirm it almost certainly is an issue for anyone with this setup.

Steps to set up and reproduce

Do the following:

  1. Create an Angular Universal app with Angular 9+ and AngularFire 7.9.3.
  2. In the ngOnInit() page lifecycle hook of any page, perform an AngularFire query. It can be any query
  3. Have a console.log() right after the AngularFire query completes and data is returned
  4. Add this code to the constructor of the page:
constructor(ref: ApplicationRef) {
  ref.isStable(isStable => console.log('isStable = ' + isStable);
}

When you do this and load the app, you will see that isStable is set to true BEFORE the console.log from step 3 is executed. This means the server view is returned before we have a chance to do anything with the data coming back from AngularFire.

Sample data and security rules

N/A - issue not related to data or security rules

Debug output

** Errors in the JavaScript console **

N/A - there is no error, it is just that the data isn't included in the server view

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

N/A - related to firestore, not the realtime database

** Screenshots **

N/A - this is not a visual issue

Expected behavior

The expected behavior is that the Angular ApplicationRef.isStable is not set to true until after all async is complete AND the sync code that immediately follows the async code is complete.

Actual behavior

The actual behavior is that immediately when the AngularFire query is run, isStable is set to true and the server response immediately returns.

@jeffwhelpley
Copy link
Author

Note that while I don't see any other Angular Universal specific issues already entered, I do see a few zone related issues with the latest AngularFire. I wonder if those zone issues are also causing this problem.

@jamesdaniels
Copy link
Member

Thanks for the report!

We did push some major changes to our Zone handling in 5.4 (#2294) and TBH most of my testing has been focused on Firestore rather than the RTDB. I'll do some more testing here.

@jamesdaniels
Copy link
Member

Please do write up on the Zone issues you are encountering. Want to make sure I squash them all since I'm working on Universal support in v6 right now.

@jamesdaniels
Copy link
Member

I've managed to reproduce this problem. I have a work around in place in #2347 (demonstrated here) and will be investigating a better solution. It seems there's a race condition between our resolving our macrotask and the results getting to your subscriber. Perhaps this was brought in when we went with source.lift or "better" queueing; rather than just "wrapping" the source observable.

I probably had some other macrotask that was hiding this in my Angular 8 test project, once I upgrade to Angular 9 I see it; presumably because it's faster.

Additionally, I've decided in version 6 to include the macrotask in non-server builds, this may mean some additional pain in the short term as we iron out the remaining Zone.js issues but at least folk won't be surprised when they go SSR.

@jamesdaniels
Copy link
Member

Closing as we've addressed all the zone issues I'm aware of at the moment.

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

No branches or pull requests

2 participants