Skip to content
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

Dependencies for Angular Universal #1377

Merged
merged 4 commits into from
Feb 6, 2018
Merged

Dependencies for Angular Universal #1377

merged 4 commits into from
Feb 6, 2018

Conversation

jamesdaniels
Copy link
Member

@jamesdaniels jamesdaniels commented Dec 8, 2017

Checklist

Description

Docs coming but this should get the basics working on Angular Universal.

Just make sure Firebase is at least 4.8.0, install peer deps, and include import "angularfire2/firebase-node"; in your server.ts file.

What changed? How does this work?

  1. Faye's websocket implementation wasn't working in Node, so RTDB wasn't functioning, WS seems to work fine. Need to expose this globally in your server.ts.
  2. Auth needs xmlhttprequest, this is already a peer of firebase, but needs to be exposed globally in server.ts.
  3. Firestore problems were actually this bug with the firebase-js-sdk where this was the wrapper if there was no window, rather than global, causing all kinds of tail-chasing problems in minified grpc code.
  4. lots of other little fixes to the JS SDK in the meantime (node entries, etc.)

Tada!

I'll do some more testing and fix bugs that might be disrupting things like app.isStable which is affecting preboot, etc. in #1454

Code sample

import "angularfire2/firebase-node";

More coming soon...

@patrickmichalina
Copy link

Disrupting app.isStable is not good. It will cause StateTransfer to not work properly FYI.

@patrickmichalina
Copy link

Also, how is this transferring data between server and client to avoid reflow once SPA takes over?

@jamesdaniels
Copy link
Member Author

jamesdaniels commented Dec 8, 2017

@patrickmichalina yeah, need to get the isStable fixes in before tackling state transfer + I'm not sure how many of those bugs have stuck around in Angular 5. From what I was told a couple of macrostate problems I was having in 4.x were solved + it seemed much better on my fresh Angular CLI project. Will have to update my larger test project.

Ultimately it's going to come down to tinkering with Zone + getting it playing nice with the websocket / grpc streams. Simply hitting it with some runOutsideOfAngular should help, e.g, zone.runOutsideOfAngular(() => ref.onSnapshot(snap => zone.run(() => subscriber.next(snap))))... but with the current class/directory structure there's going to be a bit of shuffling around to get the @inject's in the right spots.

Anyway, this is a needed first step!

@FrozenPandaz
Copy link
Contributor

Hey All,

May I be of any assistance here?

I may be able to suggest a solution to the TransferState integration:

  • The HttpInterceptors API is perfect for it
    • Something Similar to a redux reducer might be able to help here
  • The alternative is to wrap AngularFirestore...

@hiepxanh
Copy link
Contributor

hiepxanh commented Jan 3, 2018

Yahoo @FrozenPandaz is coming,
what about ZoneMacroTaskWrapper sir? is that will be use?

with transferState, I still not understand, can you explain what will you do with that integration, sir?

@patrickmichalina
Copy link

patrickmichalina commented Jan 3, 2018

My current solution in a project is to use (far from elegant, but it works for now..):

import { makeStateKey, TransferState } from '@angular/platform-browser'
import { Injectable, InjectionToken, NgZone } from '@angular/core'
import { fromPromise } from 'rxjs/observable/fromPromise'
import { fbAdminDb } from './server'
import { database } from 'firebase-admin'

export const FIREBASE_ADMIN_INSTANCE = new InjectionToken<string>('app.fb.admin')

@Injectable()
export class FirebaseAdminService {
  constructor(private zone: NgZone, private ts: TransferState) { }

  public object(path: any, query?: any) {
    return {
      valueChanges: () => {
        const timeout = setTimeout(() => undefined, 10000)
        return fromPromise(new Promise<any>((resolve, reject) => {
          this.zone.runOutsideAngular(() => { // Out of Angular's zone so it doesn't wait for the neverending socket connection to end.
            const ref = this.applyQuery(fbAdminDb.ref(path), query)
            ref.once('value').then((data: any) => {
              this.zone.run(() => { // Back in Angular's zone
                this.ts.set(makeStateKey<string>(`FB.${path}`), data.val())
                resolve(data.val())
                setTimeout(() => {
                  // Maybe getting the data will result in more components to the view that need related data.
                  // 20ms should be enough for those components to init and ask for more data.
                  clearTimeout(timeout)
                }, 20)
              })
            }, (err: any) => {
              setTimeout(() => {
                this.ts.set(makeStateKey<string>(`FB.${path}`), undefined)
                reject(err)
                clearTimeout(timeout)
              }, 20)
            })
          })
        }))
      },
      snapshotChanges: () => {
        const timeout = setTimeout(() => undefined, 10000)
        return fromPromise(new Promise<any>((resolve, reject) => {
          this.zone.runOutsideAngular(() => { // Out of Angular's zone so it doesn't wait for the neverending socket connection to end.
            fbAdminDb.ref(path).once('value').then((snapshot: database.DataSnapshot) => {
              this.zone.run(() => { // Back in Angular's zone
                const res = snapshot.val()
                // const projected = Object.keys(res || {}).map(key => ({ ...res[key], id: key }))
                this.ts.set(makeStateKey<string>(`FB.${path}`), res)
                resolve(res)
                setTimeout(() => {
                  // Maybe getting the data will result in more components to the view that need related data.
                  // 20ms should be enough for those components to init and ask for more data.
                  clearTimeout(timeout)
                }, 20)
              })
            }, (err: any) => {
              setTimeout(() => {
                this.ts.set(makeStateKey<string>(`FB.${path}`), undefined)
                reject(err)
                clearTimeout(timeout)
              }, 20)
            })
          })
        }))
      }
    }
  }

  public list(path: any, query?: any) {
    return {
      valueChanges: () => {
        const timeout = setTimeout(() => undefined, 10000)
        return fromPromise(new Promise<any>((resolve, reject) => {
          this.zone.runOutsideAngular(() => { // Out of Angular's zone so it doesn't wait for the neverending socket connection to end.
            query(fbAdminDb.ref(path)).once('value').then((snapshot: database.DataSnapshot) => {
              this.zone.run(() => { // Back in Angular's zone
                const res = snapshot.val()
                const projected = Object.keys(res || {}).map(key => res[key])
                this.ts.set(makeStateKey<string>(`FB.${path}`), projected)
                resolve(projected)
                setTimeout(() => {
                  // Maybe getting the data will result in more components to the view that need related data.
                  // 20ms should be enough for those components to init and ask for more data.
                  clearTimeout(timeout)
                }, 20)
              })
            }, (err: any) => {
              setTimeout(() => {
                this.ts.set(makeStateKey<string>(`FB.${path}`), undefined)
                reject(err)
                clearTimeout(timeout)
              }, 20)
            })
          })
        }))
      },
      snapshotChanges: () => {
        const timeout = setTimeout(() => undefined, 10000)
        return fromPromise(new Promise<any>((resolve, reject) => {
          this.zone.runOutsideAngular(() => { // Out of Angular's zone so it doesn't wait for the neverending socket connection to end.
            query(fbAdminDb.ref(path)).once('value').then((snapshot: database.DataSnapshot) => {
              this.zone.run(() => { // Back in Angular's zone
                const res = snapshot.val()
                const projected = Object.keys(res || {}).map(key => ({ ...res[key], id: key }))
                this.ts.set(makeStateKey<string>(`FB.${path}`), projected)
                resolve(projected)
                setTimeout(() => {
                  // Maybe getting the data will result in more components to the view that need related data.
                  // 20ms should be enough for those components to init and ask for more data.
                  clearTimeout(timeout)
                }, 20)
              })
            }, (err: any) => {
              setTimeout(() => {
                this.ts.set(makeStateKey<string>(`FB.${path}`), undefined)
                reject(err)
                clearTimeout(timeout)
              }, 20)
            })
          })
        }))
      }
    }
  }

  applyQuery(ref: any, query: any) {
    // tslint:disable-next-line:forin
    for (const n in query) {
      const val = query[n].getValue
        ? query[n].getValue() // BehaviorSubject
        : query[n] // Primitive
      switch (n) {
        case 'orderByKey':
          ref = ref.orderByKey()
          break
        default:
          if (!(n in ref)) {
            break
          }
          ref = ref[n](val)
          break
      }
    }
    return ref
  }
}

Then on the client I unwrap using:

import { PlatformService } from './platform.service'
import { Observable } from 'rxjs/Observable'
import { Injectable } from '@angular/core'
import { AngularFireDatabase } from 'angularfire2/database'
import { makeStateKey, TransferState } from '@angular/platform-browser'
import { database } from 'firebase'
import { PathReference } from 'angularfire2/database/interfaces'
import { sha1 } from 'object-hash'

@Injectable()
export class FirebaseDatabaseService {
  constructor(private db: AngularFireDatabase, private ts: TransferState, private ps: PlatformService) { }

  usedCache: { [key: string]: boolean } = {}

  checksum(value: any) {
    return {
      value,
      checksum: sha1(value || '')
    }
  }

  dedupe = () => <T>(source: Observable<T>) => source
    .map(value => this.checksum(value))
    .distinctUntilKeyChanged('checksum')
    .map(a => a.value)

  dedupeCached = (cached: any, ckey: any) => <T>(source: Observable<T>) => source
    .map(value => this.checksum(value))
    .startWith(this.checksum(cached) as any)
    .do(() => this.usedCache[ckey] = true)
    .distinctUntilKeyChanged('checksum')
    .map(a => a.value)

  get<T>(path: string) {
    const ckey = this.cacheKey(path)
    const cached = this.ts.get<T | undefined>(ckey, undefined)

    return cached && !this.usedCache[ckey]
      ? this.db.object(path).valueChanges<T>().pipe(this.dedupeCached(cached, ckey))
      : this.db.object(path).valueChanges<T>().pipe(this.dedupe())
  }

  getList<T>(path: PathReference, queryFn?: (ref: database.Reference) => database.Query): Observable<T[]> {
    const ckey = this.cacheKey(path.toString())
    const cached = this.ts.get<T[] | undefined>(ckey, undefined)

    return cached && !this.usedCache[ckey]
      ? this.db.list(path, queryFn).valueChanges<T>().pipe(this.dedupeCached(cached, ckey))
      : this.db.list(path, queryFn).valueChanges<T>().pipe(this.dedupe())
  }

  getListKeyed<T>(path: PathReference, queryFn?: (ref: database.Reference) => database.Query): Observable<T[]> {
    const ckey = this.cacheKey(path.toString())
    const cached = this.ts.get<T[] | undefined>(ckey, undefined)

    return cached && !this.usedCache[ckey]
      ? this.db.list(path, queryFn)
        .snapshotChanges()
        .map(a => this.keyMapper(a))
        .pipe(this.dedupeCached(cached, ckey))
      : this.db.list(path, queryFn)
        .snapshotChanges()
        .map(a => this.keyMapper(a))
        .pipe(this.dedupe())
  }

  keyMapper(res: any) {
    if (this.ps.isServer) return res
    return res.map((obj: any) => {
      return {
        id: obj.key,
        ...obj.payload.val()
      }
    })
  }

  getListRef<T>(path: PathReference, queryFn?: (ref: database.Reference) => database.Query) {
    return this.db.list<T>(path, queryFn)
  }

  getObjectRef<T>(path: string) {
    return this.db.object<T>(path)
  }

  cacheKey(path: string) {
    return makeStateKey<string>(`FB.${path}`)
  }

  static encodeKey(val: string) {
    return encodeURIComponent(val)
      .replace('.', '%2E')
  }

  static dencodeKey(encodedValue: string) {
    return decodeURIComponent(encodedValue)
      .replace('%2E', '.')
  }
}

@patrickmichalina
Copy link

@FrozenPandaz not sure how HttpInterceptor would work since this is websocket stuff outside of the HttpClient right?

@sugarme
Copy link

sugarme commented Jan 3, 2018

@jamesdaniels Could you give more details about how to use those services on server side and client side, please? I mean how to integrate them in to a angular universal project using angularfire2.

@sugarme
Copy link

sugarme commented Jan 4, 2018

@jamesdaniels the angularfire2/firebase-node does not appear when yarn add angularfire2@5.0.0-rc.5-next. I tried to install directly from universal_support branch and replaced them into node_modules/angularfire2 and also installed required peer dependencies (both at project level and angularefire2/firebase-node level) but when adding import "angularfire2/firebase-node"; at server.ts and building server npm run webpack:server I had red message ERROR in ./node_modules/angularfire2/firebase-node/index.js Module not found: Error: Can't resolve 'XMLHttpRequest'. Any idea everyone?

@sugarme
Copy link

sugarme commented Jan 4, 2018

@jamesdaniels I have a look at file angularfire2/firebase-node and found you wrote global['XMLHttpRequest'] = require("XMLHttpRequest").XMLHttpRequest; while in node_modules package name is xmlhttprequest. When I changed this line to global['XMLHttpRequest'] = require("xmlhttprequest").XMLHttpRequest; and recompiled, the red complaint disappeared. Was it a typo?

@Toxicable
Copy link

So after doing a bit of discussion about how to solve this I think the best solution for this is to make WS and HTTP known to zones, aka manually patch them.
Once patched then we can modify the streams to add in a first() so that only the first lot of data will be emitted and the Observables will no longer be long lived, therefore the appRef can become stable.

As for implementation, here's an example on how it's done for the HttpClient
https://github.com/angular/angular/blob/master/packages/platform-server/src/http.ts

@hiepxanh
Copy link
Contributor

hiepxanh commented Jan 6, 2018

@Toxicable agree with you, sir. that's so cleave, will we doing some implement ZoneMacroTaskWrapper within like ZoneMacroTaskConnection in your example and delegate with first() emit?

@Toxicable
Copy link

@hiepxanh There are a couple of places you could insert the first operators and doing it there would definitely work

@hiepxanh
Copy link
Contributor

hiepxanh commented Jan 7, 2018

export class ZoneClientBackend extends
    ZoneMacroTaskWrapper<HttpRequest<any>, HttpEvent<any>> implements HttpBackend {
  constructor(private backend: HttpBackend) { super(); }

  handle(request: HttpRequest<any>): Observable<HttpEvent<any>> { return this.wrap(request); }

  protected delegate(request: HttpRequest<any>): Observable<HttpEvent<any>> {
    return this.backend.handle(request);
  }
}

I have a question, is that I just extends ZoneMacroTaskWrapper, using this.wrap() for firebase obserable and how to delegate() this? in this case, you are using HttpBackend, is that firebase can use this?
@Toxicable Sorry sir, It's too difficult for me to understands. can you help me with some instruction and let me try this? I really need that feature to make firebase to working in production.

@Toxicable
Copy link

@hiepxanh You wont be able to do this in your own project, you'll need changes to the internals of AngularFire.
I don't know specifically how they construct their Observable streams so I cant give specifics on how to implement this solution either.

@ValentinFunk
Copy link
Contributor

Is any work still done on this? Is there any way I can help? I could get this working in previous versions by patching Angularfire to runOutsideAngular and then in my services manually create a macrotask until first() has emitted

@hiepxanh
Copy link
Contributor

hiepxanh commented Jan 31, 2018

can you share your macrotask ? how do you work on this?

@jamesdaniels
Copy link
Member Author

FYI I've kicked off the zone work in #1454

@jamesdaniels jamesdaniels changed the title (WIP) Support Angular Universal (WIP) Dependencies for Angular Universal Feb 2, 2018
@hiepxanh
Copy link
Contributor

hiepxanh commented Feb 2, 2018

wow, really expect this WIP pull request. Looking for angular universal like the series "Game of thrones" of HBO. I'm so exciting 👍

@ValentinFunk
Copy link
Contributor

ValentinFunk commented Feb 2, 2018

@hiepxanh
I pass e.g. this.angularFireDb.object('test/path') in there to block the zone until the first value has arrived.

  private universalBlockUntilFirst(obs: Observable<any>) {
    if (SERVER) {
      const task = Zone.current.scheduleMacroTask('universalBlockUntilFirst', () => { }, {}, () => { }, () => { });
      obs.first().subscribe(() => this.zone.runOutsideAngular(() => task.invoke()), () => this.zone.runOutsideAngular(() => task.invoke()));
    }
  }

@jamesdaniels looks really nice! very glad to see that you're making progress so we don't need to resort to hacks like this 👍

@hiepxanh
Copy link
Contributor

hiepxanh commented Feb 2, 2018

@kamshak hello, thanks for your instruction. I have 2 question:

  1. why do you duplicate this.zone.runOutsideAngular ?

  2. is that
    Zone.current.scheduleMacroTask mean: "angular, stop rendering, and wait my friendly function"
    task.invoke() mean: "hey angular, you can continue render with value come from firebase."

  3. if I have

myAPI$.first().subscribe((data) => this.items = data)

how can I mix that with this.zone.runOutsideAngular(() => task.invoke()), () => this.zone.runOutsideAngular(() => task.invoke()) ? where should I place that code? before or wrap this.items = data? can you make an example?
Thank you so much for your help

@ValentinFunk
Copy link
Contributor

ValentinFunk commented Feb 2, 2018

  1. Might not be needed but for some reason i wanted to invoke the task outside the anguar zone.
  2. A bit like that, basically makes sure the zone doesn't stabilize (same thing happens if you have e.g. a Http Request or a setTimeout call)
  3. What I do is
this.universalBlockUntilFirst(myApi$);
myAPI$.first().subscribe((data) => this.items = data)

be careful that you have a hot observable for this since it'll add a subscription

@hiepxanh
Copy link
Contributor

hiepxanh commented Feb 2, 2018

@kamshak it's hard for me to understand without context, can you make a simple gist for me? please... ✌️

@ValentinFunk
Copy link
Contributor

Sure, https://gist.github.com/Kamshak/670578d2ea2941a01b545952721f174c

You'll also need to patch angularfire2 to make sure that the Websocket is not created inside the angular zone https://github.com/Kamshak/angularfire2/blob/3a5c7950c590a7e42a0223fce76799c1764b0e0f/src/database/database.ts#L21-L23

@hiepxanh
Copy link
Contributor

hiepxanh commented Feb 2, 2018

wow, it looks so really nice! thank you so much, you should make some post on medium or pull request with that. it so amazing 👍 is that any side effect or anything to other subscription or life cycle check?

@patrickmichalina
Copy link

patrickmichalina commented Feb 2, 2018

@kamshak you got universal running cleanly with just that snippet? The API worked the same on both server and browser? What about state transfer?

@ValentinFunk
Copy link
Contributor

ValentinFunk commented Feb 2, 2018

@PatrickMcD pretty much that, there is a bit more fiddling if you want to use auth and sometimes I run into things that seem a bit odd where I need to run some ngZone.run(() => { }). The API works the same on server and browser otherwise. I use ngrx and transfer state using that, there is no transfer of the firebase caches. And it seems to not work with the new API - not sure why.

If you want to give it a shot i host the packge I use on github atm so in package.json

"angularfire2": "https://cdn.rawgit.com/Kamshak/packages/b561a6bd/angularfire2-v67.0.0.tgz",

@jamesdaniels jamesdaniels changed the title (WIP) Dependencies for Angular Universal Added firebase-node module and dependencies for Angular Universal Feb 6, 2018
@jamesdaniels jamesdaniels changed the title Added firebase-node module and dependencies for Angular Universal Dependencies for Angular Universal Feb 6, 2018
@jamesdaniels jamesdaniels merged commit 9278bb5 into master Feb 6, 2018
@jamesdaniels jamesdaniels deleted the universal_support branch February 6, 2018 07:25
@hrangel
Copy link

hrangel commented Feb 6, 2018

Anybody know when this will be available as a packaged version?

@hiepxanh
Copy link
Contributor

@hrangel very soon, sir. He almost done you can test some on here:
working example up FYI

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

@yharaskrik
Copy link
Contributor

Any movement on this Pull Request? I see it has been merged in but angularfire2/firebase-node cannot be found when being imported. on angularfire2@5.0.0-rc.6

@davideast
Copy link
Member

davideast commented Apr 2, 2018

@yharaskrik It's all in master, but not released. If you want you can clone, install, and build master to give it a try. We should do a release sometime soon after we do some docs.

git clone https://github.com/angular/angularfire2.git
cd angularfire2
yarn
yarn build
npm pack dist/packages-dist
# this will create a tarball you can install like a regular npm package
cd ../my-universal-project
yarn add ../angularfire2/angularfire2-rc.5-0.tgz # or something like that

Any feedback you'd like to give would be amazing.

The meat of the code is in this PR: #1454
A github example is here: https://github.com/jamesdaniels/onSnapshot

@yharaskrik
Copy link
Contributor

@davideast thank you very much for the quick response. I will do that right now. Is there an estimated time on a release for it?

@patrickmichalina
Copy link

@davideast I am able to use the latest with Universal (without wrapping in ngZone, thanks!). One thing that is critical, is to provide a Server<=>Client transfer state implementation. This is to avoid a redraw and second query once angular takes over on the client. There is no web-socket interceptor concept like there is for http-interceptor (as far as I know)... So how did you plan on getting this done?

I have a new implementation that isn't too large. The source code is here: https://github.com/patrickmichalina/fusing-angular/blob/2809e52a302a56db7e83753e2450cf45b53b225a/src/client/app/shared/services/firebase-firestore.service.ts

and a live demo:
https://fusing-angular-pr-64.herokuapp.com/firebase

Should I create a pull request to solve this issue? I have a few tweaks to make, mainly, to not store anything in the transfer state cache after app is stable in browser mode... but its a minor change.

@yharaskrik
Copy link
Contributor

For anyone else that is trying to pack the current master branch as @davideast suggested there was one edit I had to make to get it to work correctly

`git clone https://github.com/angular/angularfire2.git
cd angularfire2
yarn
yarn build
cd pack/package-dist # this needs to be done or else pack tries to pack a git repo named package-dist
npm pack

this will create a tarball you can install like a regular npm package

cd ../my-universal-project
yarn add ../angularfire2/angularfire2-rc.5-0.tgz`

@jamesdaniels
Copy link
Member Author

@patrickmichalina I was meaning to get some time in with state transfer this week; but looks like I may have more on my plate than expected. Feel free to send a PR with anything that you're thinking.

My plan was to startWith cache, just as you've done, and then figure out how to rehydrate the Firebase connection in a way that "pretends" it was just a connect/disconnect (that way it doesn't fire off events the server has already seen.) I've chatted with the Firebase JS SDK developers on the "right" way to do this + just have a bit more work to do first.

@patrickmichalina
Copy link

@jamesdaniels I suspect that is why I was seeing 2 events emitted. Notice I used a hash to check for distinct objects to avoid double emissions in the streams. But yea, that would mean its still making the request behind the scenes... Not sure how to tackle that one. Interesting problem :)

@yharaskrik
Copy link
Contributor

@davideast I was able to get it all packaged up but when adding it with yarn it does not add all of the things inside the tar file, firebase-node is left out of the yarn add for some reason, but if I copy and paste the package folder into my node_modules folder it works fine. Unsure of why that is happening but for now I will just forcfully add it as I need to get it working and will wait for the official angularfire2 release.

@davideast
Copy link
Member

@yharaskrik We'll take a look in our weekly sync today and report back.

@davideast
Copy link
Member

@yharaskrik Working on a fix for storage then hopefully doing a next release after that. See #1548

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.