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(rc): Add custom signals support #8602

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

ashish-kothari
Copy link

No description provided.

@ashish-kothari ashish-kothari self-assigned this Oct 25, 2024
Copy link

changeset-bot bot commented Oct 25, 2024

🦋 Changeset detected

Latest commit: 832258b

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

This PR includes changesets to release 4 packages
Name Type
@firebase/remote-config-types Minor
@firebase/remote-config Minor
firebase 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

@ashish-kothari ashish-kothari changed the title feat(RC): Add custom signals support feat(rc): Add custom signals support Oct 25, 2024
@ashish-kothari ashish-kothari requested review from 9kubczas4 and removed request for 9kubczas4 October 25, 2024 14:22
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 25, 2024

Size Report 1

Affected Products

  • @firebase/remote-config

    TypeBase (0f5714b)Merge (1d6176d)Diff
    browser19.2 kB21.5 kB+2.37 kB (+12.4%)
    main20.2 kB22.7 kB+2.46 kB (+12.2%)
    module19.2 kB21.5 kB+2.37 kB (+12.4%)
  • bundle

    TypeBase (0f5714b)Merge (1d6176d)Diff
    remote-config (getAndFetch)46.3 kB47.6 kB+1.25 kB (+2.7%)
  • firebase

    TypeBase (0f5714b)Merge (1d6176d)Diff
    firebase-compat.js794 kB796 kB+1.20 kB (+0.2%)
    firebase-remote-config-compat.js27.3 kB28.5 kB+1.20 kB (+4.4%)
    firebase-remote-config.js29.6 kB31.2 kB+1.56 kB (+5.3%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/8EaSzXbrIc.html

@ashish-kothari ashish-kothari marked this pull request as draft October 25, 2024 14:32
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 25, 2024

Size Analysis Report 1

Affected Products

  • @firebase/remote-config

    • activate

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size10.8 kB12.0 kB+1.20 kB (+11.0%)
      size-with-ext-deps37.4 kB38.6 kB+1.20 kB (+3.2%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • ensureInitialized

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size10.5 kB11.7 kB+1.20 kB (+11.4%)
      size-with-ext-deps37.1 kB38.3 kB+1.20 kB (+3.2%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • fetchAndActivate

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.4 kB12.7 kB+1.25 kB (+10.9%)
      size-with-ext-deps38.0 kB39.3 kB+1.25 kB (+3.3%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • fetchConfig

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.1 kB12.3 kB+1.25 kB (+11.2%)
      size-with-ext-deps37.7 kB38.9 kB+1.25 kB (+3.3%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • getAll

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.7 kB12.9 kB+1.20 kB (+10.2%)
      size-with-ext-deps38.3 kB39.5 kB+1.20 kB (+3.1%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      14 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      15 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • getBoolean

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.5 kB12.7 kB+1.20 kB (+10.4%)
      size-with-ext-deps38.1 kB39.3 kB+1.20 kB (+3.1%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      14 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      15 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • getNumber

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.5 kB12.7 kB+1.20 kB (+10.4%)
      size-with-ext-deps38.1 kB39.3 kB+1.20 kB (+3.1%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      14 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      15 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • getRemoteConfig

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size10.6 kB11.8 kB+1.20 kB (+11.3%)
      size-with-ext-deps44.3 kB45.5 kB+1.20 kB (+2.7%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • getString

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.5 kB12.7 kB+1.20 kB (+10.4%)
      size-with-ext-deps38.1 kB39.3 kB+1.20 kB (+3.1%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      14 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      15 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • getValue

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size11.5 kB12.7 kB+1.20 kB (+10.4%)
      size-with-ext-deps38.1 kB39.3 kB+1.20 kB (+3.2%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      14 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      15 dependencies

      APP_NAMESPACE_STORE
      BOOLEAN_TRUTHY_VALUES
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      DEFAULT_VALUE_FOR_BOOLEAN
      DEFAULT_VALUE_FOR_NUMBER
      DEFAULT_VALUE_FOR_STRING
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • isSupported

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size10.7 kB11.8 kB+1.20 kB (+11.2%)
      size-with-ext-deps37.2 kB38.4 kB+1.20 kB (+3.2%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

    • setCustomSignals

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size?12.0 kB? (?)
      size-with-ext-deps?38.6 kB? (?)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      functions?

      ensureInitialized
      getUserLanguage
      isRetriableError
      openDatabase
      registerRemoteConfig
      setAbortableTimeout
      setCustomSignals
      toFirebaseError

      ?
      classes?

      CachingClient
      RemoteConfig
      RestClient
      RetryingClient
      Storage
      StorageCache

      ?
      variables?

      13 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      RC_CUSTOM_SIGNAL_VALUE_MAX_LENGTH
      name
      version

      ?
      enums??

      External Dependency

      ModuleBase (0f5714b)Merge (1d6176d)Diff
      @firebase/app?

      SDK_VERSION
      _registerComponent
      registerVersion

      ?
      @firebase/component?

      Component

      ?
      @firebase/logger?

      LogLevel
      Logger

      ?
      @firebase/util?

      ErrorFactory
      FirebaseError
      calculateBackoffMillis
      getModularInstance
      isIndexedDBAvailable

      ?
    • setLogLevel

      Size

      TypeBase (0f5714b)Merge (1d6176d)Diff
      size10.7 kB11.9 kB+1.20 kB (+11.2%)
      size-with-ext-deps37.3 kB38.5 kB+1.20 kB (+3.2%)

      Dependency

      TypeBase (0f5714b)Merge (1d6176d)Diff
      variables

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      name
      version

      11 dependencies

      APP_NAMESPACE_STORE
      DB_NAME
      DB_VERSION
      DEFAULT_CACHE_MAX_AGE_MILLIS
      DEFAULT_FETCH_TIMEOUT_MILLIS
      ERROR_DESCRIPTION_MAP
      ERROR_FACTORY
      RC_COMPONENT_NAME
      RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
      name
      version

      + RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/ngcURvTg7d.html

- Add new `getWithTransaction` and
`setWithTransaction` APIs
- Refactor generic `get` and `set` APIs for code
reuse
- Update `setCustomSignals` to perform upsert
operation in a single transaction.
@@ -9,6 +9,12 @@ import { FirebaseApp } from '@firebase/app';
// @public
export function activate(remoteConfig: RemoteConfig): Promise<boolean>;

// @public
export interface CustomSignals {
// (undocumented)

Choose a reason for hiding this comment

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

Hmm. This would appear to indicate we've missed documentation on a public API, but I see a comment on CustomSignals in public_types.ts, so it's unclear what more we should be doing. This would be a question for the core JS team. I see getRemoteConfig is also flagged as undocumented, so this may be a non-issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

This indicates there's no documentation comment for the key/value pair here which it looks like probably isn't needed here given the thorough documentation on the parent interface.

packages/firebase/compat/index.d.ts Outdated Show resolved Hide resolved
packages/remote-config/src/storage/storage.ts Show resolved Hide resolved
- Removed changes to firebase/compat and
remote-config-compat packages.
@ashish-kothari ashish-kothari marked this pull request as ready for review October 30, 2024 18:46
@ashish-kothari ashish-kothari requested review from a team as code owners October 30, 2024 18:46
Copy link
Contributor

github-actions bot commented Oct 30, 2024

Changeset File Check ✅

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

@ashish-kothari ashish-kothari force-pushed the ashish-kothari/rc-custom-targeting branch from f0a9278 to 4dd52d3 Compare October 30, 2024 18:52
@ashish-kothari ashish-kothari force-pushed the ashish-kothari/rc-custom-targeting branch from 4dd52d3 to 805db83 Compare October 30, 2024 18:54
@erikeldridge
Copy link

erikeldridge commented Nov 1, 2024

I heard my approval motivated some discussion, so I'll add additional context:

  • I was approving as a teammate who is familiar with the change, to indicate this change matches the proposed design.
  • I was assuming there would be a follow up review from the platform team before the change could be merged, and that this would be enforced by my lack of merge permission. The "Code owner review required" notice above is consistent with my expectation 👍
  • To reiterate the two points above, I've heard this referred to as a two-step review process. The first review comes from inside the team and indicates the change is ready for a final review from outside the team. (I think we're all familiar with this approach, but I'm over-communicating since there are multiple parties involved, our comms are async, and an overall working agreement is still in progress.)
  • I was following the pattern we used for recent Admin SDK changes, documented by go/rc-admin-sdk#review, but I see we don't have corresponding documentation on go/rc-web-g3doc, which covers this repo. I can add that.

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.

4 participants