angular / angularfire

Angular + Firebase = ❤️
https://firebaseopensource.com/projects/angular/angularfire2
MIT License
7.66k stars 2.19k forks source link

Constructor must be set `public`, `protected`, or `private` #3173

Open tdkehoe opened 2 years ago

tdkehoe commented 2 years ago

Version info

Angular: 13.2.5

Firebase: 10.1.4

AngularFire: 7.2.1

How to reproduce these conditions

Build the AngularFire Quickstart. https://github.com/angular/angularfire/blob/master/docs/install-and-setup.md

Step 7 fails.

Steps to set up and reproduce

This code is provided in the tutorial:

  constructor(firestore: AngularFirestore) {
    this.items = firestore.collection('items').valueChanges();
  }

That produces this error message:

src/app/app.component.ts:29:12 - error TS2339: Property 'firestore' does not exist on type 'AppComponent'.

I can fix the problem with this code:

  constructor(public firestore: AngularFirestore) {
    this.items = firestore.collection('items').valueChanges();
  }

Or with this code:

  constructor(protected firestore: AngularFirestore) {
    this.items = firestore.collection('items').valueChanges();
  }

This also works:

  constructor(private firestore: AngularFirestore) {
    this.items = firestore.collection('items').valueChanges();
  }

In other words, setting the constructor to public, protected, or private works. Setting nothing throws an error.

Debug output

src/app/app.component.ts:29:12 - error TS2339: Property 'firestore' does not exist on type 'AppComponent'.

Expected behavior

Not setting this and setting public should be the same, according to the TypeScript Handbook.

Actual behavior

Not setting this throws an error.

geromegrignon commented 2 years ago

Hi @tdkehoe, could you provide a link to the related documentation on Typescript Handbook? The current error is the expected behavior in my opinion and unrelated to this library.

tdkehoe commented 2 years ago

https://www.typescriptlang.org/docs/handbook/2/classes.html#public

"Because public is already the default visibility modifier, you don’t ever need to write it on a class member, but might choose to do so for style/readability reasons."

geromegrignon commented 2 years ago

An element passed in the constructor is a parameter, not a class member.

You need to explicitly create a dedicated class member for this parameter to initialize it.

class Foo {
myService: MyService;

constructor(myService: MyService) {
  this.myService = myService
  }
}

As you are missing this class member, you had the related error.

By using public, protected or private, it prevents you to explicitly creating the class member.

Here is an example of the error reproduced by injecting an Angular service in a COmponent (without this library): https://stackblitz.com/edit/angular-ivy-kw9ua9?file=src%2Fapp%2Fapp.component.ts

tdkehoe commented 2 years ago

Your code breaks my auth service.

export class HeaderComponent {
  auth: AngularFireAuth;

constructor(
    auth: AngularFireAuth, <-- 'auth' is grayed out in Visual Studio Code
   ) 
}

  loginGoogle() {
    this.auth.signInWithPopup(new firebase.auth.GoogleAuthProvider())
  }
}

The error message is

6485 ERROR TypeError: Cannot read properties of undefined (reading 'signInWithPopup')

It seems like loginGoogle() is trying to use the variable auth: AngularFireAuth; (the first one) when it should use the constructor auth: AngularFireAuth; (the second).

Looking at your Stackblitz, adding the variable as you suggested fixes the problem:

import { Component, OnInit, VERSION } from '@angular/core';
import { TodoService } from './todo.service';

@Component({
  selector: 'my-app',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.css'],
})
export class AppComponent implements OnInit {
  name = 'Angular ' + VERSION.major;
  todoService: TodoService; // I added this code

  constructor(todoService: TodoService) {}

  ngOnInit() {
    console.log(this.todoService.todos);
  }
}

Here's my complete (working) code.

import { Component } from '@angular/core';
import { Router } from '@angular/router';
import { RouteUrlService } from '../services/route-url/route-url.service';
import { AngularFireAuth } from '@angular/fire/compat/auth';
import firebase from 'firebase/compat/app';
import { TranslocoService } from '@ngneat/transloco';

@Component({
  selector: 'app-header',
  templateUrl: './header.component.html',
  styleUrls: ['./header.component.css'],
})
export class HeaderComponent {
  // auth: AngularFireAuth;
  L1Language: string = 'en';
  loggedIn: boolean = false;
  userFullName: string | null | undefined = null;
  userPhotoUrl: string | null | undefined = null;
  availableLangs: string[] | { id: string, label: string }[];
  showEmailLink: boolean = false;

  constructor(
    private router: Router,
    public auth: AngularFireAuth,
    private service: TranslocoService
  ) {  }

  loginGoogle() {
    this.auth.signInWithPopup(new firebase.auth.GoogleAuthProvider())
      .then((result) => {
        this.loggedIn = true;
        this.userFullName = result.user?.displayName;
        this.userPhotoUrl = result.user?.photoURL;
        this.router.navigate(['/', 'home']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  loginFacebook() {
    this.auth.signInWithPopup(new firebase.auth.FacebookAuthProvider())
      .then((result) => {
        this.loggedIn = true;
        this.userFullName = result.user?.displayName;
        this.userPhotoUrl = result.user?.photoURL;
        this.router.navigate(['/', 'home']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  loginTwitter() {
    this.auth.signInWithPopup(new firebase.auth.TwitterAuthProvider())
      .then((result) => {
        this.loggedIn = true;
        this.userFullName = result.user?.displayName;
        this.userPhotoUrl = result.user?.photoURL;
        this.router.navigate(['/', 'home']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  loginGithub() {
    this.auth.signInWithPopup(new firebase.auth.GithubAuthProvider())
      .then((result) => {
        this.loggedIn = true;
        this.userFullName = result.user?.displayName;
        this.userPhotoUrl = result.user?.photoURL;
        this.router.navigate(['/', 'home']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  loginEmail() {
    this.auth.signInWithPopup(new firebase.auth.EmailAuthProvider())
      .then((result) => {
        this.loggedIn = true;
        this.userFullName = result.user?.displayName;
        this.userPhotoUrl = result.user?.photoURL;
        this.router.navigate(['/', 'home']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  loginPhone() {
    this.auth.signInWithPopup(new firebase.auth.PhoneAuthProvider())
      .then((result) => {
        this.loggedIn = true;
        this.userFullName = result.user?.displayName;
        this.userPhotoUrl = result.user?.photoURL;
        this.router.navigate(['/', 'home']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  logout() {
    this.auth.signOut()
      .then((result) => {
        this.loggedIn = false;
        this.router.navigate(['/', 'landing']);
      })
      .catch((error) => {
        console.error(error);
      });
  }

  changeL1Language(lang: string) {
    this.service.setActiveLang(lang); // changes the L1 language in the view
    this.L1Language = lang; // changes the displayed flag
  }

  toggleEmailLink() {
    switch (true) {
      case (this.showEmailLink === true):
        this.showEmailLink = false;
        break;
      case (this.showEmailLink === false):
        this.showEmailLink = true;
        break;
      default:
        console.error(`Error in toggleEmailLink`);
    }
  }
}
geromegrignon commented 2 years ago

It seems like loginGoogle() is trying to use the variable auth: AngularFireAuth; (the first one) when it should use the constructor auth: AngularFireAuth; (the second).

@tdkehoe That's because the constructor parameter and your class member aren't linked together. In the body of your constructor, you would need to initialize your class member with the constructor parameter.

As your class member is not initialized, it's undefined. So you are getting an error trying to access signInWithPopup from an undefined variable.

Please note the common usage is to use public, protected or private keywords for simplicity.