Open danielrhodes opened 8 months ago
I've noticed this issue too after upgrading to 0.73 (expo 50). I've also noticed a memory leak issue (the number of views in the performance monitoring wouldn't stop going up after multiple mount unmounts). I believe that the preview isn't being unmounted correctly (i tried all the props related to garbage collection) .
Additionally, I noticed that the Auxiliary preview crashes when dismissed.
@valentinchelle I haven't been able to consistently reproduce it locally, but previously I found that it was quite vulnerable to remounts of the parent component / tree and fiddling with the cleanup may have helped somewhat.
Confirming we are getting the same issue:
sorry for the late response, i'll take a look and start debugging - progress log for issue #94
2024-02-06-17:08
- hi, during the refactor to expo, i moved a bunch of stuff around and accidentally broke the code that's responsible for the cleanup logic/routine.2024-02-06-17:38
- i've decided to re-write the logic that mounts the "aux. preview"/"custom preview" views + the logic that handles the cleanup routine (this might take some time) — in the meantime i'll push some temp. fixes.2024-02-06-19:44
- react-native-ios-context-menu
(Release: v2.3.1 | changes)
ContextMenuView
render function.2024-02-06-21:31
- react-native-ios-context-menu
(Release: v2.3.2 | changes)
nilReactBridge
and viewNotFoundForReactTag
errors (i.e. silent failure). 2024-02-06-21:44
- react-native-ios-utilities
(Release: v4.2.4 | changes)
RNIDetachedView.shouldNotifyOnComponentWillUnmount
, and suppress nilReactBridge
error from RNIDetachedViewModule.notifyOnComponentWillUnmount
.2024-02-07-02:17
- react-native-ios-utilities
(Release: v4.3.0 | changes)
v4.3.0-11
and v4.2.4
.2024-02-07-16:34
- react-native-ios-context-menu
(Release: v2.4.0 | changes)
react-native-ios-adaptive-modal
- Raise min. dep. version to react-native-ios-utilities@v4.3.0
, and fix native build/TS type errors.2024-02-07-17:24
- DGSwiftUtilities
(Release: 0.13.0 | changes)
HorizontalAlignmentPosition.createHorizontalConstraints
.2024-02-07-17:57
- ContextMenuAuxiliaryPreview
(Release: 0.4.0 | changes)
DGSwiftUtilities@0.13.0
, and fix exit animation bug that occurs when AuxiliaryPreviewConfig.horizontalAlignment
is set to HorizontalAlignmentPosition.stretch
.2024-02-08-03:57
- react-native-ios-context-menu
(Release: v2.4.1 | changes)
2024-02-08-15:40
- Log: Testing - Push multiple screens and pop them.
2024-02-08-15:45
- Log: Testing - Show and close the context menu example test items that have aux. previews; this is a test for RNIDetachedView
cleanup.
2024-02-12-22:14
- react-native-ios-context-menu
(Release: v2.5.0-0 | changes)
internalViewCleanupMode
prop.2024-02-13-05:06
- DGSwiftUtilities
(Release: 0.14.0 | changes)
DeinitialzationObserver
+ related sources from react-native-ios-utilities
.2024-02-13-05:22
- react-native-ios-utilities
(changes)
2024-02-13-05:32
- react-native-ios-context-menu
(Release: v2.5.0-1 | changes)
RNICleanableView
, and replace w/ impl. from react-native-ios-utilities
.2024-02-14-03:41
- DGSwiftUtilities
(changes)
2024-02-15-18:57
- react-native-ios-utilities
(changes)
2024-02-15-22:06
- react-native-ios-context-menu
(changes)
2024-02-15-23:31
- react-native-ios-utilities
(v4.4.0-2 | changes)
RNIDetachedView.internalViewCleanupMode
prop, update RNICleanableViewRegistry
, impl. RNIUtiltiesModule
+ RNICleanableViewRegistryEnv
. 2024-02-18-09:40
- DGSwiftUtilities
(changes)
2024-02-20-07:11
- react-native-ios-utilities
(Release: v4.4.0-3 | changes)
RNICleanableView
+ related sources.2024-02-21-01:07
- Log: react-native-ios-utilities
- Testing
RNIDetachedViewTest01
2024-02-26-10:01
- react-native-ios-utilities
(v4.4.0-4 | changes)
2024-02-27-02:26
- react-native-ios-utilities
(v4.4.0-5 | changes)
2024-02-27-18:57
- react-native-ios-context-menu
(Release: v2.5.0-2 | changes)
react-native-ios-utilities
, and update cleanup logic for RNIContextMenuView
, and RNIContextMenuButton
.2024-02-27-19:05
- # Log: react-native-ios-context-menu
- Testing
hello, it looks like the recent patches i've made has caused a bunch of crashes; i apologize if you were affected by this.
i'll write a short explanation on why the "clean up" function is needed (hopefully, with the help of he community, we can find a solution that's reliable):
RCTUIManager
singleton has a private variable called _viewRegistry
.
_viewRegistry
is a dictionary (i.e. NSDictionary<NSNumber, RCTView>
)_viewRegistry
.reactTag: NSNumber
property (which AFAIK just mirrors UIView.tag
).reactTag
together to reference a react view in the app's view hierarchy.
ReactNative.findNodeHandle
), but has since been deprecated in the new architecture (and should no longer be used).onReactTagDidSet
event, and pass the "react tag" value to JS.RCTUIManager._viewRegistry
keeps a strong a reference to every RCTView
instance to prevent them from vanishing into the void.
RCTUIManager
is a singleton, and will "live" for the entire duration of the app, anything inside _viewRegistry
will also "live" for the entire duration of the app.UIKit
, nevertheless RCTUIManager
does not like it when you interfere with an RCTView
's view lifecycle (e.g. moving it to a different view, removing it from it's parent, etc).RCTUIManager
, it'll no longer manage the lifecycle for that specific view instance.RCTView
instance to get "orphaned" and live forever.
CALayer
, as such it shouldn't be using any resources).reanimated
worklet), a crash will occur.onComponentWillUnmount
, and deinit
to trigger the cleanup routine.I encountered this issue before, and it was fixed by react-native-ios-context-menu
v2.4.0 and react-native-ios-utilities
v4.3.0 on my end.
Could you guys try it out?
I updated but this is still occurring for me, but I might have a better clue about what is happening:
I am using react-navigation, and if I click a menu item which triggers a screen change, it might cause this error. I believe this is because the menu has not closed by the time the view is removed.
I have no idea if this will fix it, but I am putting in a setTimeout after receiving an event - will see if that helps.
@danielrhodes hi, i need some sort of small repro/code snippet so i can debug this further (i'll add it as a test case in the example app for future reference). i'm having some trouble trying to recreate the issues you're describing.
if possible, can you please provide some screenshots/video of the performance monitor while re-creating the issue/bug? (you can crop the video via the photos app to redact the contents of your app)
(i've added some screen recordings in my previous reply)
@dominicstop I haven't been able to replicate it locally, but I am seeing it quite often from exception logs in production.
Here is an example project that isn't much different from my own to give you something to work with. https://github.com/danielrhodes/ios-context-menu-bug-example
yeah, we only found this error log in the production log. but so far I have discovered that this error has not caused the app to crash yet. so it's kind of weird...
@danielrhodes it looks like the view count in the performance monitor is not up to date somehow (there is no memory leak)
@danielrhodes it looks like the view count in the performance monitor is not up to date somehow (there is no memory leak)
If that's using the project I shared above, this could be because FlashList does view recycling. I'm not too familiar with how this all works under the hood, but one thing I've wondered is if this error happens while a re-render is happens and the context menu is still displaying.
@danielrhodes hello, i've done some more research...
i look at the code for RCTPerformanceManager
, and it looks like it just counts the views from the view registry....
after that, i started debugging again...
RCTView
instances in memoryRCTUIManager._viewRegistry
, it's saying there's 1584 items...REAUIManager
from react-native-reanimated
? (but i'm pretty sure it's not related to the issue)
RCTBridge.uiManager
, the instance it returns is REAUIManager
RCTUIManager
and REAUIManager
have the same memory address (i just thought it was interesting, but probably unrelated ashsdhkllsh)EDIT: I think i might finally fixed this in the latest prerelease version.
Interestingly, I don't think I've had this issue since upgrading to 2.4.2 and ios-utilities 4.3.1
EDIT: I think i might finally fixed this in the latest prerelease version.
Nice! When the release comes out, I can cut new release to try it.
hello @danielrhodes
for now you would have to use: react-native-ios-context-menu@v2.5.0-2
+ react-native-ios-utilities@v4.4.0-5
.
i'm still testing all of the example + test items in the example app; i just wanna be sure first that the changes i've made recently won't cause a crash somehow (i haven't found any issues so far...)
it would be great if you could try it out, and see if there are any bugs + give feedback ahsdjhkdsldfkdj
if you decide to install the pre-release version, i recommend adding the code snippet below in you App.js
/app entry file so we can get some more debugging info (you can disable/remove them anytime).
import { setSharedEnvForRNICleanableViewRegistry, setSharedEnvForRNIUtilitiesModule } from 'react-native-ios-utilities';
setSharedEnvForRNIUtilitiesModule({
debugShouldLogViewRegistryEntryRemoval: true,
overrideEnableLogStackTrace: true,
overrideShouldLogFileMetadata: true,
overrideShouldLogFilePath: true
});
setSharedEnvForRNICleanableViewRegistry({
debugShouldLogCleanup: true,
debugShouldLogRegister: true,
shouldGloballyDisableCleanup: false,
});
I am seeing these come up as unhandled errors in my exception tracker in production. I am on v 2.3 of this library and v 4.2.3 of react-native-ios-utilities. It seems to happen regardless of iOS version or device or React Native version (recently upgraded to 0.73 from 0.72).
I suspect what is happening is some sort of re-render is happening and the context menu loses its underlying reference to the root view.
which references https://github.com/dominicstop/react-native-ios-utilities/blob/master/ios/Sources/Helpers/RNIModuleHelpers.swift#L38-L50
Is there anything I can do to mitigate this?