feat(rc): Add custom signals support #8602

ashish-kothari · 2024-10-25T14:11:17Z

No description provided.

- Also takes care of setting the signals in the fetch request

changeset-bot · 2024-10-25T14:11:20Z

🦋 Changeset detected

Latest commit: 805db83

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

google-oss-bot · 2024-10-25T14:25:04Z

Size Report ¹

Affected Products

@firebase/remote-config
Type Base (0f5714b) Merge (46e819b) Diff
browser 19.2 kB 20.5 kB +1.31 kB (+6.8%)
main 20.2 kB 21.6 kB +1.40 kB (+6.9%)
module 19.2 kB 20.5 kB +1.31 kB (+6.8%)
bundle
Type Base (0f5714b) Merge (46e819b) Diff
remote-config (getAndFetch) 46.3 kB 47.1 kB +807 B (+1.7%)
firebase
Type Base (0f5714b) Merge (46e819b) Diff
firebase-compat.js 794 kB 795 kB +779 B (+0.1%)
firebase-remote-config-compat.js 27.3 kB 28.1 kB +779 B (+2.9%)
firebase-remote-config.js 29.6 kB 30.5 kB +914 B (+3.1%)

Test Logs

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

google-oss-bot · 2024-10-25T14:37:33Z

Size Analysis Report ¹

Affected Products

`@firebase/remote-config`

activate
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 10.8 kB 11.6 kB +757 B (+7.0%)
size-with-ext-deps 37.4 kB 38.2 kB +758 B (+2.0%)
ensureInitialized
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 10.5 kB 11.3 kB +757 B (+7.2%)
size-with-ext-deps 37.1 kB 37.9 kB +758 B (+2.0%)
fetchAndActivate
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.4 kB 12.3 kB +806 B (+7.0%)
size-with-ext-deps 38.0 kB 38.8 kB +807 B (+2.1%)
fetchConfig
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.1 kB 11.9 kB +806 B (+7.3%)
size-with-ext-deps 37.7 kB 38.5 kB +807 B (+2.1%)
getAll
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.7 kB 12.5 kB +757 B (+6.5%)
size-with-ext-deps 38.3 kB 39.1 kB +758 B (+2.0%)
getBoolean
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.5 kB 12.3 kB +757 B (+6.6%)
size-with-ext-deps 38.1 kB 38.9 kB +758 B (+2.0%)
getNumber
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.5 kB 12.3 kB +757 B (+6.6%)
size-with-ext-deps 38.1 kB 38.9 kB +758 B (+2.0%)
getRemoteConfig
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 10.6 kB 11.4 kB +757 B (+7.1%)
size-with-ext-deps 44.3 kB 45.1 kB +758 B (+1.7%)
getString
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.5 kB 12.3 kB +757 B (+6.6%)
size-with-ext-deps 38.1 kB 38.9 kB +758 B (+2.0%)
getValue
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 11.5 kB 12.3 kB +757 B (+6.6%)
size-with-ext-deps 38.1 kB 38.9 kB +758 B (+2.0%)
isSupported
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 10.7 kB 11.4 kB +757 B (+7.1%)
size-with-ext-deps 37.2 kB 38.0 kB +758 B (+2.0%)

`setCustomSignals`

Size

Type	Base (`0f5714b`)	Merge (`46e819b`)	Diff
size	?	11.4 kB	? (?)
size-with-ext-deps	?	38.0 kB	? (?)

Dependency

Type	Base (`0f5714b`)	Merge (`46e819b`)	Diff
functions	?	ensureInitialized getUserLanguage isRetriableError openDatabase registerRemoteConfig setAbortableTimeout setCustomSignals toFirebaseError	?
classes	?	CachingClient RemoteConfig RestClient RetryingClient Storage StorageCache	?
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	?
enums	?		?

External Dependency

Module	Base (`0f5714b`)	Merge (`46e819b`)	Diff
`@firebase/app`	?	SDK_VERSION _registerComponent registerVersion	?
`@firebase/component`	?	Component	?
`@firebase/logger`	?	LogLevel Logger	?
`@firebase/util`	?	ErrorFactory FirebaseError calculateBackoffMillis getModularInstance isIndexedDBAvailable	?

setLogLevel
Size
Type Base (0f5714b) Merge (46e819b) Diff
size 10.7 kB 11.4 kB +757 B (+7.1%)
size-with-ext-deps 37.3 kB 38.0 kB +758 B (+2.0%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/jgEEZd6y4c.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.

erikeldridge · 2024-10-28T16:17:51Z

common/api-review/remote-config.api.md

@@ -9,6 +9,12 @@ import { FirebaseApp } from '@firebase/app';
 // @public
 export function activate(remoteConfig: RemoteConfig): Promise<boolean>;

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


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.

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

packages/remote-config/test/storage/storage.test.ts

packages/remote-config/src/storage/storage.ts

packages/remote-config-compat/src/remoteConfig.ts

- Removed changes to firebase/compat and remote-config-compat packages.

github-actions · 2024-10-30T18:47:19Z

Changeset File Check ✅

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

erikeldridge · 2024-11-01T19:47:29Z

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.

ashish-kothari added 6 commits October 25, 2024 19:33

Add support to set/update custom signals

4cfd43b

- Also takes care of setting the signals in the fetch request

Add explanatory comments

2bd7dc0

Minor fixes in the custom signals storage API

33444c5

Add setCustomSignals() to RemoteConfigCompat

ea317aa

Add setCustomSignals() under firebase/compat

ad74ca5

Address lint errors + add/fix few tests

77f1cb6

ashish-kothari self-assigned this Oct 25, 2024

ashish-kothari requested review from erikeldridge and a team as code owners October 25, 2024 14:11

ashish-kothari changed the title ~~feat(RC): Add custom signals support~~ feat(rc): Add custom signals support Oct 25, 2024

Add changeset

ed9a8b1

ashish-kothari requested review from 9kubczas4 and removed request for 9kubczas4 October 25, 2024 14:22

ashish-kothari marked this pull request as draft October 25, 2024 14:32

ashish-kothari added 5 commits October 25, 2024 20:16

Run yarn format

26ce7e1

Run yarn docgen:all

f3a0f16

Add & update unit tests

44015b1

Run yarn format

3f7a55c

Refactor setCustomSignals storage API

95df4e4

- 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.

erikeldridge approved these changes Oct 28, 2024

View reviewed changes

hsubox76 reviewed Oct 28, 2024

View reviewed changes

packages/remote-config-compat/src/remoteConfig.ts Outdated Show resolved Hide resolved

Add unit test + improve public API documentation

ff99a7c

- Removed changes to firebase/compat and remote-config-compat packages.

ashish-kothari marked this pull request as ready for review October 30, 2024 18:46

ashish-kothari requested review from a team as code owners October 30, 2024 18:46

ashish-kothari force-pushed the ashish-kothari/rc-custom-targeting branch from f0a9278 to 4dd52d3 Compare October 30, 2024 18:52

Update changeset

805db83

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rc): Add custom signals support #8602

feat(rc): Add custom signals support #8602

ashish-kothari commented Oct 25, 2024

changeset-bot bot commented Oct 25, 2024 •

edited

Loading

google-oss-bot commented Oct 25, 2024 •

edited

Loading

`@firebase/remote-config`

`bundle`

`firebase`

google-oss-bot commented Oct 25, 2024 •

edited

Loading

`@firebase/remote-config`

`activate`

`ensureInitialized`

`fetchAndActivate`

`fetchConfig`

`getAll`

`getBoolean`

`getNumber`

`getRemoteConfig`

`getString`

`getValue`

`isSupported`

`setCustomSignals`

`setLogLevel`

erikeldridge Oct 28, 2024

hsubox76 Oct 28, 2024

github-actions bot commented Oct 30, 2024 •

edited

Loading

erikeldridge commented Nov 1, 2024 •

edited

Loading

feat(rc): Add custom signals support #8602

Are you sure you want to change the base?

feat(rc): Add custom signals support #8602

Conversation

ashish-kothari commented Oct 25, 2024

changeset-bot bot commented Oct 25, 2024 • edited Loading

🦋 Changeset detected

google-oss-bot commented Oct 25, 2024 • edited Loading

Size Report 1

Affected Products

@firebase/remote-config

bundle

firebase

Test Logs

google-oss-bot commented Oct 25, 2024 • edited Loading

Size Analysis Report 1

Affected Products

@firebase/remote-config

activate

ensureInitialized

fetchAndActivate

fetchConfig

getAll

getBoolean

getNumber

getRemoteConfig

getString

getValue

isSupported

setCustomSignals

setLogLevel

Test Logs

erikeldridge Oct 28, 2024

Choose a reason for hiding this comment

hsubox76 Oct 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 30, 2024 • edited Loading

Changeset File Check ✅

erikeldridge commented Nov 1, 2024 • edited Loading

changeset-bot bot commented Oct 25, 2024 •

edited

Loading

google-oss-bot commented Oct 25, 2024 •

edited

Loading

Size Report ¹

`@firebase/remote-config`

`bundle`

`firebase`

google-oss-bot commented Oct 25, 2024 •

edited

Loading

Size Analysis Report ¹

`@firebase/remote-config`

`activate`

`ensureInitialized`

`fetchAndActivate`

`fetchConfig`

`getAll`

`getBoolean`

`getNumber`

`getRemoteConfig`

`getString`

`getValue`

`isSupported`

`setCustomSignals`

`setLogLevel`

github-actions bot commented Oct 30, 2024 •

edited

Loading

erikeldridge commented Nov 1, 2024 •

edited

Loading