-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Minor modular sample fixes #3026
base: main
Are you sure you want to change the base?
Conversation
…d be declared in app module providers
@hakimio User/ScreenTracking are automatic, they don't need to be called to begin functioning. The isSupported error is likely cause the firebase JS SDK needs to be updated in that sample. |
@jamesdaniels not sure I understand what you mean by "don't need to be called". In angularfire v6, you had to add those services to the app providers. Isn't that the case in v7? How are they supposed to be used now? |
@hakimio woops, don't mind me. I peek at this on mobile and it looked like you deleted those lines, rather than added them. Good catch. |
@jamesdaniels
Unrelated question/issue: because of EDIT: imho, changing |
In the samples I don't think we want to take on any additional dependencies, just for the sake of being able to quickly validate correct functionality with any Angular version, including RCs, before other NPM modules are compatible. Thanks for pointing out this is the case with |
Description
A couple minor fixes for the modular demo:
Additional info
Tried building and serving the demo project, but in both cases it throws the following error: