Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tdkehoe opened this issue Mar 8, 2022 · 5 comments
Open

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

tdkehoe opened this issue Mar 8, 2022 · 5 comments

Comments

@tdkehoe
Copy link

tdkehoe commented Mar 8, 2022

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
Copy link

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
Copy link
Author

tdkehoe commented Mar 20, 2022

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
Copy link

geromegrignon commented Mar 20, 2022

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
Copy link
Author

tdkehoe commented Mar 20, 2022

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants