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 environment check to Remote-Config Module #3623

Merged
merged 6 commits into from
Sep 21, 2021
Merged

Conversation

XuechunHou
Copy link
Contributor

@XuechunHou XuechunHou commented Aug 12, 2020

Issue 2393 fix for Remote-Config module.

Added isSupported method where users can call to check if indexedDB is supported before initializing the module.

Fixes #5502

@changeset-bot
Copy link

changeset-bot bot commented Aug 12, 2020

🦋 Changeset detected

Latest commit: d11c83d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
firebase Minor
@firebase/remote-config Minor
@firebase/remote-config-compat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 12, 2020

Binary Size Report

Affected SDKs

  • @firebase/remote-config

    Type Base (8d3aca7) Head (696cd52) Diff
    browser 23.1 kB 23.8 kB +681 B (+2.9%)
    esm2017 17.7 kB 18.2 kB +481 B (+2.7%)
    main 23.1 kB 23.8 kB +681 B (+2.9%)
    module 22.7 kB 23.4 kB +701 B (+3.1%)
  • firebase

    Type Base (8d3aca7) Head (696cd52) Diff
    firebase-remote-config.js 37.0 kB 37.9 kB +888 B (+2.4%)
    firebase.js 823 kB 823 kB +490 B (+0.1%)

Test Logs

Copy link

@erikeldridge erikeldridge left a comment

Choose a reason for hiding this comment

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

Thank you!

packages/remote-config/index.ts Outdated Show resolved Hide resolved
packages/remote-config/src/errors.ts Outdated Show resolved Hide resolved
* 2. check if the current browser context is valid for using IndexedDB.
*
*/
async function isSupported(): Promise<boolean> {

Choose a reason for hiding this comment

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

Thinking (no action req'd): the change to registerRemoteConfig will throw if IndexedDB is unavailable, in which case we'd need a method to call prior to registration, which is why isSupported is static, ie:

import * as remoteconfig from 'firebase/remote-config';
import * as firebase from 'firebase/app';

if (remoteconfig.isSupported()) {
  const rc = firebase.remoteconfig();
  ...
}

packages/remote-config/src/storage/storage.ts Show resolved Hide resolved
packages/firebase/index.d.ts Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Aug 20, 2020

Changeset File Check ✅

  • No modified packages are missing from the changeset file.
  • No changeset formatting errors detected.

@@ -1772,6 +1772,15 @@ declare namespace firebase.remoteConfig {
* Defines levels of Remote Config logging.
*/
export type LogLevel = 'debug' | 'error' | 'silent';
/**
* Returns true if current browser context supports initialization of remote config module
Copy link
Member

Choose a reason for hiding this comment

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

@egilmorez Can you please take a look at it?

@hsubox76
Copy link
Contributor

Changeset File Check ⚠️

Warning: This PR modifies files in the following packages but they have not been included in the changeset file:

  • firebase
  • @firebase/remote-config

Make sure this was intentional.

Sorry, this should go away if you pull master now, or you can just ignore it.

@hsubox76 hsubox76 assigned egilmorez and unassigned XuechunHou Sep 17, 2021
@hsubox76 hsubox76 changed the title Issue 2393 - Add environment check to Remote-Config Module Add environment check to Remote-Config Module Sep 17, 2021
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

LG, thanks!

@google-oss-bot google-oss-bot added the doc-changes PRs that affect docs label Sep 17, 2021
@hsubox76 hsubox76 assigned Feiyang1 and unassigned hsubox76 Sep 17, 2021
@hsubox76
Copy link
Contributor

I've rebased and updated this since it was last updated a year ago.

@Feiyang1 Feiyang1 assigned hsubox76 and unassigned Feiyang1 Sep 20, 2021
@hsubox76 hsubox76 merged commit f90c1d0 into master Sep 21, 2021
@hsubox76 hsubox76 deleted the issue2393-remote-config branch September 21, 2021 16:52
@google-oss-bot google-oss-bot mentioned this pull request Sep 21, 2021
@firebase firebase locked and limited conversation to collaborators Oct 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
6 participants