-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add Interaction Manager fix for iOS events not firing when transition… #590
Conversation
…ing from inactive to foreground
…ing from inactive to foreground
"When an iOS app is inactive and coming to the foreground it temporarily will drop function calls from the call stack if there is too much happening." Any docs on this behavior anywhere? Would like to know what type of things they drop |
I'm not sure but if they get triggered when the app is being foregrounded from inactive state then possibly. |
https://reactnative.dev/docs/javascript-environment This shows that iOS uses JavaScriptCore which is different than V8. It contains an API for pausing the execution of JavaScript. I didn't find any direct references for the JavaScript being paused but it seems that the Apple Documentation asks developers to free up memory when they receive warnings. That might include pausing the execution of JavaScript in JavaScriptCore. It seems that this is possible within JavaScriptCore so I would imagine it's being used. |
All of those are being subscribed during init as a backchannel to our plugin |
I do not think method calls are getting lost, pausing is not going to lose instructions. I think whats happening is its just delaying that call slightly and that is making it work for you. Any chance you could jump on a call with me and we can remote debug the current version to see whats going on? I don't want to add this to the plugin and find out it only fixes it for you part of the time |
I'm pretty sure it does because If it's not adding instructions to the JavaScript heap to conserve memory then it would stop execution. We even printed the logs and the listener never receives an event unless we add this code. Probably because even though the Native Layer is pushing events. That JavaScript is not ready to receive them because it's not adding function calls to the stack because it's paused. |
@syntacticsolutions Are any events firing or just not push received/response? What if you added the listener before calling takeOff? I just want to make sure its not some other race condition happening to avoid possibility of this not fixing it in all scenarios |
I think we figured out a better way to do it. If we add the listeners before calling takeoff it breaks Airship without any errors. Airship just stops receiving events on the RN layer completely. We're testing the changes right now |
The listener being added should not have anything to do with airship being ready or not. You should be able to add the listeners anytime in the app. So far I have been unable to reproduce with new and old architecture (but did find a compile issue with new, ill post a PR). Ill provide the home screen in the support ticket to avoid sharing any code you provided. I am seeing a possible race condition though with |
It might fix the addListener call before init but I don't think it will fix the issue of init being called when the app is in inactive state. |
@rlepinski I figured out a way to use react-native's EDIT: |
The first time you call takeOff it will cache the config and apply it as soon as the app is initialized on the next run. If you try to change the config, it wont be applied til next run. So it really doesn't matter where you are calling takeOff and it should have no effect on subscribing to event listening. We also store any events until we have a listener to accept them, so where you are registering for the event listener also does not matter, except for I would suggest you call takeOff in the root of your App.tsx file outside of a useEffect and assume its ready everywhere else in your app. That was the intended use so you do not have to worry about the state of Airship. For the example that we include, I usually just call it around here - https://github.com/urbanairship/react-native-airship/blob/main/example/src/App.tsx#L16 |
src/UAEventEmitter.ts
Outdated
|
||
if (Platform.OS === 'ios' && AppState.currentState !== 'active') { | ||
AppState.addEventListener('change', (nextAppState) => { | ||
if (nextAppState === 'active' && !this.iosListenerInitialized) { |
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 will break content-available pushes (wake app up in the BG)
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.
You're right. I will fix that.
@syntacticsolutions if you could give #591 a try to see if that resolves the issue? |
We tried that already but we have a lot of things happening when the app is initialized from closed state which causes it to go into inactive mode and when it's in inactive mode (According to the documentation). iOS does not receive events and it will just drop them. The example app would work in a small app but we have a very large app with many things happening at launch. |
That is not the correct documentation to be pulling from. That is saying not receiving user input events. This happens if for instance you pull down the notification shade while the app is up. It has nothing to do with being able to process events from JS. |
According to other parts of the documentation there are other events that don't get processed as well. The documentation says to do minimal work while the app is in the inactive state. Our thought is that it could be a combination of both things because even when the events were fired and we tried to set state it would not pass the state to the state setter. |
AppState uses a NativeEventEmitter as well, so its effectively replacing the call you believe is being dropped with an identical call that should be dropped if your theory is correct. I dont think this is the root cause. |
After looking at the logs your provided, I think it might be an issue with a conflict with another plugin, possibly https://github.com/invertase/react-native-firebase I see this from our SDK logs that you provided:
And this from another log you provided:
On iOS, all push events come to either the app delegate or a notification center delegate. Airship, Firebase, and a few others use method swizzling to intercept the methods. Usually we do swizzling up front, but on the first launch its when takeOff is called, so the first launch order will be different then subsequent launches. The name changes, indicating we are getting different swizzle orders First could you add this to the root of your app (outside of useEffect) so we can see if events are working at all?
Could you then provide verbose logs showing the event not working (not just first render, send push and tap it), and again when it is working? |
@rlepinski I updated the code to match the final code changes from our project. |
@syntacticsolutions So the root issue is Airship not being compatible with an app that has multiple bridge being setup. Each bridge will load its own copy of the plugins, but the Airship module assumes a single bridge and sets the event listener on a singleton when things are being observed from the JS side:
This basically makes it the Airship plugin only use the last bridge that was set on. Not only that, when you refresh the context, the bridge order changes. I set up two bridges, and when I first loaded Airship would set the listener for the second bridge (the one I believe you want). After a refresh, the order swapped, and Airship will only notify events to the first bridge. Refreshing further still resulted in the same bridge order. I believe on your first run you have a context refresh that is happening causing the bridge order to be swapped. The second bridge is coming from the creation of this view The changes you have now work because you delay when the listener is being added on the Airship side to only the bridge that is calling takeOff, so I am going to look into changes the way our plugin notifies events to be multi bridge compatible if that becomes a real use case or the norm. |
…ing from inactive to foreground
What do these changes do?
When an iOS app is inactive and coming to the foreground it temporarily will drop function calls from the call stack if there is too much happening. In some instances many things are being initialized and the call to
dispatchEvents
gets dropped because of iOS performance mechanisms. This code change ensures that the events are always dispatched and handled because the code only runs when the JavaScript is ready.Why are these changes necessary?
The changes are necessary here because it seems like the simplest way to fix the issue.
How did you verify these changes?
We tested it in a react-native project.
Verification Screenshots:
Anything else a reviewer should know?