Repository navigation
fix(plugin-firebase): use the modular API functions instead of instance methods - #1341
Open
Jack-Gill-TH wants to merge 1 commit into
Open
Jack-Gill-TH wants to merge 1 commit into
Jack-Gill-TH wants to merge 1 commit into
Conversation
…ce methods React Native Firebase v22 deprecates calling methods on the analytics instance. segmentio#1075 created the instance with getAnalytics(getApp()), but the plugin still called logEvent, logScreenView, setUserId, setUserProperties and resetAnalyticsData on that instance. Each call logged a deprecation warning. Call the modular functions with the instance as the first argument. logScreenView is deprecated in both APIs, so send a screen_view event with logEvent and keep the screen_name and screen_class payload. Refs segmentio#1105
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #1075 (issue #1105). That change moved instance creation to the
modular
getAnalytics(getApp()), which removed thegetApp()warning. Theplugin still called methods on the returned instance, so React Native
Firebase still logged a deprecation warning for every call.
Changes
track:logEvent(analytics, name, props)identify:setUserId(analytics, id)andsetUserProperties(analytics, traits)reset:resetAnalyticsData(analytics)screen:logEvent(analytics, 'screen_view', props).logScreenViewisdeprecated in the namespaced and the modular API.
The
screen_viewoverload in the RNFB typings names the paramsfirebase_screenandfirebase_screen_class. The oldlogScreenViewforwarded
screen_nameandscreen_class, which the native SDKs read. Thepayload is unchanged, so
screen.tscasts the params type, with a comment.The unit tests now expect the modular functions with the instance as the
first argument. They also assert that the deprecated instance methods are not
called.
Testing
jestin the plugin: 4 suites, 11 tests pass. All 11 failed before the change.tsc --noEmit,eslintandprettier --checkpass. The typecheck runsagainst RNFB 22.2.0.
Plugin 1.0.0 produced 18 deprecation warnings for 18 Firebase calls. With
this change, 16 calls (
logEventx14,setUserId,setUserProperties) alltook the modular path and produced no warning.
Not tested:
resetat runtime (same pattern as the others), Android, and RNFBversions between 22.2.0 and 22.4.0 at runtime.