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

feat(remote-config)!: API update to match web SDK #3537

Merged
merged 65 commits into from
Jul 16, 2020

Conversation

russellwheatley
Copy link
Member

@russellwheatley russellwheatley commented Apr 24, 2020

RemoteConfig Module

const remoteConfig = firebase.remoteConfig();

Updates

  • added ensureInitialized API.
  • added reset API for android only
  • console.warn() if user tries to set defaultConfig which is part of web sdk.
  • console.warn() if user tries to set settings which is part of web sdk.
  • console.warn() if user tries to set setLogLevel which is part of web sdk.

RemoteConfig.setConfigSettings()

const remoteConfig = firebase.remoteConfig().setConfigSettings({});

Updates

  • can set 'minimumFetchIntervalMillis' in setConfigSettings to match web sdk.
  • can set 'fetchTimeMillis' in setConfigSettings to match web sdk.

Removed

  • isDeveloperModeEnabled from config settings and console.warn() if tried to set.
  • minimumFetchInterval config setting and console.warn() if tried to set.

RemoteConfig.getValue(key)

const remoteConfig = firebase.remoteConfig().getValue('key');

Updates

  • asBoolean() resolves value to a boolean.
  • asNumber() resolves value to a number.
  • asString() resolves value to a string.
  • getSource() source of the property.

Removed

  • value removed property. Not sure how to warn users it is no longer available
  • source removed property. Again, not sure how to warn users.

Internal Changes

  • Switched to setConfigSettingsAsync for Android, nothing similar for iOS.
  • Switched to fetchAndActivate API for both platforms.
  • Switched to async activate API for iOS. No changes needed for Android.
  • Switched to multi-app support for both platforms.

@russellwheatley russellwheatley marked this pull request as ready for review April 30, 2020 15:32
Copy link
Contributor

@Salakar Salakar left a comment

Choose a reason for hiding this comment

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

Minor feedback, ta

@Salakar
Copy link
Contributor

Salakar commented Jul 14, 2020

Minor points above re the types should be Promise<void> to match the Web SDK, fetch() also not in review above needs to be Promise<void> also; https://github.com/invertase/react-native-firebase/pull/3537/files#diff-a974aa1fe8536324f6b55551e53f184eR417

Also, there's 3 methods missing from firebase.remoteConfig().X, getBoolean, getNumber, getString, these are just convenience methods so JS only, should be fairly quick to add to types and JS; e.g. getBoolean(key) fn would just be return this.getValue(key).asBoolean();

Copy link
Contributor

@Salakar Salakar left a comment

Choose a reason for hiding this comment

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

Minor points above re the types should be Promise<void> to match the Web SDK, fetch() also not in review above needs to be Promise<void> also; https://github.com/invertase/react-native-firebase/pull/3537/files#diff-a974aa1fe8536324f6b55551e53f184eR417

Also, there's 3 methods missing from firebase.remoteConfig().X, getBoolean, getNumber, getString, these are just convenience methods so JS only, should be fairly quick to add to types and JS; e.g. getBoolean(key) fn would just be return this.getValue(key).asBoolean();

@russellwheatley russellwheatley merged commit 256b6a4 into master Jul 16, 2020
@russellwheatley russellwheatley deleted the @russell/remote-config-rework branch July 16, 2020 11:33
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.

2 participants