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

fix(database,typescript): transaction update types #6782

Merged

Conversation

alexkuttig
Copy link
Contributor

Description

There was an error in the type definition of transactions of the realtime database. When using transactions with non-object values like number or boolean, tsc threw an error (TS2345: Argument of type '(data: object) => number' is not assignable to parameter of type '(currentData: object) => object'.   Type 'number' is not assignable to type 'object'.). When comparing the type definition of react-native-firebase with the one of the official firebase documentation here: https://firebase.google.com/docs/reference/js/v8/firebase.database.Reference#transaction you can see the difference. I changed the type definitions accordingly.

Related issues

Release Summary

fixed typedefinition for realtime database transactions

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

You can test the new type definition by creating a realtime database transaction with a non object type (like the example in the comment).

const userRef = firebase.database().ref('users/ada/profileViews);

userRef.transaction((currentViews) => {
  if (currentViews === null) return 1;
  return currentViews + 1;
});

With the old definition, this will throw an error. With the new definition, this will work.

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Dec 14, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 14, 2022 at 2:45PM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
react-native-firebase-next ⬜️ Ignored (Inspect) Dec 14, 2022 at 2:45PM (UTC)

@alexkuttig alexkuttig changed the title fixed transation update types fix: transacion update types Dec 14, 2022
@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Merging #6782 (32cadd3) into main (e3bd896) will decrease coverage by 18.63%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##               main    #6782       +/-   ##
=============================================
- Coverage     72.22%   53.59%   -18.62%     
- Complexity        0      690      +690     
=============================================
  Files           119      218       +99     
  Lines          4855    10895     +6040     
  Branches       1083     1746      +663     
=============================================
+ Hits           3506     5838     +2332     
- Misses         1266     4739     +3473     
- Partials         83      318      +235     

@alexkuttig alexkuttig changed the title fix: transacion update types fix(database,typescript): transacion update types Dec 14, 2022
@alexkuttig alexkuttig changed the title fix(database,typescript): transacion update types fix(database,typescript): transaction update types Dec 14, 2022
@github-actions
Copy link

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Jan 11, 2023
@github-actions github-actions bot closed this Jan 26, 2023
@mikehardy
Copy link
Collaborator

This still needs a review! I'm behind, apologies

@mikehardy mikehardy reopened this Jan 26, 2023
@github-actions github-actions bot removed the Stale label Jan 26, 2023
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this, it looks correct, thank you!

@mikehardy mikehardy added Workflow: Pending Merge Waiting on CI or similar plugin: database Firebase Realtime Database tools: typings TypeScript / Flow Type: Bug Fix labels Jan 27, 2023
@mikehardy mikehardy merged commit 1788a2d into invertase:main Jan 27, 2023
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: database Firebase Realtime Database tools: typings TypeScript / Flow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants