Why can't I import NavController and ViewController into Service or App?

In my scenario I wanted to override the Angular 2 exception handler and alert the error messages when they were caught (for device debugging).

In the code I produced I injected the IonicApp service and that had a method getActiveNav(), which I think is undocumented, to return the current active nav.

Here is my code anyway, not sure how ‘good practice’ it is as I’m still learning stuff

import {Injectable, ExceptionHandler} from 'angular2/core';
import {IonicApp, Alert} from 'ionic-angular';

@Injectable()
export class MyExceptionHandler extends ExceptionHandler {
    nav: any;
    constructor(public app: IonicApp) {
        super(app);
    }

    call(error: any, stackTrace?: any, reason?: string) {
        let message = error.message;
        let alert = Alert.create({
            title: 'Exception Caught',
            message: message,
            buttons: ['OK']
        });

        this.nav = this.app.getActiveNav();
        this.nav.present(alert);
    }
}

Works perfeclty fine for me tho

import {NotificationInterface} from "./notification.interface";
import {Injectable} from "angular2/core";
import {Alert} from "ionic-angular/index";
import {NavController} from "ionic-angular/index";
import {LanguageService} from "../language/language.service";

@Injectable()
export class NotificationService implements NotificationInterface {
    private nav:NavController;
    private languageService:LanguageService;

    constructor(nav: NavController, languageService:LanguageService) {
        this.nav = nav;
        this.languageService = languageService;
    }

    alert(msg:string, title?:string) {
        console.debug("alert:" + msg);

        let alert = Alert.create({
            title: title,
            subTitle: msg,
            buttons: [this.languageService.getTranslation("ok")]
        });

        this.nav.present(alert);
    }

    confirm(msg:string, onOk:()=>void, onCancel?:()=>void, title?:string) {

        onCancel = onCancel || (() => {console.debug("cancel")});

        let alert = Alert.create({
            title: title,
            subTitle: msg,
            buttons: [
                {text: this.languageService.getTranslation("ok"), handler: onOk},
                {text: this.languageService.getTranslation("cancel"), handler: onCancel},
            ]
        });

        this.nav.present(alert);
    }

}

Just make sure you provide it in your Component, and not in the app itself

@Page({
    templateUrl: 'build/pages/main/tabs/reload/reload.html',
    pipes:[BundleUnitPipe, TranslationPipe],
    providers:[NotificationService]
})

@mathijsliquix Injecting NavController in an Injectable should be impossible either before or now with beta.7, that code shouldn’t even work, however now that the beta.7 deleted the IonicApp.getComponent() method, i think now it’s not possible to use the nav component from Injectables.

Hmm could be my project is stil at six, I dont see why iT shouldt work als long as you inject it into a navcontrol, but adding a param to the methods is also fine with me

May be app.getActiveNav() ?

2 Likes

@vadag Daaaaamn son, that’s the answer i was looking for.

Just to chime in on this, you shouldn’t be injecting ViewController or NavController into Service.
This is not their intended purpose.

As for using NavController in @App, you can use @ViewChild instead to grab the NavController instance.

1 Like

Are there any way to present alert or loading components from a service? Or is that something that is being looked into making possible? (I noticed a comment in the ionic roadmap saying something along those lines)

roadmap

1 Like

Currently no, and there really shouldn’t be. The service shouldn’t be responsible for displaying alerts/loading/ or any component that needs to be activated by nav. A service should just be for getting and returning data.

Anything else should be done within the component.

3 Likes

Fair enough. It’s just that it seems a bit un-DRY to have error handling code in every Component that uses a service, especially if you want to display the same alert with the error message.

5 Likes

I’m having that same un-DRY feeling. For example, I want to have an authentication service so I can inject it in whatever page I need because logout can happen in more than one situation. Otherwise I would have to repeat code in every page component I need to handle authentication, and that’s weird.

1 Like

Actually using getActiveNav() fixes my issue.

5 Likes

You can achieve this with a directive that listens to events on the authentication service.

hey @mhartington, I wanted to quickly followup on this… Now, I am likely misusing the term “service” here, but it seems like there are many useful scenarios where an “abstract grouping of functions” that wraps commonly used functionality (not just data retrieval) would be helpful. For instance, a Camera to interact with the camera plugin. This allows you to standardize the default parameters for your app, handle success/fail scenarios consistently, and present error messages when failures occur. For alerts to display in my service, I need the NavController, and beyond that I have an “AlertService” to standardize how my app works with alerts/popups.

Repeating the Camera.getPicture() call with all of the options everytime I want to use the camera seems redundant. As does the Alert.create(…)… this.nav.present(…), as mentioned in the original question here.

Is there no way to abstract these actions out?

@Injectable()
export class CameraService {
    private defaultOptions: CameraOptions;
    @Inject(AlertService) private alert: AlertService;


    constructor(private nav: NavController) { 
        this.defaultOptions = {
            destinationType: Camera.DestinationType.DATA_URL,            
            encodingType: Camera.EncodingType.PNG,
            quality: 100,
            targetHeight: 800,
            targetWidth: 800,
            allowEdit: true,
            saveToPhotoAlbum: false,
            correctOrientation: true,
            cameraDirection: Camera.Direction.FRONT
        };

      // *** THIS FEELS WRONG ***  But is there no other way to do this?  Is the answer to 
      // repeat the Alert.create().. this.nav.present(..) whenever we want to show an alert?
        this.alert = new AlertService(this.nav);
    }

    getPicture (sourceType: Camera.PictureSourceType): Promise<{display: string, raw: string}>{
        this.defaultOptions.sourceType = sourceType;

         return Camera.getPicture(this.defaultOptions).then((imageData) => {
            return {
                display: 'data:image/jpeg;base64,' + imageData,
                raw: imageData
            };
        }, (err) => {
            this.alert.showPopup('Picture error', 'There was an error saving your picture. Please try again.');
            return null;
        });
    }    
}

@Injectable()
export class AlertService {

    constructor(private nav: NavController) { }

    showPopup(title, message) {        
        let alert = Alert.create({
            title: title,
            subTitle: message,
            buttons: ['OK']
        });
        this.nav.present(alert);
    }
}
2 Likes

Disclaimer: my opinion only, feel free to completely disregard.

I think it’s really important for maintainability to draw a bright line between the model layer (where services live) and the view layer (where component controllers reside). You can have a service supply sensible default parameters for things like alerts, but the actual instantiation and management needs to be done by a view layer component.

If I’m reading the signature of getPicture, I would assume that it’s going to give me a Promise. If there is an error, that promise will reject. It’s the ultimate caller’s responsibility to handle that. It’s confusing and counterintuitive when functions along the promise chain start eating errors like this one is doing.

So, my personal opinion is that the motivation for wanting to access navigation from within the CameraService, which is "to eat errors in getPicture", is misguided in the first place.

Thanks so much for the quick response, @rapropos! I tend to agree with you, and like having a clear line between services and the view. The example you’ve given makes sense, but my question is a bit broader than the topic for this forum post (perhaps would be better to address elsewhere) but, since we’re here :)…

The larger issue I have is when you need to inject a service into a service. As shown in the example below (my questions are prefixed with *******). I have read through Pascal Precht’s post on this topic, but it doesn’t address the scenario when both services have dependencies…

@Injectable()
export class RcApiService {
    private _apiUrl;
    private _headers: Headers;
    

    constructor(private _http: Http) {
        this._apiUrl = 'https://www.somecoolapi.com';        
        this._headers = new Headers({ 'Content-Type': 'application/json' });
    }

    get(route: string, queryParams?: URLSearchParams) {
        let options = new RequestOptions({ method: RequestMethod.Get });

        if (queryParams)
            options.search = queryParams;

        return this.processRequest(route, options);
    }

    post(route: string, postObject: Object, headers?: Headers) {
        let options = new RequestOptions({ method: RequestMethod.Post });
                
        if(postObject) {
            options.body = postObject;
        }        

        return this.processRequest(route, options, headers);
    }

    private processRequest(route: string, options: RequestOptions, headers?: Headers) {
        this.setAuthHeader();
        options.headers = this._headers;

        if (headers) {            
            Object.assign(options.headers, headers);            
        }

        return this._http.request(this._apiUrl + route, options)
            .map(res => res.json())
            .catch(this.handleApiError);
    }

    private handleApiError(error: any) {
        let errMsg = (error.message) ? error.message :
            error.status ? `${error.status} - ${error.statusText}` : 'Server error';

        if(error.status == 401) {
            // TODO: Redirect to LoginPage
            // *****How can this be done without the nav?  I imagine there is a better way to do this, but i'm new new ng2
        }        

        return Observable.throw(errMsg);
    }

    private setAuthHeader() {
        let authData = null; // TODO: get auth data
        if (authData) {
            let bearerToken = 'Bearer ' + authData;

            if (this._headers.has('Authorization')) {
                this._headers.set('Authorization', bearerToken);
            }
            else {
                this._headers.append('Authorization', bearerToken);
            }
        } else {
            // TODO: handle auth error
        }
    }
}

@Injectable()
export class AuthService {
    @Inject(RcApiService) private apiService: RcApiService
    private storage: Storage;

    constructor(private _http: Http) {
        // ******** TODO: This feels odd to have to inject http into the injected api service.  This only
        //  seems necessary if you have an injectable service that is using another injectable service.
        this.apiService = new RcApiService(_http);

        this.storage = new Storage(SqlStorage);
    }

    isAuthenticated() {
        return this.storage.get('authData').then(data => {
            return data && data.token;
        });
    }

    login(loginData) {
        let headers = new Headers({ 'Content-Type': 'application/x-www-form-urlencoded' });
        let queryParams = `grant_type=password&username=${loginData.username}&password=${loginData.password}&client_id=rcAdminDashboard&client_secret=abc@123`;

        return this.apiService.post('/OAuth/Token', queryParams, headers).toPromise().then(
            data => {
                this.persistToken(data);
                return true;
            }
        );
    }

  private persistToken(data) {
    this.storage.set('authData', { token: data.access_token });
  }
}

You shouldn’t need to be explicitly instantiating anything here.

@Injectable()
export class RcApiService {
  constructor(private _http:Http) {
  }
}
@Injectable()
export class AuthService {
  constructor(private _rcapi:RcApiService) {
  }
}
ionicBootstrap(MyApp, [RcApiService, AuthService])
export class HomePage {
  constructor(private _auth:AuthService) {
  }
}

Works fine for me on a scratch project, and I think that’s the same structure you’re working with. I still fundamentally disagree with doing error handling in services: pass the errors through and let the callers decide how to properly handle them.

Have you tried ViewChild?

https://angular.io/docs/ts/latest/api/core/index/ViewChild-var.html

@ViewChild('Alert') alert:Alert;

or something like that.

1 Like

not allowing me to inject NavController into a service is forcing me to use even more hacky and broken design

No it isn’t. Services manage and provide data. They must not interact directly with the view layer, and if you try to circumvent that it is de facto evidence of hacky and broken design. Nobody is forcing you to use Ionic, but if you choose to, then it seems silly to me to fight against the framework.