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

[V2] Upgrade to RN72.6 #2037

Closed
wants to merge 6 commits into from
Closed

[V2] Upgrade to RN72.6 #2037

wants to merge 6 commits into from

Conversation

iBotPeaches
Copy link

What

This PR adds RN72 into /Example for testing confirmation of compatibility on the v2 branch as well as upgrading to the latest in reanimated v3.

Changes

  • Changes were made following upgrade guide, skipping anything was already pre-modified.
  • This entails things like
    • Skipping Flipper
    • ESLint
    • Jest

Tested on

  • Galaxy A13 5G
  • Pixel 7 Pro
  • iPhone 13 Pro

Related issues

Nothing related. I recently fixed frame processors on v2 on the example app here. I did this in hopes that it would fix our plugin (https://github.com/sourcetoad/vision-camera-plugin-barcode-scanner) and internal application that are still on v2.

I did this because our application, our sample plugin application and the vision sample application all had the same exact error of some form of ReanimatedError: property: __scanQRCodes doesn't exist

While I fixed the sample here - it did not solve the problem on our plugin or internal application. So once I realized our internal application was on RN72 and this was not, I upgraded the example to RN72 to presumably see an error. However, I did not which saddened me.

This points at an undiscovered error on my side with our plugin. So I figured I might as well upstream my work on v2 for RN72.

video attached of proof.

vision.mp4

@vercel
Copy link

vercel bot commented Oct 18, 2023

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

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 19, 2023 7:53pm

@iBotPeaches
Copy link
Author

iBotPeaches commented Oct 18, 2023

Oops. good thing for CI. Will work on that.

@iBotPeaches iBotPeaches marked this pull request as draft October 18, 2023 22:54
@mrousavy
Copy link
Owner

Thanks for your PR, Great stuff! Is this ready for review?

@iBotPeaches
Copy link
Author

Thanks for your PR, Great stuff! Is this ready for review?

Not yet, can't figure out Android CI failure. Still looking into it. Ignoring the JS lint failures as those have been broken since reanimated 3 merge.

@jeroenvanhattem
Copy link

Could we get some eyes on this? The updating of react-native-reanimated maybe fixes #2207 as well?

@iBotPeaches
Copy link
Author

Could we get some eyes on this? The updating of react-native-reanimated maybe fixes #2207 as well?

Eyes for what? This just updates the example app. I did this because I couldn't isolate the issues I had in our application. I wanted to use/update the example in order to replicate my problem.

I did, I submitted fix and then kinda put this on backburner. Updating the example RN version wouldn't do anything directly.

@iBotPeaches
Copy link
Author

Closing as 72 is now outdated and if I revisit I'll jump to 73.

@iBotPeaches iBotPeaches deleted the rn72-v2 branch December 21, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants