Why am I getting a "Cannot access 'variable' before initialization" error from a subscriber referencing itself?

Question

In my Angular 8 application I have a component that subscribes to a service and awaits a notification that the service has loaded like this:

constructor(public contentService: ContractService) {
    let self = this;
    let sub = contentService.loaded.subscribe(function(loaded) {
        if (loaded) {
            self.loading = false;
            if (sub) {
                sub.unsubscribe();
            }
        }
    });
}

This works correctly in most cases but sometimes I am seeing the following error message:

ERROR ReferenceError: Cannot access 'sub' before initialization
    at SafeSubscriber._next (abstract-content.component.ts:51)
    at SafeSubscriber.__tryOrUnsub (Subscriber.js:185)
    at SafeSubscriber.next (Subscriber.js:124)
    at Subscriber._next (Subscriber.js:72)
    at Subscriber.next (Subscriber.js:49)
    at BehaviorSubject._subscribe (BehaviorSubject.js:14)
    at BehaviorSubject._trySubscribe (Observable.js:42)
    at BehaviorSubject._trySubscribe (Subject.js:81)
    at BehaviorSubject.subscribe (Observable.js:28)
    at Observable._subscribe (Observable.js:76)

As you might gather from the name the abstract-content-component is a parent class that has several implementations so this is called from a child class, but that is effectively an empty shell with no logic of its own that I am using to make it easy to switch templates.

The error message seems nonsensical to me ( line 51 in this case is the if(sub) check which I added because I was seeing the same error in the line after ) because how can the subscription not exist if we are inside its own event handler? I use this pattern in other places successfully, why is there a problem here?

Solution 1

You could get rid of this error by declaring sub as an attribute of the component.

sub: Subscription = new Subscription();
// ...

constructor(public contentService: ContractService) {
    let self = this;
    this.sub = contentService.loaded.subscribe(function(loaded) {
          // ...
          this.sub.unsubscribe();
          // ...
    });
}

Although I need to say the proper place to do this kind of calls is not in the constructor. Maybe inside the ngOnInit is a good place.

---

To make sure the subscriber do not receive any other values once the service returns the first one, you can indeed unsubscribe after its "job" is done (like you were doing), but declare the sub variable as an attribute and initialize it with new Subscription();

---

To make sure you don't leave any subscription hanging you can unsubscribe from it in ngOnDestroy in the component:

ngOnDestroy() {
  // ...
  this.sub.unsubscribe();
  // ...
}

---

Additionally if you feel you need a better understanding of how the Observables pattern works see Angular Docs for Observables

Solution 2

You are checking for sub before you've set sub.

If you view your code logically, and set by step, you'll see that you are declaring sub with an initial value. However, that initial value is a function, or specifically in this case a subscription. Which is still fine.

However - the issue lies in the fact, that in the function itself, you rely on the sub variable being set. Which it isn't, because you're in the process of setting it, during its declaration.

Hence the error:
Cannot access 'sub' before initialization

It's actually unclear as to what you're trying to do, because logically it looks like you're trying to subscribe to something, but if your subscribed to it, unsubscribe? But you shouldn't do that in the unsubscribe.

If you would prefer, you could change your code to the following,

// I assume you have a variable "loaded" above, because you've set self to this in your code
private unsubscribeStream = new Subject();

this.contentservice.loaded.pipe(
  filter(x => x !== null), // Filter out where x doesn't exist
  takeUntil(this.unsubscribeStream)).subscribe(loaded => {
    this.loaded = loaded;
  });

// When the component is destroyed, call the next value of unsubscribe stream which will unsubscribe from anything we've set to takeUntil
ngOnDestroy() {
   this.unsubscribeStream.next();
}

Solution 3

I just came across the same issue realizing that my code was old and written when I didnt know as much about rxjs, jet. I now see it as an antipattern to write code this way.

What you want is to get just 1 value emitted and then be done. Several options

1. What I do:

   contentService.loaded .pipe( first(), ) .subscribe(...);

2. OR, just as good:

   contentService.loaded .pipe( take(1), ) .subscribe(...);

3. use Promises, they are not deprecated and if you just need always exactly ONE value, then there is really no useCase for an "Observable" (or would you always use a number and make a string out of it when you need a string.. just because numbers are an amazing thing ;)

Take care!

Comments

[user1539401]

The goal is that this event may fire more than once, but I only want to respond to it the first time. Once it has been called for the first time I am no longer interested in that service, so keeping the subscription hanging around feels a bit superfluous. It just seems so weird that it is possible for the function reference not to exist when the function is being called, like sitting in a car that hasn't been built yet. In fact I'd go so far as to say that if I view my code logically, I would expect the reference to be created before the call it refers to executed.

[user1539401]

If I leave it as it is and just stop trying to unsubscribe to it, does that mean my subscriber won't be called again if the loaded observable changes because it doesn't exist and never has, in spite of having worked? You might be onto something with the timing, this might only be happening when a new object subscribes to a service that already has a value.

[cmprogram]

If that's your goal, then replace takeUntil, with take(1). You can then remove the unsubscribeStream and ngOnDestroy. However: the way you've worded this doesn't really make an awful lot of sense. This example would be more like looking up a the instructions for how to build a car, and in the set of instructions it says, "use the car to build the car". Additionally, it's not superflous - the way you've set this up, you SHOULD be notified continually if the conditions change, which is the purpose of subscribing to anything.

[user1539401]

So perhaps this is a misunderstanding on my part - in the old days we used to just use await, but from what I could gather the pattern for Angular2 and onwards is to use a subscription instead. Is there a better way to do this?

[lealceldeiro]

@glenatron I don't quite understand what you mean. Could you, please, elaborate a bit more?

[user1539401]

I guess I'm worried about the way that this interacts with scopes - am I going to leave some half-created subscriber hanging onto the service after its job is complete? What is the best way to ensure that it only happens once?

[lealceldeiro]

@glenatron I updated the answer after your comments, when you have a time check it.

[Miniskurken]

You are a king! :D