Skip to content
This repository has been archived by the owner on Jun 16, 2023. It is now read-only.

Use empty string for undefined cameraId #3117

Merged

Conversation

cristianoccazinsp
Copy link
Contributor

Following up from here (#3113) .

The reason the RNCamera component is always rendered twice (aka, a defaultView is created on the native side) is because of RCT_CUSTOM_VIEW_PROPERTY combined with a property that is set to null. When this happens, RN creates a "fake" default view to store default props. Although this works fine most of the time, it creates some unnecessary classes and resources being kept in memory even after the camera unmounts.

After digging a bit, the prop cameraId being null is the sole reason RN creates this defaultView. This PR uses an empty string as the "no value" value instead, removing the extra view creation.

Note: The change is not breaking, anyone using cameraId=null will continue to work as before. However, to prevent this extra view from being created, the prop must be set to cameraId='' instead.

Lastly, this only shows how we must be careful with null properties and resources acquired/released in the init/dealloc methods!

@cristianoccazinsp
Copy link
Contributor Author

@MarcoScabbiolo sorry that I keep adding changes that just adds more stuff for the v4 migration, this should be the last one! But since we constantly use this library, these fixes are a must for us.

Copy link
Collaborator

@MarcoScabbiolo MarcoScabbiolo left a comment

Choose a reason for hiding this comment

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

OK, didnt test myself.

@MarcoScabbiolo
Copy link
Collaborator

@MarcoScabbiolo sorry that I keep adding changes that just adds more stuff for the v4 migration, this should be the last one! But since we constantly use this library, these fixes are a must for us.

No problem, I'm keeping track of all the changes to make sure they land in v4 as well. Thanks for contributing.
BTW in v4 only RCT_EXPORT_VIEW_PROPERTY is used, so this should be fixed out of the box.

@MateusAndrade MateusAndrade merged commit bed16e9 into react-native-camera:master Feb 23, 2021
n1ru4l pushed a commit that referenced this pull request Feb 23, 2021
## [3.42.3](v3.42.2...v3.42.3) (2021-02-23)

### Bug Fixes

* Use empty string for undefined cameraId ([#3117](#3117)) ([bed16e9](bed16e9))
@n1ru4l
Copy link
Collaborator

n1ru4l commented Feb 23, 2021

🎉 This PR is included in version 3.42.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@cristianoccazinsp cristianoccazinsp deleted the fix-cameraid branch March 22, 2021 17:46
sunil-dev7 added a commit to sunil-dev7/react-native-camera that referenced this pull request Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants