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

doc missing: initFirebase call needed before anything #827

Closed
loicginoux opened this issue Sep 6, 2018 · 5 comments
Closed

doc missing: initFirebase call needed before anything #827

loicginoux opened this issue Sep 6, 2018 · 5 comments

Comments

@loicginoux
Copy link

Hello,
First, thanks for your plugin.
I think it would be helpful to add to the README doc that a call to initFirebase() is necessary before anything. I had issues at initialisation and could not find anywhere the cause of the problem.

If it can help others here is a "get started" code:

      // init firebase
      window.FirebasePlugin.initFirebase()
      // grant permission 
      window.FirebasePlugin.hasPermission(function(data){
        if (!data.isEnabled) {
          window.FirebasePlugin.grantPermission();
        };
      });
      // get FCM registration token
      window.FirebasePlugin.getToken(function(token) {
          // save this server-side and use it to push notifications to this device
          console.log(`FirebasePlugin.getToken: ${token}`);
      }, function(error) {
          console.log(`FirebasePlugin.getToken error: ${error}`);
      });

      // do something when user open notification
      window.FirebasePlugin.onNotificationOpen(function(notification) {
          console.log(`FirebasePlugin.onNotificationOpen: ${notification}`);
      }, function(error) {
          console.log(`FirebasePlugin.onNotificationOpen error: ${error}`);
      });

NOTE: for meteor, it needs to be wrapped in a Meteor.startup function to work too.

@loicginoux loicginoux changed the title doc missin: initFirebase doc missing: initFirebase call needed before anything Sep 6, 2018
@soumak77
Copy link
Contributor

soumak77 commented Sep 6, 2018

@loicginoux would you mind submitting a PR with updates to the README?

@briantq
Copy link
Contributor

briantq commented Sep 6, 2018

I think an alternative fix here would be to initialize Firebase by default or when data is requested from the client, at least Firebase proper. This was disabled in the Crashlytics PR submitted a few days ago. I understand that for GDPR compliance one might not want to enable crash reporting and making that configurable makes sense.

For things that require a call into the plugin to get data (with Messaging you need to get the Token), it seems like initializing it when the call is made would make sense. That way you only have to explicitly start the components that you never interact with, but everything else works seamlessly (and doesn't required a documentation update)

@soumak77
Copy link
Contributor

soumak77 commented Sep 6, 2018

@briantq agreed. I guess I didn't catch that the Crashlytics PR disabled the auto-init functionality. This should be added back immediately. If someone finds a reason for not needing to auto-init firebase, a configuration variable can be added which allows disabling the auto-init functionality.

@soumak77
Copy link
Contributor

soumak77 commented Sep 7, 2018

This is being fixed via #830. If we can have some people help test out that PR, we can get the fix published sooner.

@soumak77
Copy link
Contributor

soumak77 commented Sep 8, 2018

This should be fix via v1.1.4. Please be sure to read my comment above needing to revert some changes to make it work.

@soumak77 soumak77 closed this as completed Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants