Fixed new arch support #741
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Oh boy.
Changes
Why
New arch support was broken
Background
The new arch is mainly 2 components from our stand point:
The new arch support was generously contributed, however as the new arch has changed, so has the APIs we need to use, the tooling, and the code we need to write, and the code we wrote.
For reasons that aren't clear, the view component was probably never working on iOS, and possibly never on Android either. It's a wonder it even compiled as dead code.
There are a multitude of issues that I am addressing with this PR, but it centers around Codegen.
Codegen is really broken and needs some attention, as it's at the heart of the new arch.
Codegen doesn't work out of the box
includesGeneratedCodesetting and base its behavior on it (warn/error/generate differently). But it does not.android/build.gradlemust be updated is undocumented, but I found that using a flag served no purpose despite what others are doingThis broke the
zoom,maxZoom, and a few other properties. This wasn't directly fixable. The PR rewrites everything to use-1as a stand-in forundefinedview properties where necessary. The community is aware of this shortcoming but there is no fix in sight:Fabric component: codegen drops optional state for component properties and in events properties facebook/react-native#49920 (comment)
cmakeListsPathseems to be a requirement that should match codegen, but you have to guess how it works because docs are (also) missing for thisCMakeLists.txtfile that causes the consuming app to crash on startup with a opaque segmentation fault. Through debugging it turns outtarget_compile_reactnative_optionsis the solution in 0.80 and newer, but in older apps you need the old crashy code. This was only technically documented in a blog post, and is technically a thing that can break if you decide to use "includesGeneratedCode". It would have been nice if they had included this in codegen instead of/in addition to writing a blog post about it.In addition to this, the docs can easily lead to issues in your library because of documentation like this:
https://github.com/reactwg/react-native-new-architecture/blob/main/docs/fabric-native-components.md
Pay special attention to how header files are included here. This causes issues like unable to find files like:
and
Which are C++ standard library stuff, which I could guess, but unfortunately all the AIs in the world will also tell you. They will lead you down a path of trying to add C++ support to your pod, but the truth is in these issues instead:
CocoaPods/CocoaPods#12105 (comment)
facebook/react-native#45424 (comment)
TLDR: Don't include .h files next with .swift files. Swift compiler can't handle it.