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

add firebase remote config service #49

Merged
merged 4 commits into from
Oct 22, 2019
Merged

add firebase remote config service #49

merged 4 commits into from
Oct 22, 2019

Conversation

fachrihawari
Copy link
Contributor

Add Firebase Remote Config support to nuxt-fire and will resolve #48 issue

Copy link
Member

@lupas lupas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, everything perfect! I love how you thought of every little detail.

Two things though:

  • We don't want the users have to set the minimumFetchIntervalMillis by themselves, so I would suggest we add that (with an option to set it via nuxt.config.js) within the setup
  • Same goes for defaultConfig.

As seen in Setps 1.2 and 2 here:
https://firebase.google.com/docs/remote-config/use-config-web

See my comments within the files.

Could you add that? Thanks so much :)

BTW:
I added a dev branch to which you can open pull requests to in the future, so I can approve such a change already and merge in into dev and add such details by myself before merging to master. So I don't have to ask you to do all the chances ^^'

THANKS A LOT! 👍


const fireConfig = firebase.remoteConfig()
const fireConfigObj = firebase.remoteConfig
inject('fireConfig', fireConfig)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about we add the following functionality here?

const optionalFetchInterval = options.remoteConfig.minimumFetchIntervalMillis
remoteConfig.settings = {
  minimumFetchIntervalMillis: optionalFetchInterval ? optionalFetchInterval : 3600000,
};

remoteConfig.defaultConfig = (options.remoteConfig.defaultConfig)

So remoteConfig is setup completely and the default values can, if needed, be passed through nuxt.config.js.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was just a quick write up, need to do it better since this will break if options.remoteConfig doesn't exist.

@@ -74,7 +74,7 @@ modules: [
}
},
// The following options are optional:
useOnly: ['auth','firestore','functions','storage','realtimeDb', 'messaging', 'performance', 'analytics'],
useOnly: ['auth','firestore','functions','storage','realtimeDb', 'messaging', 'performance', 'analytics', 'remoteConfig'],
customEnv: false,
functionsLocation: 'us-central1',
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here add

remoteConfig: {
   minimumFetchIntervalMillis: 3600000,
   defaultConfig: {
      'welcome_message': 'Welcome'
   }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And later we would also need to add a description to line 206

@fachrihawari
Copy link
Contributor Author

Hi @lupas, I already push the update.

@lupas lupas merged commit f9ba1f1 into nuxt-community:master Oct 22, 2019
@lupas
Copy link
Member

lupas commented Oct 22, 2019

Added with v.2.3.0.

Thanks a lot @fachrihawari, this is neat! :)))

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

Successfully merging this pull request may close these issues.

Add Remote Config service
2 participants