Angular 6+ TypeError: Cannot read property 'firstName' of undefined

Question

My (Angular 6.1.2) view is loading in before my observable does thus creating the undefined error. The data ends up there properly after the console error.

COMPONENT

ngOnInit() {
   this.route.params.subscribe(params => {
        this.UserId = params['id'];

        this.objectService.get(this.UserId).subscribe((ret: user) => {
            this.User = user;
        });
    });
}

SERVICE

public get(id: string): Observable<User> {
    return this.http.get<User>(`${this.api}/${id}`);
}

HTML

<h1>{{User.firstName}}</h1>

Checking it first suppresses the error.

<h1 *ngIf="User">{{User.firstName}}</h1>

but I'd like to know how to do this correctly from the controller and or service so that my view works without checking for the data. Using the in-memory-api btw.

UPDATE 2019: I've switched to resolving data with the router where I can then this issue is easy to handle with async pipe in the template. This method has saved me many headaches.

UPDATE: It seems unanimous that we shouldn't be checking in the view. Properly setting the variables before they are used seems to be the best way. I've chosen to leverage the exported class when I set the variable like this:

public user: User = new User(); which stops the undefined error.

Solution 1

You are right, ideally we shouldn't put the checking logic in the view. You can do something like this:

In your component:

public firstName: string;

ngOnInit() {
    this.route.params.subscribe(params => {
        this.UserId = params['id'];

        this.objectService.get(this.UserId).subscribe((ret: user) => {
            if (user) {
                this.firstName = user.firstName;
            }
        });
    });
}

Then in your view html:

<h1>{{firstName}}</h1>

Also, as a best practice for subscriptions and for better performance, you should subscribe in ngOnInit, then unsubscribe in ngOnDestroy, something like this:

import { Subscription } from "rxjs";

public firstName: string;

private userSub: Subscription;

ngOnInit() {
    this.route.params.subscribe(params => {
        this.UserId = params['id'];

        this.userSub = this.objectService.get(this.UserId).subscribe((ret: user) => {
            if (user) {
                this.firstName = user.firstName;
            }
        });
    });
}

public ngOnDestroy() {
    this.userSub.unsubscribe();
}

Solution 2

You have three ways of solving this:

1 - Check the variable before binding it with ? like <h1>{{User?.firstName}}</h1>

2 - Starting your variable with an empt object User = {};

3 - Starting your variable with an empt User instance

class User {
  name: string;
  ...
}

class UserGrid {
  user: User = new User();
}

As the error says cannot read property of undefined

Tip: do not use first letter Upper for variable names. This is for classes and Types like user: User = new User();

Solution 3

Rxjs switchmap is elegant way to subscribe inside another subscribe

User={}
ngOnInit() {
   this.route.params.switchMap(params=>{
     this.objectService.get(params['id'])
   }).subscribe((ret: user) => {
            this.User = ret;
        });
    });
}

Solution 4

First of all, the way you did now is the one of the right way. The other better way would be the solution 1 suggested by @vinagreti.

This happen because when your component initialised, the template is loaded but your User object is undefined because this.User = user; is sit inside an async rxjs call.

That's why in the template {{User.firstName}} return error because you are reading firstName from undefined.

The proposed solutions by @vinagreti are the best solution already. Personally I preferred using <h1>{{User?.firstName}}</h1> in most cases.

View more solutions

Comments

[kshetline]

You could use <h1>{{User?.firstName}}</h1> and then not need the ngIf, but that's just checking for the data in another way.

[Ben Racicot]

Add blank values to the entire User object on init? Hmm, may be the cheapest option but perhaps I'm doing something wrong? This seems like a new problem I've never ran into before.

[Ben Racicot]

Thanks for this answer. A couple things: Why shouldnt we be checking in the view? I like your idea of assigning variables to each expected piece of data. That may be the best way I've seen to check on the controller side.

[Jian Li]

Ideally we should put as less logic in the view as possible, view should only be used for displaying data. Putting logic in the view will decrease the performance of your application. It may not be obvious when you don't have that much logic in your view, but as a best practice you shouldn't.