Skip to content
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

regression on InteractionManager.runAfterInteractions(f), not always calling f #8624

Closed
gre opened this issue Jul 7, 2016 · 47 comments
Closed
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@gre
Copy link
Contributor

gre commented Jul 7, 2016

Since RN 0.28, InteractionManager.runAfterInteractions(f) happen to sometimes never call f.
I couldn't reproduce it with some simple code, but I have an app that schedule stuff in background when leaving a screen, and use InteractionManager.runAfterInteractions when re-entering the screen (which happen during the navigator transition) and sometimes f is not called.

Should we assume that runAfterInteractions can sometimes not happen, since it's a cancellable now? (but i'm not using cancel() anywhere in my code)
or is this a bug?

Thanks

@gre
Copy link
Contributor Author

gre commented Jul 7, 2016

quick workaround if anyone interested.

import { InteractionManager } from "react-native";
export default {
  ...InteractionManager,
  runAfterInteractions:  f => {
    // ensure f get called, timeout at 500ms
    // @gre workaround https://github.com/facebook/react-native/issues/8624
    let called = false;
    const timeout = setTimeout(() => { called = true; f() }, 500);
    InteractionManager.runAfterInteractions(() => {
      if (called) return;
      clearTimeout(timeout);
      f();
    });
  }
};

gre referenced this issue Jul 20, 2016
Summary:
Returns a promise-like object with a new cancel function that will dig through the queue
and remove relevant tasks before they are executed. Handy when tasks are scheduled in react
components but should be cleaned up in unmount.

Reviewed By: devknoll

Differential Revision: D3406953

fbshipit-source-id: edf1157d831d5d6b63f13ee64cfd1c46843e79fa
@satya164
Copy link
Contributor

satya164 commented Aug 6, 2016

I'm also facing this issue in a project. cc @sahrens

@satya164
Copy link
Contributor

satya164 commented Aug 6, 2016

Hmm... it seems different issue where my render method is not called until a touch the screens. Super weird.

@chirag04
Copy link
Contributor

chirag04 commented Aug 6, 2016

@satya164 I may have seen something similar and using react-native-tab-view. my page won't render if i use scrollview but would work fine without a scrollview! yet to debug.

@satya164
Copy link
Contributor

satya164 commented Aug 6, 2016

@chirag04 I think I've seen this too. If you have a minimal example give me a link and I'll try to debug it. otherwise I'll try setting up one. It's super weird though :/

@satya164
Copy link
Contributor

satya164 commented Aug 6, 2016

Ok, so seems InteractionManager not calling the callback also happens for me. I added a breakpoint and found that _interactionSet.size is always >= 1, so something in not clearing the interaction handle. I don't use createInteractionHandle in my code, and I don't think any packages I use, use it either.

Edit: Seems if the panHandler gets removed before releasing the touch, it never clears the handle

satya164 added a commit to satya164/PocketGear that referenced this issue Aug 7, 2016
@freakinruben
Copy link

@satya164 I had something similar when I upgraded to RN 0.29 with a scrollview on a tab that wasn't primary. The scrollview content wouldn't show until a touch-event happened. But if the tab with the scrollview was active when the app starts it would work just fine.

I fixed it by disabling clipping on my scrollview.. Not the best solution but it works temporarly. You could also set removeClippedSubviews to true after the layout has been measured or something

@carbureted
Copy link

Getting bit badly by this.

@fmnxl
Copy link

fmnxl commented Nov 17, 2016

In my case it was a looping animation (i.e. with the Animated API) that prevented f from being called. For now I'm going to replace the animation with a static element. I wonder if it's possible to make runAfterInteractions work with an infinitely looping animation elsewhere.

@aforty
Copy link
Contributor

aforty commented Dec 6, 2016

This is a major issue for me as well and not consistently reproducible either. I hope someone tracks this down. Every time I think I think it's fixed it rears its ugly head again and causes my components not to load after navigation.

@jqn
Copy link

jqn commented Feb 7, 2017

Has anyone figured out a fix for this issue. I randomly started experiencing this with RN 0.40 Android. It used to work great before.

@Exilz
Copy link

Exilz commented Feb 8, 2017

Same here, we rely extensively on this callback, and in some unidentified cases, the callback isn't fired (RN 0.39).

@freakinruben
Copy link

@jqn, no fix but I've been using the workaround described above without issues

@mienaikoe
Copy link

mienaikoe commented Feb 9, 2017

Still having this problem with 0.41
I've witnessed what @freemanon mentioned - if you have a cyclical animation, the callback will never be called. It would be nice to have something just for Navigator transitions, since that's 95% of what people are using InteractionManager for.

@scarlac
Copy link
Contributor

scarlac commented Feb 21, 2017

@Exilz @jqn @aforty I had this exact issue. Solved it: The callback for InteractionManager.runAfterInteractions was never being called. Turns out that when components are unmounted, their animations are not stopped completely. I had a Animated.decay animation which seems to make InteractionManager never return.
Calling this.myDecayAnimation.stopAnimation() in componentWillUnmount() did not work for me (more on this below).

For me, a couple of things worked:

  1. Use isInteraction: false. E.g. Animated.decay(this.myDecayAnimation, {velocity: 1/5, isInteraction: false}); This is an undocumented feature that tells RN that your animation will not block InteractionManager. This solved it for me, but I kept looking.
  2. Removing the callback for Animated.Value().start(...) where I was conditionally repeating the animation.

Regarding 2: The callback was checking this.state.shouldAnimate === true. If true, it re-ran the animation. The problem was that the callback was being run after the component unmounted, but without access to the correct state, resulting in the animation being run a second time, thus making my stopAnimation useless.

You may have similar looping animations or stopAnimation code that isn't really stopping your animation, or you could be assuming that your animation stops when the component is unmounted, regardless of your loops. Re-check all those assumptions and see where it gets you.

@Exilz
Copy link

Exilz commented Feb 21, 2017

Hey @scarlac, thanks for your thorough explanation. I found out the exact same things, and isInteraction: false solved it for me too.

This really should be documented, I guess a lot of people once struggled with infinite loading and never suspected a looping or unmounted animated component to be the source of their problems... This is a huge "gotcha" 😛

@aforty
Copy link
Contributor

aforty commented Feb 21, 2017

That makes sense @scarlac and I agree that it should be documented.

However, I've encountered the issue with simply waiting for InteractionManager.runAfterInteractions within componentDidMount of a new component being animated "in" by Navigation. I have seen it happen in extremely simple use cases but usually randomly and without rhyme or reason.

@gre
Copy link
Contributor Author

gre commented Feb 21, 2017

and it's still a breaking changes that runAfterInteractions no longer guarantees that your callback will be called. Having to set a flag on the animation side (and not on the runAfterInteractions) is not a viable fix. (imagine I use a library that underneath would do some animations)

@satya164 satya164 reopened this Feb 21, 2017
@satya164
Copy link
Contributor

It was always like that. Don't think it's a breaking change if it does what's it supposed to do, defer callbacks until no animations or touch is active.

It' library's responsibility to mark an animation as not interaction of it shouldn't block interaction manager. How'd you specify it on interaction manage's side? How'd it even know what's an interaction or not?

I guess it could warn if an animation is running for a long time and it's not able to run the callback.

@gre
Copy link
Contributor Author

gre commented Feb 22, 2017

It's getting historical now, but I'm pretty sure it was not like that before 0.28 (because I start seeing the bug when migrating to that version, using same code base). maybe it was designed to be like that, but before 0.28 I'm pretty sure the bug was not existing (see my initial message).

so I expected runAfterInteractions to execute the callback exactly once but in fact it's 0 or 1 times, randomly depending on user interactions. If the contract runAfterInteractions was from the start, intending to not guarantee callback is called, i'm ok with that, but please we need to make sure the documentation write it down, it's an important disclaimer of using that function.
Sometimes, it's useful to have guarantee that a code is executed, imagine you want to schedule something to show up after entering a screen, if using runAfterInteractions, you randomly won't have it happen. So maybe we need another function for that.

basically it means runAfterInteractions would not be a good candidate to be a "scheduler" function (e.g. that you would give to Relay) in opposite to requestAnimationFrame or setTimeout.

@satya164
Copy link
Contributor

https://facebook.github.io/react-native/docs/interactionmanager.html#runafterinteractions says that the callback will only be called after all interactions have been completed, and afaik it was the same from start. Of course if the animation never completes it'll never be called. The contract is to wait for animations to finish.

randomly depending on user interactions

I don't think depending on user interactions is random behaviour. The method name is run after interactions, so it's gonna wait for interactions to complete. And that's what the docs say too.

imagine you want to schedule something to show up after entering a screen

If you don't need to wait for animations to complete, why use runAfterInteractions? just use a timeout

If you faced this in your code after upgrading only react native and nothing else, probably be it's just a bug and has nothing to do with interactions.

@gre
Copy link
Contributor Author

gre commented Feb 22, 2017

ok, maybe I should just use setTimeout then ;-)
runAfterInteractions is only for fire & forget work that you don't care it's potentially not called I guess.

Yeah, the method name is run after interactions but it doesn't say it's not guaranteed. Even if it returns a Cancellable, as the caller of runAfterInteractions, you don't own that cancellable, maybe it's cancelled by something else. IMO it's error prone.

@satya164
Copy link
Contributor

maybe it's cancelled by something else. IMO it's error prone.

If it is, then it's a bug if I understand correctly

@sahrens
Copy link
Contributor

sahrens commented Feb 22, 2017

@gre: no, I'm not sure where you got that. All tasks scheduled with runAfterInterActions should be executed in order after interactions complete. If the interactions never complete (e.g. infinite animation) then those tasks (and any subsequently scheduled tasks) will ever get run. If you ever schedule tasks A then B with runInteractionManager and you don't see A run before B but B does run, there is definitely a bug (assuming you didn't cancel A).

Separately, animations should be stopped when the component being animated unmounts, which should allow runAfterInteractions to continue if those animations were blocking it. If that's not happening, that's also a bug, although the previous reports indicate that the problems were in user code erroneously creating an infinite loop.

@sohobloo
Copy link
Contributor

sohobloo commented Feb 27, 2017

Here is my condition:

  • A ListView with several cells.
  • A panGesture for each cell. onStartShouldSetPanResponder returns true.
  • **Call setState() on onPanResponderGrant. **
  • From now on, InteractionManager.runAfterInteractions will never execute the callback.

--- Update ---
Add some logs in InteractionManager.js I can see create without clear. _interactionSet.size increases continually.

--- Update Again ---
After read the source code of PanResponder, I find a way to fix this condition:

let panResponde = PanResponder.create({
      onStartShouldSetPanResponder: () => {
        return true;
      },
      onPanResponderGrant: (e, gestureState) => {
        // This is the KEY POINT LINE!
        this._panResponder = panResponder; 
        this.setState({pressIn: true});
      },
     onPanResponderRelease: () => {
        // Manually clear the interaction handle.
        InteractionManager.clearInteractionHandle(this._panResponder.getInteractionHandle());
      },
     onPanResponderTerminate: () => {
        // Manually clear the interaction handle.
        InteractionManager.clearInteractionHandle(this._panResponder.getInteractionHandle());
      }
});
```javascript

@sahrens
Copy link
Contributor

sahrens commented Feb 27, 2017

Hmm, not sure why that helps - onResponderTerminate should call clearInteractionHandle before calling onPanResponderTerminate. I think something else must be going on, but glad to hear you found a workaround.

@satya164
Copy link
Contributor

What I've seen is if the component unmounts when touch is active, it never clears the handle.

@chirag04
Copy link
Contributor

I just got hit by this. We inject taskScheduler for our relay environment and in some cases the runAfterInteractions callback will never be fired and the screen would never be able to fetch the data (Only happens on android. works fine on ios).
environment.injectTaskScheduler(InteractionManager.runAfterInteractions);

I don't have an isolated repro yet. Will debug this now.

@chirag04
Copy link
Contributor

chirag04 commented Mar 15, 2017

Ok figured it out. So in our case, react-native-drawer-layout moved to a JS based implementation even on android without bumping the major version! Due to whatever reason, the interaction handle is not cleared after opening and closing the drawer menu.

rnc-archive/react-native-drawer-layout@20212c7#commitcomment-21337227

@woowalker
Copy link

Still having this problem with 0.40, and it occur frequently, i have to remove all of it .

@lyxia
Copy link

lyxia commented Jul 4, 2017

Still having this problem with 0.45, and it occur frequently when use react-navigation

@davojan
Copy link

davojan commented Aug 4, 2017

0.46 is also affected.

@mjylfz
Copy link

mjylfz commented Aug 10, 2017

how to solve the problem?User ‘isInteraction: false’can only solve the problem when you code the animation by your own.How about the animation when you navigate or you click the TouchableOpacity?
It also doesnot work Sometimes.

@gre
Copy link
Contributor Author

gre commented Aug 12, 2017

@mjylfz you may use the workaround shared above (second comment on this issue)

@stale
Copy link

stale bot commented Oct 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 13, 2017
@stale stale bot closed this as completed Oct 20, 2017
@sujameslin
Copy link

I'm on RN 0.50.3, i'm still seeing this issue.
Why its closed?

@sibelius
Copy link

this is still happening on RN51

@natakina
Copy link

Te same issue. I have no animationds, I tried to delete almost everything from render function, but callback from InteractionManager.runAfterInteractions still doesn't run....Any ideas?

@satya164
Copy link
Contributor

Please post a repro if you face the issue. Replying with "this happens to me" doesn't help and sends notifications to everyone.

@cihadturhan
Copy link

I was planning to use runAfterInteractions but I found out the callback is never called.
I can't even trust setTimeout because it doesn't guarantee the animation finishes. And also, I thought using it will remove another bug happens on ios which is screen freezes when you call alert when another animation happen.

Unfortunately, this is a blocking issue for me. Nevertheless, I will give a try to setTimeout but I believe I'll have some issue reports regarding the bug above I mentioned.

@sibelius
Copy link

this comment #8624 (comment) can cause some troubles as well

@chrfalch
Copy link

chrfalch commented May 11, 2018

Could this be related to react-navigation/react-navigation#4144?

A solution (implemented in React Navigation) is to save the panResponder and call InteractionManager.clearInteractionHandle(myPanResponder.getInteractionHandle()) before recreating the responder.

The problem seems to be that when creating new panResponders in the render method the wrong release-method is called.

@facebook facebook locked and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests