refactor(remote-config)!: migrate to TypeScript#8972
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request refactors the Remote Config package to use TypeScript and aligns its structure with the modular API of the Firebase JS SDK. Key changes include the introduction of comprehensive type definitions for internal, namespaced, and modular APIs, the implementation of modular API functions, and the adoption of react-native-builder-bob for the build process. Feedback was provided regarding the reset method implementation on iOS, suggesting a cleaner way to resolve the promise.
…at still have methods (such as StorageReference) and fire console warning incorrectly
|
Updated to main and re-pushed to verify things still work since we're not using a merge queue here release notes will come from the commit, so here's the draft of the commit message pulled from the description: BREAKING CHANGE: remote-config types now match firebase-js-sdk as closely as possible Please see https://rnfirebase.io/migrating-to-v25 for help migrating if needed react-native-firebase has a goal to be a drop-in replacement for firebase-js-sdk, with native extensions and performance. It has always worked that way at the javascript level but the typescript types have been divergent We are fixing that as we refactor to typescript. Please bear with us as we get closer to our goal of react-native-firebase matching firebase-js-sdk both in functionality where possible, but also in exact typescript typing. Specifics for Remote Config:
|
mikehardy
left a comment
There was a problem hiding this comment.
Looks good to go to me, excellent!
| } | ||
| return Reflect.get(target, prop, receiver); | ||
| }, | ||
| set(target: any, prop: string | symbol, value: unknown, receiver: unknown) { |
There was a problem hiding this comment.
you weren't wrong, this is an ever-deepening rabbit-hole of proxied introspective javascript trickery in order to emit these deprecation warnings - will be great when it's modular-only
| // To make Firebase web v9 API compatible, we update the settings first so it immediately | ||
| // updates settings on the instance. We then pass to underlying SDK to update. We do this because | ||
| // there is no way to "await" a setter. We can't delegate to `setConfigSettings()` as it is setup | ||
| // for native. |
There was a problem hiding this comment.
was curious how you were going to handle the lack of async on the properties, I think this is fair - restriction will go away with TurboModules in the future and in the present having it work consistently (if non-atomically) seems reasonable as a tradeoff
|
remote-config CI checks just passed (I'm watching real-time) - clear for merge |
Description
Breaking Changes
breaking: the primary modular remote-config types now use Firebase JS SDK names:
LogLevel,FetchStatus,Value, andRemoteConfigSettings. types/remote-config.ts#L21-L37breaking:
RemoteConfig.settingsis now typed asRemoteConfigSettings, which usesfetchTimeoutMillisrather than the older RNFB-stylefetchTimeMillison the modular surface. types/remote-config.ts#L34-L37 types/remote-config.ts#L55-L63breaking: modular
getAll()andgetValue()now return SDK-aligned types:Record<string, Value>andValue. modular.ts#L91-L123breaking: modular
setLogLevel()now matches the Firebase JS SDK signature and returnsvoid. modular.ts#L126-L133breaking: the legacy modular helper exports
fetchTimeMillis(),settings(), andlastFetchStatus()have been removed from@react-native-firebase/remote-config. Modular callers should readremoteConfig.fetchTimeMillis,remoteConfig.settings, andremoteConfig.lastFetchStatusfrom theRemoteConfiginstance instead. types/remote-config.ts#L55-L63breaking: modular
fetch()has been removed. Modular callers should usefetchConfig(remoteConfig)instead; the RNFB-only modularexpirationDurationSecondshelper is no longer part of the public modular API. modular.ts#L81-L86breaking: modular
setConfigSettings()andsetDefaults()have been removed. Modular callers should useremoteConfig.settings = ...andremoteConfig.defaultConfig = ...on theRemoteConfiginstance instead. types/remote-config.ts#L55-L63breaking: modular
onConfigUpdated()has been removed. Modular callers should useonConfigUpdate(remoteConfig, observer)instead. modular.ts#L165-L174 types/remote-config.ts#L39-L49breaking: deprecated RemoteConfigValue.value and .source getters have been removed. Callers should use asString() and getSource() instead. 69ffd4c
breaking - Remove LastFetchStatus, ValueSource, ConfigSettings, ConfigDefaults, ConfigValue, ConfigValues, LastFetchStatusType, and RemoteConfigLogLevel from modular exports f22f19b
Related issues
Release Summary
Checklist
AndroidiOSOther(macOS, web)e2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter