runOutsideAngular for Universal compatibility and allow advanced configuration with DI #1454

jamesdaniels · 2018-02-02T07:22:49Z

Checklist

Issue number for this PR: Dependencies for Angular Universal #1377, ApplicationRef.isStable never goes to "true" when an instance of AngularFireAuth is injected #1347, Allow us to pass a factory of FirebaseAppConfig to AngularFireModule.initializeApp() #1314, Document properly how to use multiple Firebase projects in one app #1305, Documentation Request: add guide on how to use Dependency Injection or create new providers for advanced use cases #1026
Docs included?: no
Test units included?: yes
In a clean directory, yarn install, yarn test run successfully? yes

Description

Doing some bulldozing in preparation for 5.0 final.

Components can now configure their own Firebase apps w/the FirebaseAppConfig and FirebaseAppName Injection Tokens
Support storage multi-bucket with the StorageBucket Injection Token
Support multi-database with the RealtimeDatabaseURL Injection Token
Wrap our observables in NgZone runOutsideAngular to allow operation in Angular Universal
Drop some of our bespoke typings in favor of those provided by Firebase now
Cleanup

Code sample

Coming soon...

davideast

Looks really good! My only objection is the removing of the AngularFireModule. It feels a little rough to make users do their own providers and we move away from mirroring the Firebase API.

I also think we need to go carefully through the runOutsideAngular calls. There are sprinkled here and there throughout the code and it would be nice to find a cleaner way of including them. But I don't have a great yet.

davideast · 2018-02-03T21:55:08Z

src/core/firebase.app.module.ts

-import { FirebaseFirestore } from '@firebase/firestore-types';
-
-export const FirebaseAppConfigToken = new InjectionToken<FirebaseAppConfig>('FirebaseAppConfigToken');
-


I'm not a fan of removing theAngularFireModule in place of making people do their own providers. I also like having the FirebaseApp as an injectable.

davideast · 2018-02-03T21:59:21Z

test/ng-build/ng5/src/app/app.module.ts

    AngularFireAuthModule,
    AngularFireDatabaseModule,
    AngularFirestoreModule
  ],
-  providers: [],
+  providers: [


Seeing this in action, I feel like it increases the "getting started" complexity. We should continue to mirror the Firebase API rather.

Yeah I was thinking this too. I'll bring AngularFireModule back in and allow users to use both whichever fits their use case best. Say in a specific module they need to call out to another app/database/whatnot.

patrickmichalina · 2018-02-04T01:18:41Z

Do you intend to bring in a mechanism for Auth between server and client? I currently use cookies in a working project. I didn't see code related to cookies in this branch.

jamesdaniels · 2018-02-04T01:57:32Z

@patrickmichalina I've been considering this. I don't think I'll tackle in this PR however; just want to get the basics working first + see how adoption goes.

jamesdaniels · 2018-02-04T02:03:04Z

@davideast yeah I tried to pull all the zone wrapping into a FirebaseZoneScheduler static function but was getting some binding issues all the way down in the minified grpc stuff. Only on the server too, so probably webpack doing something weird. I'll think on a cleaner pattern here BUT it works ;)

Added WS and xmlhttprequest as dependencies, as they are required to allow normal functions of the Firebase JS SDK in Universal. Created a new `angularfire/firebase-node` module, which includes these + will include any additional fixes for Universal in the future. The rest of the SSR work is taking place in #1454.

hiepxanh · 2018-02-06T13:21:37Z

thanks for your work, I'm looking to this, what was both of you did so amazing!!! 💯

davideast · 2018-02-06T22:05:19Z

src/core/firebase.app.module.ts

-
-export const FirebaseAppConfigToken = new InjectionToken<FirebaseAppConfig>('FirebaseAppConfigToken');
-
-export class FirebaseApp implements FBApp {


I would still like for us to provide the FirebaseApp token for DI. It does conflict with the SDK but this would be a breaking change to remove it.

hiepxanh · 2018-02-10T15:28:12Z

hello sir, is that any experiment api that I can test? I love to do something to help you :)

jamesdaniels · 2018-03-20T07:53:43Z

@davideast working example up FYI https://onsnapshot-flex.appspot.com/

curl https://onsnapshot-flex.appspot.com
curl https://onsnapshot-flex.appspot.com/articles/kidpCvq4bYVf5ZYVO24Y

Added WS and xmlhttprequest as dependencies, as they are required to allow normal functions of the Firebase JS SDK in Universal. Created a new `angularfire/firebase-node` module, which includes these + will include any additional fixes for Universal in the future. The rest of the SSR work is taking place in angular#1454.

…iguration with DI (angular#1454) * First. * SSR work * StorageBucket is a string... * Add AngularFireModule back in * Fix up storage and the tests * Adding StorageBucket and DatabaseURL DI tests * Adding specs for DI in auth + firestore * More complete tests + fixed the storage test config * Adding app.module back in and writing tests for app injection * Export RealtimeDatabaseURL from database-deprecated * Export FirebaseApp * Working, needs wrapper * More * State changes and auditlog * Wrap auth * Reverted database-depricated a bit * More cleanup * Cleanup unused imports

jamesdaniels added 2 commits February 1, 2018 20:53

First.

fa34c8e

SSR work

3121b94

googlebot added the cla: yes label Feb 2, 2018

StorageBucket is a string...

440b5e6

jamesdaniels mentioned this pull request Feb 2, 2018

Dependencies for Angular Universal #1377

Merged

jamesdaniels requested a review from davideast February 2, 2018 07:34

jamesdaniels mentioned this pull request Feb 2, 2018

(WIP) Latest SSR work jamesdaniels/onSnapshot#1

Merged

jamesdaniels self-assigned this Feb 2, 2018

jamesdaniels added the in progress label Feb 2, 2018

davideast requested changes Feb 3, 2018

View reviewed changes

jamesdaniels added 2 commits February 5, 2018 22:10

Add AngularFireModule back in

6e86fc7

Fix up storage and the tests

331de24

jamesdaniels changed the title ~~(WIP) SSR, providers for config, DI, and cleanup~~ Use NgZone to allow expected operation in Universal and use providers/dependency-injection for configuration Feb 6, 2018

jamesdaniels changed the title ~~Use NgZone to allow expected operation in Universal and use providers/dependency-injection for configuration~~ runOutsideAngular for Universal compatibility and allow advanced configuration with DI Feb 6, 2018

jamesdaniels removed the in progress label Feb 6, 2018

jamesdaniels added 2 commits February 5, 2018 23:51

Merge branch 'master' into providers_and_di

2700a36

Adding StorageBucket and DatabaseURL DI tests

b9cc132

wceolin mentioned this pull request Feb 6, 2018

"Unexpected token import" with angularfire2 on angular-cli, AOT, SSR, and firebase-functions #1073

Closed

Adding specs for DI in auth + firestore

63a94f9

davideast requested changes Feb 6, 2018

View reviewed changes

jamesdaniels added 4 commits February 6, 2018 14:49

More complete tests + fixed the storage test config

a77b21d

Adding app.module back in and writing tests for app injection

32ed0a5

Export RealtimeDatabaseURL from database-deprecated

345e23c

Export FirebaseApp

ee36144

GulajavaMinistudio mentioned this pull request Feb 7, 2018

Update upstream TypescriptID/angularfire2#24

Merged

jamesdaniels added 2 commits February 7, 2018 16:06

Working, needs wrapper

0c0569e

More

e68d66a

davideast mentioned this pull request Feb 9, 2018

Error: The XMLHttpRequest compatibility library was not found. #1461

Closed

davideast mentioned this pull request Feb 24, 2018

Features like Angular Universal and Cloud Messaging #1480

Closed

davideast approved these changes Mar 20, 2018

View reviewed changes

davideast and others added 3 commits March 19, 2018 21:15

Merge branch 'master' into providers_and_di

7d35667

Merge and tests

a563f3f

State changes and auditlog

5df96ef

jamesdaniels added 4 commits March 20, 2018 01:12

Wrap auth

b1be1f5

Reverted database-depricated a bit

c20d10e

More cleanup

7287f01

Cleanup unused imports

c995ee5

davideast merged commit e343f13 into master Mar 20, 2018

GulajavaMinistudio mentioned this pull request Mar 21, 2018

Update upstream TypescriptID/angularfire2#29

Merged

davideast mentioned this pull request Apr 3, 2018

Injecting AngularFireAuth to component triggers ChangeDetection continuously #1544

Closed

This was referenced Apr 23, 2018

[Firestore] Support for ES2015 transpilation #1525

Closed

angularfire2@5.0.0-rc.7.0-next Change Detection Irregularities #1583

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runOutsideAngular for Universal compatibility and allow advanced configuration with DI #1454

runOutsideAngular for Universal compatibility and allow advanced configuration with DI #1454

jamesdaniels commented Feb 2, 2018 •

edited

Loading

davideast left a comment •

edited

Loading

davideast Feb 3, 2018

davideast Feb 3, 2018

jamesdaniels Feb 4, 2018

patrickmichalina commented Feb 4, 2018

jamesdaniels commented Feb 4, 2018

jamesdaniels commented Feb 4, 2018

hiepxanh commented Feb 6, 2018 •

edited

Loading

davideast Feb 6, 2018

hiepxanh commented Feb 10, 2018

jamesdaniels commented Mar 20, 2018 •

edited

Loading

		import { FirebaseFirestore } from '@firebase/firestore-types';

		export const FirebaseAppConfigToken = new InjectionToken<FirebaseAppConfig>('FirebaseAppConfigToken');


		export const FirebaseAppConfigToken = new InjectionToken<FirebaseAppConfig>('FirebaseAppConfigToken');

		export class FirebaseApp implements FBApp {

runOutsideAngular for Universal compatibility and allow advanced configuration with DI #1454

runOutsideAngular for Universal compatibility and allow advanced configuration with DI #1454

Conversation

jamesdaniels commented Feb 2, 2018 • edited Loading

Checklist

Description

Code sample

davideast left a comment • edited Loading

Choose a reason for hiding this comment

davideast Feb 3, 2018

Choose a reason for hiding this comment

davideast Feb 3, 2018

Choose a reason for hiding this comment

jamesdaniels Feb 4, 2018

Choose a reason for hiding this comment

patrickmichalina commented Feb 4, 2018

jamesdaniels commented Feb 4, 2018

jamesdaniels commented Feb 4, 2018

hiepxanh commented Feb 6, 2018 • edited Loading

davideast Feb 6, 2018

Choose a reason for hiding this comment

hiepxanh commented Feb 10, 2018

jamesdaniels commented Mar 20, 2018 • edited Loading

jamesdaniels commented Feb 2, 2018 •

edited

Loading

davideast left a comment •

edited

Loading

hiepxanh commented Feb 6, 2018 •

edited

Loading

jamesdaniels commented Mar 20, 2018 •

edited

Loading