Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add new architecture support #6966

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

Draft
j-piasecki wants to merge 20 commits into invertase:main
base: main
Choose a base branch
Loading
from j-piasecki:@jpiasecki/app-turbomodule

Conversation

Copy link

@j-piasecki j-piasecki commented Mar 6, 2023
edited
Loading

PR adding New Architecture support to the library 🎉

We at Software Mansion have been working on improving support for the new architecture for quite a while now. If you need help with anything related to New Architecture, like:

or you just want to ask any questions, hit us up on projects@swmansion.com


Description

This PR aims to provide the new architecture support in the library, currently migrated packages are:

  • app
  • analytics
  • perf
  • crashlytics

Added FabricExample will most probably be removed when the PR is done, but it makes the testing somewhat easier.

Related issues

Release Summary

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


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

🔥

faceyspacey reacted with thumbs up emoji kacperkapusciak, ha3, iamchrispham, jeongshin, andresribeiro, dmahajan980, trevorzablocki2, AndreiBehel, nmeibergen, rahul2992, and 2 more reacted with hooray emoji andresribeiro, dmahajan980, manuhook, if-fi, HamoBoker, and Pashkagreen reacted with heart emoji kacperkapusciak, ha3, iamchrispham, andresribeiro, mr-ryan-james, dmahajan980, if-fi, and HamoBoker reacted with rocket emoji faceyspacey and HamoBoker reacted with eyes emoji
Copy link

vercel bot commented Mar 6, 2023
edited
Loading

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 Mar 22, 2023 at 3:50PM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
react-native-firebase-next ⬜️ Ignored (Inspect) Mar 22, 2023 at 3:50PM (UTC)

Copy link

CLAassistant commented Mar 6, 2023
edited
Loading

CLA assistant check
All committers have signed the CLA.

@j-piasecki j-piasecki changed the title (削除) @jpiasecki/app turbomodule (削除ここまで) (追記) Add new architecture support (追記ここまで) Mar 6, 2023
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

Copy link

Is anyone going to look into this?

Copy link

andresribeiro commented Apr 19, 2023
edited
Loading

i think they're waiting for 0.72 to be released to merge

faceyspacey reacted with thumbs up emoji

Copy link

does firebase/messaging needs to be migrated to new arch too ? @andresribeiro

Copy link

Is there any list of remaining tasks in order for this to work?

We use RNFB a lot and would like to get some benefits from new architecture as well, we also prefer to keep RNFB if possible instead of switching to something else, so we are happy to contribute based on an estimation of efforts required.

jphilipstevens and rogerkerse reacted with thumbs up emoji

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

Copy link
Collaborator

This is on my radar and it's getting closer to possible with both mixed new/old modules supported and use_frameworks better supported - apologies for the delay / lack of communication. react-native-firebase is actively supported and that necessarily implies that new architecture is very interesting to merge!

rvasseur31, zeallat, HamoBoker, reichhartd, lexengineer, Nantris, and henninghall reacted with heart emoji HamoBoker reacted with rocket emoji

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

Copy link

Hello, is there any news on unifying support for the new architecture?

Copy link
Collaborator

In react-native 0.72+ there are reports that running New Architecture in your app works, as long as you are in the transition / bridged mode - bridgeless mode is not reported to work. I have not tested it myself.

We are not currently working on actually converting to New Architecture yet, but as it is the future, we'll have to at some point

Copy link

Thanks for the answer! We’ll have to try, but of course it would be very cool if react-native-firebase received full support faster; this is one of the few libraries (and at the same time the most important) that is slowing down our transition to a new architecture =(

rvasseur31 reacted with thumbs up emoji

Copy link
Collaborator

If this module works (as reported success indicates it does) using the interop layer they added in 0.72 I believe that unblocks your app and everyone though, even without this module going through the internal rebuild?

That's my understanding, I'm hoping that's correct - use_frameworks support was totally blocking us but now that it's not we would be the blocker if we're not working (at least with interop layer) and I'm hoping we are not the blocker any longer

Copy link

Hi @mikehardy, do you think this can be reopend and merged?

Copy link
Collaborator

I can reopen it - are you willing to test it @GideonAgboba ? Invertase has native new architecture support on the radar but I must emphasize: there is no problem running react-native-firebase currently with new architecture enabled in bridgeless mode. There is nothing currently stopping you from using it. I believe the large-scale roadmap here is to deprecate the old firebase-js-sdk API (currently in process) so that the modular API is the only one in use, then migrate to typescript, and then new architecture support. With Vertex AI support in there somewhere as well. Each of these is large jobs though and in open source work ETAs should never be promised, as there are not really paying customers right?

vladimirivanoviliev reacted with eyes emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Keep Open avoids the stale bot

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /