-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: add Fabric on iOS without ComponentViews #1821
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments regarding React Native 0.69+, but then I noticed that you went for 0.68.1 in FabricExample
, so thats fine.
Pointed some style-regarding things below.
source 'https://rubygems.org' | ||
|
||
# You may use http://rbenv.org/ or https://rvm.io/ to install and use this version | ||
ruby '2.7.4' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ruby was bumped up to '2.7.5' with React Native 0.69.0 however I doubt it would cause any problems.
libfolly_futures \ | ||
libfolly_json \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also was changed in recent React Native versions.
libfolly_futures \ | |
libfolly_json \ | |
libfolly_runtime \ |
Most of Android changes for Fabric and bump of FabricExample to RN 0.69.2. iOS and JS changes are available in #1821. The most notable change on Android is adding methods to components that accept String values of each NumberProp instead of Dynamic. Another change is changed structure of RenderableViewManager.java since we needed to abstract methods that belong only to components inheriting from VirtualView in order to be able to properly override them in their children.
Version of software-mansion#1754 without usage of ComponentViews. It seems like a more proper way, but introduces the necessity of clearing whole state of each component on recycling for it not to be used when view is recycled. Still known problems: We stringify props of type NumberProp since codegen only accepts props of a single type. It is the fastest way of dealing with it, but maybe there could be a better way to handle it. Image resolving should be probably handled the same as in RN core SvgView needs to set opaque = NO so it is does not have black background (it comes from the fact that RCTViewComponentView overrides backgroundColor setter which in turn somehow messes with the view being opaque). All other svg components do it already so maybe it is not such an issue. transform prop won't work when set natively since it is not parsed in Fabric
Version of software-mansion#1754 without usage of ComponentViews. It seems like a more proper way, but introduces the necessity of clearing whole state of each component on recycling for it not to be used when view is recycled. Still known problems: We stringify props of type NumberProp since codegen only accepts props of a single type. It is the fastest way of dealing with it, but maybe there could be a better way to handle it. Image resolving should be probably handled the same as in RN core SvgView needs to set opaque = NO so it is does not have black background (it comes from the fact that RCTViewComponentView overrides backgroundColor setter which in turn somehow messes with the view being opaque). All other svg components do it already so maybe it is not such an issue. transform prop won't work when set natively since it is not parsed in Fabric
Summary
Version of #1754 without usage of
ComponentView
s. It seems like a more proper way, but introduces the necessity of clearing whole state of each component on recycling for it not to be used when view is recycled.Still known problems:
NumberProp
since codegen only accepts props of a single type. It is the fastest way of dealing with it, but maybe there could be a better way to handle it.opaque = NO
so it is does not have black background (it comes from the fact thatRCTViewComponentView
overridesbackgroundColor
setter which in turn somehow messes with the view beingopaque
). All other svg components do it already so maybe it is not such an issue.transform
prop won't work when set natively since it is not parsed in Fabric