-
Notifications
You must be signed in to change notification settings - Fork 0
ios new arch compatibility #24
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
base: 3.19.4-discord
Are you sure you want to change the base?
Changes from all commits
139a131
e6476e3
c83b298
3cc7d70
a7a923b
16eb74d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,7 +102,25 @@ RequestRenderFunction makeRequestRender(REANodesManager *nodesManager) | |
| } | ||
|
|
||
| #ifdef RCT_NEW_ARCH_ENABLED | ||
| // nothing | ||
| PreserveMountedTagsFunction makePreserveMountedTagsFunction() | ||
| { | ||
| //std::function<std::optional<std::unique_ptr<int[]>>(std::vector<int> &)>; | ||
| auto preserveMountedTagsFunction = [](std::vector<int> &tags) -> std::optional<std::unique_ptr<int[]>> { | ||
| // TODO: Implement me | ||
| return nullptr; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This didn't seem to be called by ios new arch and can maybe be left as a noop. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, thats noop on iOS 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just wondering if we can also ifdef android that 🤔 thats how SWM did it in the v4 code: but i'd also be fine with having this here + a comment 🤷 |
||
| }; | ||
| return preserveMountedTagsFunction; | ||
| } | ||
|
|
||
| SynchronouslyUpdateUIPropsFunction makeSynchronouslyUpdateUIPropsFunction(REANodesManager *nodesManager) | ||
| { | ||
| auto synchronouslyUpdateUIPropsFunction = [nodesManager](jsi::Runtime &rt, Tag tag, const jsi::Object &props) -> void { | ||
| NSNumber *viewTag = @(tag); | ||
| NSDictionary *uiProps = convertJSIObjectToNSDictionary(rt, props); | ||
| [nodesManager synchronouslyUpdateViewOnUIThread:viewTag props:uiProps]; | ||
| }; | ||
| return synchronouslyUpdateUIPropsFunction; | ||
| } | ||
| #else // RCT_NEW_ARCH_ENABLED | ||
| UpdatePropsFunction makeUpdatePropsFunction(REAModule *reaModule) | ||
| { | ||
|
|
@@ -279,7 +297,9 @@ KeyboardEventUnsubscribeFunction makeUnsubscribeFromKeyboardEventsFunction(REAKe | |
| auto requestRender = makeRequestRender(nodesManager); | ||
|
|
||
| #ifdef RCT_NEW_ARCH_ENABLED | ||
| // nothing | ||
| auto preserveMountedTagsFunction = makePreserveMountedTagsFunction(); | ||
|
|
||
| auto synchronouslyUpdateUIPropsFunction = makeSynchronouslyUpdateUIPropsFunction(nodesManager); | ||
| #else | ||
| RCTUIManager *uiManager = nodesManager.uiManager; | ||
| auto updatePropsFunction = makeUpdatePropsFunction(reaModule); | ||
|
|
@@ -338,7 +358,7 @@ KeyboardEventUnsubscribeFunction makeUnsubscribeFromKeyboardEventsFunction(REAKe | |
| PlatformDepMethodsHolder platformDepMethodsHolder = { | ||
| requestRender, | ||
| #ifdef RCT_NEW_ARCH_ENABLED | ||
| // nothing | ||
| synchronouslyUpdateUIPropsFunction, | ||
| #else | ||
| updatePropsFunction, | ||
| scrollToFunction, | ||
|
|
@@ -367,6 +387,10 @@ PlatformDepMethodsHolder makePlatformDepMethodsHolderBridgeless( | |
| REAModule *reaModule) | ||
| { | ||
| auto requestRender = makeRequestRender(nodesManager); | ||
|
|
||
| auto preserveMountedTagsFunction = makePreserveMountedTagsFunction(); | ||
|
|
||
| auto synchronouslyUpdateUIPropsFunction = makeSynchronouslyUpdateUIPropsFunction(nodesManager); | ||
|
|
||
| auto getAnimationTimestamp = makeGetAnimationTimestamp(); | ||
|
|
||
|
|
@@ -392,6 +416,7 @@ PlatformDepMethodsHolder makePlatformDepMethodsHolderBridgeless( | |
|
|
||
| PlatformDepMethodsHolder platformDepMethodsHolder = { | ||
| requestRender, | ||
| synchronouslyUpdateUIPropsFunction, | ||
| getAnimationTimestamp, | ||
| progressLayoutAnimation, | ||
| endLayoutAnimation, | ||
|
|
||
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 needs further investigation. In the monorepo this is throwing an invalid_argument exception but not being caught.
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.
yeah i remember we already fixed that once somewhere.
I think the bug was that
auto nativeId = stoi(nativeIdString);the nativeid is usually a number, but when the user sets thenativeIdprop as a regular text then this can throw, let me find what we did back then …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.
ah right, the std::invalid_argument should catch this but in the past didn't due to misconfigured rtti and exceptions flags:
we should double check if we did regress that?