-
Notifications
You must be signed in to change notification settings - Fork 975
Process experiment metadata in RC fetch response #9276
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
Conversation
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1Affected Products
Test Logs |
36bd6d9
to
d3e0838
Compare
export interface FetchResponse { | ||
config?: FirebaseRemoteConfigObject; | ||
eTag?: string; | ||
// Warning: (ae-forgotten-export) The symbol "FirebaseExperimentDescription" needs to be exported by the entry point index.d.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't look far enough up and see that this is part of FetchResponse, which is already public. In that case I guess we need to export FirebaseExperimentDescription as well. You can keep the documentation on it minimal if you don't expect users to have to populate it. I was trying to see why FetchResponse needs to be public and the only place I see it exposed to users is that they can specify a "initialFetchResponse" in RemoteConfigOptions when they initialize remote config but I don't know where they get that object from. I assume from server-side RC somewhere, and they just pass it in. So in that case they probably don't inspect it and probably don't need much documentation about what's in it, but since they're handling it and passing it, I guess they need a somewhat accurate typing for it.
I'll leave it up to you and your tech writer how much or how little documentation you think it needs but if FetchResponse needs to stay public, then we need to at least export the types of everything in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
This change introduces experiment details in the response of
fetch
call in Remote Config. The change is split into 3 PRs where:This is 1/3 mentioned above
Design doc (internal): go/experiments-web