Strange oddities to data access in a Angular 8 component

Question

I'm developing a service that processes an xml file and returns a data structure of interfaces.
At first I thought that the service had correctly returned all the data, but then I realized some unclear things, in particular when I was going to read the data structure in a component.
This is my service:

import { HttpClient } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { AppConfig } from 'src/app/app.config';
import { forkJoin, Subscription } from 'rxjs';

@Injectable({
  providedIn: 'root'
})
export class BibliographyParserService {

  private editionUrls = AppConfig.evtSettings.files.editionUrls || [];
  private bibliographicCitations: Array<BibliographicCitation> = [];
  private subscriptions: Array<Subscription> = [];

  constructor(
    private http: HttpClient,
  ) {
  }

  private getHttpCallsOBSStream() {
    return this.editionUrls.map((path) =>  this.http.get(path, { responseType: 'text'}));
  }

  public getBibliographicCitations(): Array<BibliographicCitation> {
    const parser = new DOMParser();
    this.subscriptions.push(forkJoin(this.getHttpCallsOBSStream()).subscribe((responses) => {
      responses.forEach(response => {
        Array.from(parser.parseFromString(response, 'text/xml').getElementsByTagName('bibl')).forEach(citation => {
          if (citation.getElementsByTagName('author').length === 0 &&
              citation.getElementsByTagName('title').length === 0 &&
              citation.getElementsByTagName('date').length === 0) {
            const interfacedCitation: BibliographicCitation = {
              title: citation.textContent.replace(/\s+/g, ' '),
            };
            if (!this.bibliographicCitations.includes(interfacedCitation)) { this.bibliographicCitations.push(interfacedCitation); }
          } else {
            const interfacedCitation: BibliographicCitation = {
              authors: citation.getElementsByTagName('author'),
              title: String(citation.getElementsByTagName('title')[0]).replace(/\s+/g, ' '),
              date: citation.getElementsByTagName('date')[0],
            };
            if (!this.bibliographicCitations.includes(interfacedCitation)) { this.bibliographicCitations.push(interfacedCitation); }
          }
        });
      });
    }));
    return this.bibliographicCitations;
  }
}

export interface BibliographicCitation {
  authors?: HTMLCollectionOf<Element>;
  title: string;
  date?: Element;
}

And this is my component:

import { Component, AfterViewInit } from '@angular/core';
import { BibliographyParserService } from 'src/app/services/xml-parsers/bibliography-parser.service';

@Component({
  selector: 'evt-bibliography',
  templateUrl: './bibliography.component.html',
  styleUrls: ['./bibliography.component.scss']
})
export class BibliographyComponent implements AfterViewInit{

  constructor(
    public bps: BibliographyParserService,
  ) {
    console.log(this.bps.getBibliographicCitations());         // WORKS, return the correct data structure
    this.bps.getBibliographicCitations().forEach(console.log); // DOESN'T RETURN ANYTHING!
    console.log(this.bps.getBibliographicCitations().length);  // RETURN 0
  }

  ngAfterViewInit() {
    (document.querySelectorAll('.cloosingRood')[0] as HTMLElement).onclick = () => {
      (document.querySelectorAll('.biblSpace')[0] as HTMLElement).style.display = 'none';
    };
  }
}

The very strange thing is with those three logs. We can see different things between them.
With the first log I can see the whole data structure in console.
With the second, nothing happens.
With the third, the length is equal to 0, which is not true because as shown in the first log, the data structure is full...!
I don't understand why these oddities. Is there anything I missed from the angular documentation?

PS: I don't want to make the subscription in the component, otherwise I would have already solved... I want to separate logic from visualization and create the data structure in the service, as I did.

I wouldn't rely on `console` as there are some counterintuitive behavior that can occur. See [here](https://stackoverflow.com/questions/11284663/console-log-shows-the-changed-value-of-a-variable-before-the-value-actually-ch) for one explanation. — Phix, Dec 06 '19 at 18:51
You can not return array from a subscription. You need to return an observable or a promise then subscribe it or await it . — Eldar, Dec 08 '19 at 18:52

blid · Answer 1 · 2019-12-09T22:19:56.357

There are two problems here:

You are mixing imperative and reactive programming.

You can in no way know when the forkJoin will emit. Therefore, you cannot be sure that the instance variable bibliographicCitations will have updated when you return from getBibliographicCitations. In could be synchronous or asynchronous. You need to make the method observable:

getBibliographicCitations(): Observable<Array<BibliographicCitation>>;

A simple way of doing this would be to refactor the method to just setup an Observable:

private refreshSub = new Subject<void>();
private bibliographicCitations$: Observable<BibliographicCitation[]>;

refresh(): void {
  this.refreshSub.next();
}

private buildObservables(): void {
  this.bibliographicCitations$ = this.refreshSub.pipe(
    switchMap(() => forkJoin(this.getHttpCallsOBSStream()),
      map(responses => {
        // Get all elements from response.
        const elements = responses.reduce((acc, response) => [
          ...acc,
          ...parser.parseFromString(response, 'text/xml').getElementsByTagName('bibl')
        ], [] as Element[]);

        // Use all elements to query for stuff.
        return elements.reduce((acc, element) => {
          if (['author', 'title', 'date'].every(tag => element.getElementsByTagName(tag).length === 0)) {
            return [...acc, { title: element.textContent.replace(/\s+/g, ' ') }];
          } else {
            return [...acc, {
              authors: element.getElementsByTagName('author'),
              title: `${element.getElementsByTagName('title')[0]}`.replace(/\s+/g, ' '),
              date: element.getElementsByTagName('date')[0],
            }];
          }
        }, [] as BibliographicCitation[]);
      })
    shareReplay(1)
  );
}

Then you can add a getter-method for that Observable in your service.

getBibliographicCitations(): Observable<Array<BibliographicCitation>> {
  return this.bibliographicCitations$;
}

And the refresh-method can be used to retrigger a read.

With all that in place, you can use the getBibliographicCitations inside the component and subscribe to it there. The key is that you should only subscribe when you're truly ready to use the value. Storing emissions from an observable is an anti-pattern.

You are creating new subscriptions at every call to getBibliographicCitations

Every time you call your method getBibliographicCitations a new Subscription is created. That means that after calling it three times, there will be 3 subscriptions operating with their own DOMParser. And each one will modify the instance variable bibliographicCitations.

If you want to avoid duplicate subscriptions, you would have to unsubscribe on previous subscriptions before creating new ones. But, none of that would be necessary if you go with the code above and set up the Observable once.

Can you update to the entire answer? I can't understand how I can update the observable if you assign it immediately with `this.bibliographicCitations$ = forkJoin [...]` — Memmo, Dec 09 '19 at 09:28
To update the stream, you can use `switchMap` to say that any time refreshSub emits, then do another forkJoin. Observable streams works best if you set them up once. Look at my answer. — blid, Dec 09 '19 at 10:55
okay ... but I don't understand how do you assign the new interface to the `BibliographicCitation` array (which I assume is inside the observable)! What must happen inside `map`? I do a series of checks, and then I do an assignment. Your assignment here is at the beginning... and I don't know how to build the rest inside `map`... — Memmo, Dec 09 '19 at 11:05
From `map`, return the value you are interested in. `const obs$ = of({ a: 'a', b: 'b' });` `const b$ = obs$.pipe(map(obs => obs.b));` Whatever the map returns is whatever the subscriber will receive when they subscribe to the stream. — blid, Dec 09 '19 at 13:43
If possible, could you edit your answer based on my problem? In practice, if you could, give me the complete answer XD — Memmo, Dec 09 '19 at 21:30
I added the content of the map. It will get your `BibliographicCitation[]`. You have to subscribe to it in your component. — blid, Dec 09 '19 at 22:20
suggesting flatMap instead of reduce for clarity (on responses). Also suggesting map instead of reduce on elements. — corolla, Dec 14 '19 at 18:14

Memmo · Accepted Answer · 2019-12-11T10:46:45.610

You don't have to subscribe into the service, but return an observable.
The way you create it is indifferent, the important thing is to respect the syntax, which for an angular novice is not easy, understandably!
In light of the answer given by blid, I propose this solution that mediates reactive and imperative programming, without destroying what you've done so far.
Obviously if someone is comfortable with the imperative programming to create certain data structures, he is free to do what he likes, but if you decide to use an angular environment, you need to know all the opportunities it offers!

Anyway... this is the service:

import { HttpClient } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { AppConfig } from 'src/app/app.config';
import { forkJoin, Observable } from 'rxjs';
import { shareReplay, map } from 'rxjs/operators';

@Injectable({
  providedIn: 'root'
})
export class BibliographyParserService {
  private editionUrls = AppConfig.evtSettings.files.editionUrls || [];
  private bibliographicCitations$: Observable<BibliographicCitation[]>;

  constructor(
    private http: HttpClient,
  ) {
    const parser = new DOMParser();
    const bibliographicCitations: Array<BibliographicCitation> = [];
    this.bibliographicCitations$ = forkJoin(this.getHttpCallsOBSStream()).pipe( //use pipe...
      map(responses => {                                                        //...and map
        responses.forEach(response => {
          Array.from(parser.parseFromString(response, 'text/xml').getElementsByTagName('bibl')).forEach(citation => {
            if (citation.getElementsByTagName('author').length === 0 &&
                citation.getElementsByTagName('title').length === 0 &&
                citation.getElementsByTagName('date').length === 0) {
              const interfacedCitation: BibliographicCitation = {
                title: citation.textContent.replace(/\s+/g, ' '),
              };
              if (!bibliographicCitations.includes(interfacedCitation)) { bibliographicCitations.push(interfacedCitation); }
            } else {
              const interfacedCitation: BibliographicCitation = {
                authors: citation.getElementsByTagName('author'),
                title: String(citation.getElementsByTagName('title')[0]).replace(/\s+/g, ' '),
                date: citation.getElementsByTagName('date')[0],
              };
              if (!bibliographicCitations.includes(interfacedCitation)) { bibliographicCitations.push(interfacedCitation); }
            }
          });
        });
        return bibliographicCitations; //This is the core!!!
        }),
      shareReplay(1)
    );
  }

  private getHttpCallsOBSStream() {
    return this.editionUrls.map((path) =>  this.http.get(path, { responseType: 'text'}));
  }

  public getBibliographicCitations(): Observable<Array<BibliographicCitation>> {
    return this.bibliographicCitations$;
  }
}

export interface BibliographicCitation {
  authors?: HTMLCollectionOf<Element>;
  title: string;
  date?: Element;
}

And this is an example of what you will do in a component:

constructor(
    public bps: BibliographyParserService,
  ) {
    this.bps.getBibliographicCitations().subscribe(response => {
        response.forEach(cit => {
            console.log(cit);
        });
    });
}

For what it's worth, this is identical to my answer above. `forkJoin` will fire once, since it's emits on the `complete` notification of its source observables. Moving `bibliographicCitations` outside of the map accomplishes nothing, it just makes the code fragile. — blid, Dec 13 '19 at 17:13
Next week I will update this piece of code to optimize it as much as possible. — Memmo, Dec 13 '19 at 18:10

Strange oddities to data access in a Angular 8 component

2 Answers2