Angular4 module-based services vs global service

Question

We're currently working on an Angular4 app and looking for feedback on architecture for services.

The five 'modules' of the app are:

• one
• two
• three
• four
• five

Currently we have one data service that specific to one, however, the abstract class ApiService could be imported across the other four modules (see code below).

Here are some options on what I'm thinking:

Option 1: Move the abstract class ApiService into our shared folder module i.e. the shared folder module gets imported into each of the five modules.

Then create a service specific to each module which is inherited from ApiService. This would make it easy to manage each individual service.

Option 2: Move the abstract class into our shared folder and create a global service that contains all the api calls for all of the five modules. That way we have a single service to manage all API connections. However, the file might get a bit big and hard to manage. Thoughts on organizing?

Option 3: Scrap the observable services all together and go with something like ngrx/store to handle state.

I'm looking for feedback on the data service architecture.

module-one-data-service.ts

import { Injectable } from '@angular/core';
import { Http, Response, Headers, RequestOptions } from '@angular/http';

import {Observable} from 'rxjs/Observable';
import 'rxjs/add/operator/map';

import { IPosition } from './position.model';
import { IPositionReference } from '../shared/side-pane/position-reference.model';
import { Subject } from 'rxjs/Subject';


export abstract class ApiService {
  protected BASE_URL = 'http://justtheurl.com';
  protected baseAndModuleUrl: string;

  private OPTIONS = new RequestOptions({
      headers: new Headers({
        'Authorization' : 'Basic thisIsJustForTesting'
      }),
      withCredentials: true
  });

  constructor(private http: Http, private module: string) {
    this.baseAndModuleUrl = `${this.BASE_URL}${module}`;
  }

  public getBaseModuleUrl() { return this.baseAndModuleUrl; }

  protected fetch(apiAction: string): Observable<any> {
    return this.http
      .get(`${this.baseAndModuleUrl}${apiAction}`, this.OPTIONS)
      .map((res: Response) => res.json().data);
  }

  protected post(apiAction: string, positions: Object[]): Observable<any> {
    return this.http
      .post(`${this.baseAndModuleUrl}${apiAction}`, positions, this.OPTIONS)
      .map((res: Response) => res.json());
  }

  protected upload(apiAction: string, file: FormData): Observable<any> {
    return this.http
      .post(`${this.baseAndModuleUrl}${apiAction}`, file, this.OPTIONS)
      .map((res: Response) => res.json());
  }

}

@Injectable()
export class ModuleOneDataService extends ApiService {
  public editableValues = new Subject<any>();

  constructor(http: Http) { super(http, '/api/module/one'); }

  public fetchTree(): Observable<IPosition[]> { return this.fetch('/tree'); }

  public fetchList(): Observable<IPosition[]> { return this.fetch('/list'); }

  public fetchIndividual(id: string): Observable<IPositionReference> { return this.fetch(`/node/${id}`); }

  public savePositionsToSubgraph(positions: Object[]): Observable<any> { return this.post('/subgraph/upsert', positions); }

  public mergeSubraphToMaster(): Observable<Object> { return this.post('/subgraph/merge', [{}]); }


}

Answer 1

ApiService should probably not be abstract at all. Looking over what you posted, it actually is acting as a wrapper to manage the Angular HTTP service. Wrapping the angular HTTP service is frequently needed because it has such an awful API.

The service classes that need access to the wrapped HTTP facilities should rather inject this API service instead of inheriting from it.

The reason for this is that they are not logical descendants of the base class and using inheritance simply for code-sharing leads to a confusing code base. There are better ways.

Here is what I would recommend

app/module-one/data-service.ts

import {Injectable} from '@angular/core';
import {Observable} from 'rxjs/Observable';

import ApiServiceFactory, {ApiService} from 'app/shared/api';

@Injectable() export class DataService {
  constructor(apiServiceFactory: ApiServiceFactory) {
    this.api = apiServiceFactory.create('/api/module/one');
  }

  api: ApiService;

  fetchTree(): Observable<IPosition[]> { 
    return this.api.fetch('/tree');
  }

  fetchList(): Observable<IPosition[]> {
    return this.api.fetch('/list');
  }

  fetchIndividual(id: string): Observable<IPositionReference> {
    return this.api.fetch(`/node/${id}`);
  }
}

app/shared/api.ts

import {Injectable} from '@angular/core';
import {Http} from '@angular/http';
import {Observable} from 'rxjs/Observable';
import 'rxjs/add/operator/map';

@Injectable() export default class ApiServiceFactory {
  constructor(readonly http: Http) {}

  create(moduleApiSubpath: string): ApiService {
    return new ApiServiceImplementation(this.http, moduleApiSubpath);
  }
}

export interface ApiService {
  fetch(url): Observable<{}>;

  post(url:string, body: {}): Observable<{}>;

  // etc.
}

const baseUrl = 'http://justtheurl.com';

// there's actually no need to make this a class at all.
// It could be as simple as a function that returns an object literal.
// It doesn't matter much since it's not exported.
class ApiServiceImplementation implements ApiService {
  constructor(readonly http: Http, readonly moduleApiSubpath: string){}

  get baseModuleUrl() {
    return `${baseUrl}${this.moduleApiSubpath}`;
  }

  fetch(apiAction: string): Observable<{}> {
    return this.http
      .get(`${this.baseModuleUrl}${apiAction}`, this.options)
      .map(res => res.json().data);
  }

  // etc.
}

Using this approach, the only shared injectable will be ApiServiceFactory. You could provide it in a shared module or you could provide it independently in each of the modules that have a service that injects it. You won't have to worry about extra instances or anything of that sort since the service is stateless and the actual objects returned by the factory are effectively transient.

Note that it would be nice if angular provided built-in support for this pattern as transitive injection is fairly commonplace and has a lot of use cases. While it's currently possible to achieve transient transient dependency injection behavior on the component level, there's no way do it on the service level without creating such a factory.

By contrast, frameworks like Aurelia allow for services to be decorated with a simple @transient, and consumers to explicitly request a new instance in a similarly simple fashion.