Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

reduxifyNavigator is causing this.props.navigation.addListener('didFocus') to not work #53

Closed
diegoddox opened this issue Jul 20, 2018 · 26 comments

Comments

@diegoddox
Copy link

First thanks for the lib.

Description
I encounter a strange behavior while upgrading to react-navigation@2. Following the example in the docs I noticed that the navigation.AddListener('didFcous' was not working on every screen.

After many hours of debugging, I found out that by removing the reduxifyNavigator all screen that was listening for the event didFocus was working normally.

The issue
This accour when you have a screen that is listening for didFcous/some other events and is connected to redux and by dispatching an action inside of the first didFocus the next didFcous on the stack will not work.

I've used some time to create a repo that can reproduce this issue.
https://github.com/diegoddox/navigation-redux-helper

This is the file that you wanna take a look.
https://github.com/diegoddox/navigation-redux-helper/blob/master/Route.js

dependencies.
react: 16.4.1
react-native: 0.54.0 | 0.56.0
react-navigation: 2.6.2 | 2.8.0
react-navigation-redux-helpers: 2.0.2
react-redux: 5.0.7
redux: 4.0.0

@Ashoat
Copy link
Member

Ashoat commented Jul 20, 2018

Thanks for setting up the repro repo. I'll try and take a look today.

@Ashoat
Copy link
Member

Ashoat commented Jul 20, 2018

I think I have fixed this in [email protected]. Please give it a try and let me know if it works.

@diegoddox
Copy link
Author

I was able to reproduce it again with the Android back button depending on how nested navigation stack you have.

Will try to reproduce on the repo that I've created.

@diegoddox diegoddox reopened this Jul 24, 2018
@Ashoat
Copy link
Member

Ashoat commented Jul 26, 2018

Any luck @diegoddox?

@diegoddox
Copy link
Author

Hi @Ashoat sorry I haven't had the time yet

@javierbustamante
Copy link

Hi, I'm also suffering the issue...
I add listeners for willFocus, willBlur, didFocus and for didBlur but none works...
I'm working with the version 2.0.5.

xxyan0205 added a commit to xxyan0205/react-navigation-redux-helpers that referenced this issue Aug 28, 2018
@mmarvick
Copy link

mmarvick commented Oct 2, 2018

Hey @Ashoat - I'm running into the same issue and did verify that #60 fixed it for me locally. I'm on react-navigation 2.4.1 and react-navigation-redux-helpers 2.0.5. I'll see if I can put together a repo with a repro later today.

@shyaniv7
Copy link

shyaniv7 commented Oct 9, 2018

xxyan0205@b7299f5

fixes the issue for me.
Using react-navigation 2.17 and redux-helpers 2.0.6

adding the timeout around didUpdateCallback works

setTimeout(() => {
  didUpdateCallback();
}, 0);

@Ashoat can you please merge it and release a new version?

@Ashoat
Copy link
Member

Ashoat commented Oct 10, 2018

Hey guys - the fix in xxyan0205@b7299f5 is to gate the didUpdateCallback behind a setTimeout call. This seems to indicate that didUpdateCallback is firing too early.

The problem with the fix is that it is brittle. It may work on your simulators, but will it work on physical devices? On old devices? A new version of React Native or React Navigation might make it break, or force us to increase the timeout value.

The real solution is to figure out why calling didUpdateCallback immediately in componentDidUpdate doesn't work, and what needs to happen first. Then we can update the code to call didUpdateCallback exactly when it needs to.

I am willing to investigate this issue and try to figure out what the right solution is. If the setTimeout is the only thing that works, I will merge it (though I doubt that).

But first, I need either an Expo Snack or a repo that has a minimal project that reproduces the issue. This is an expectation in the React Navigation org, and in the broader React Native ecosystem. It is nigh-on impossible to debug and fix issues when you aren't able to reproduce them.

To be absolutely clear: if you want a fix for this issue to be merged, then please take the time to create either an Expo Snack or a repo that reproduces the issue. Copy-pasting code will not suffice, nor will linking me to your existing project.

@shyaniv7
Copy link

hey @Ashoat,

here is an example that I found on a different thread that should be sufficient for your investigation
https://snack.expo.io/@mrloh/tabnavigator-with-redux-didfocus

@Ashoat
Copy link
Member

Ashoat commented Oct 10, 2018

@shyaniv7, you've linked to a Snack that is using an outdated version of this package, and that is demonstrating a related but distinct issue that has long been fixed.

@shyaniv7
Copy link

shyaniv7 commented Oct 10, 2018

@Ashoat 💩 I apologize, this is the wrong link. I cannot find the example I saw a few days ago

this is another thread with the same issue described
react-navigation/react-navigation#4670

@mmarvick
Copy link

I had trouble making a repro-able case when I tried last week, for what it's worth. I tried building something from scratch using all the same packages and package versions and didn't get the buggy behavior, so there's something more subtle going on than just a bad combination of dependency versions.

I'll try going the other direction (taking my app and breaking it down to a simple repro case), but it might be a while before I get a chance -- so if somebody else has time and beats me to it, please do.

@ddthuan87
Copy link

Hi @Ashoat!

this issue has been fixed?

@Ashoat
Copy link
Member

Ashoat commented Dec 21, 2018

The original reported issue was fixed. The second reported issue still doesn’t have a repro. There is a hack in #60 that may help avoid the second issue if you’re experiencing it.

@ddthuan87
Copy link

i'm using this tut and not word @Ashoat

componentDidUpdate(){
        setTimeout(() => {
            this.props.navigation.addListener('didFocus', () => {
                this._handleDefaultLoad();
            });
        }, 0);
    }	     

@Ashoat
Copy link
Member

Ashoat commented Dec 24, 2018

I can investigate further if you can provide an MCVE that reproduces the issue, in the form of an Expo Snack or a react-native init'd repo hosted on GitHub.

@ivan-kaminskyi
Copy link

ivan-kaminskyi commented Jan 14, 2019

I am also experiencing this issue randomly. Can't find out the exact reason. On some screens it does work as expected, but on the others they don`t within the same navigator. Hope this will be fiixed soon.

@bencergazda
Copy link

bencergazda commented Feb 20, 2019

@Ashoat I am facing with the same issue. I am using Infinite Red's Ignite boilerplate (Andross), and the issue exists on a completely clean install. I created a somewhat MCVE (a clean Ignite Andross install), you can find it on https://github.com/bencergazda/react-navigation-redux-helpers-issue-53

I could not catch any event at all, but the implementation looks to be OK. Could you please have a look at it?

By default Andross uses [email protected] and [email protected], but upgrading to [email protected] and [email protected] still doesn't help.

@Ashoat
Copy link
Member

Ashoat commented Feb 20, 2019

Are you able to reproduce without Ignite? If yes, can you put up an MCVE (as specified above, either through Expo Snack or react-native init). If not, I'd guess it has something to do with the interplay with Ignite?

@Oscarz90
Copy link

has it been well tested?? Im facing the same problem u.u please help

setTimeout(() => {
this.props.navigation.addListener('didFocus', () => {
this._handleDefaultLoad();
});
}, 0);

@Ashoat Ashoat closed this as completed Apr 29, 2019
@parkhanseong
Copy link

has it been well tested?? Im facing the same problem u.u please help

setTimeout(() => {
this.props.navigation.addListener('didFocus', () => {
this._handleDefaultLoad();
});
}, 0);

mee too

@mo6moming
Copy link

@Ashoat I am facing with the same issue. I am using Infinite Red's Ignite boilerplate (Andross), and the issue exists on a completely clean install. I created a somewhat MCVE (a clean Ignite Andross install), you can find it on https://github.com/bencergazda/react-navigation-redux-helpers-issue-53

I could not catch any event at all, but the implementation looks to be OK. Could you please have a look at it?

By default Andross uses [email protected] and [email protected], but upgrading to [email protected] and [email protected] still doesn't help.

FYR: I tried below and it works :

  • ReduxNavigation.js , export Middleware instead of just createReactNavigationReduxMiddleware
export const appNavigatorMiddleware = createReactNavigationReduxMiddleware(
  'root',
  (state) => state.nav
)
  • then import to CreateStore.js and modify as below.
import {appNavigatorMiddleware} from '../Navigation/ReduxNavigation'

/* ------------- Navigation Middleware ------------ */
  // const navigationMiddleware = createReactNavigationReduxMiddleware(
  //   'root',
  //   state => state.nav
  // )
  // middleware.push(appNavigatorMiddleware)
  middleware.push(appNavigatorMiddleware)

@superp
Copy link

superp commented May 20, 2019

@Ashoat I am facing with the same issue. I am using Infinite Red's Ignite boilerplate (Andross), and the issue exists on a completely clean install. I created a somewhat MCVE (a clean Ignite Andross install), you can find it on https://github.com/bencergazda/react-navigation-redux-helpers-issue-53
I could not catch any event at all, but the implementation looks to be OK. Could you please have a look at it?
By default Andross uses [email protected] and [email protected], but upgrading to [email protected] and [email protected] still doesn't help.

FYR: I tried below and it works :

  • ReduxNavigation.js , export Middleware instead of just createReactNavigationReduxMiddleware
export const appNavigatorMiddleware = createReactNavigationReduxMiddleware(
  'root',
  (state) => state.nav
)
  • then import to CreateStore.js and modify as below.
import {appNavigatorMiddleware} from '../Navigation/ReduxNavigation'

/* ------------- Navigation Middleware ------------ */
  // const navigationMiddleware = createReactNavigationReduxMiddleware(
  //   'root',
  //   state => state.nav
  // )
  // middleware.push(appNavigatorMiddleware)
  middleware.push(appNavigatorMiddleware)

You save my life! :)
I had a problem with black screen camera after switching tab, and now it works!

@clerecudzio
Copy link

@Ashoat I am facing with the same issue. I am using Infinite Red's Ignite boilerplate (Andross), and the issue exists on a completely clean install. I created a somewhat MCVE (a clean Ignite Andross install), you can find it on https://github.com/bencergazda/react-navigation-redux-helpers-issue-53
I could not catch any event at all, but the implementation looks to be OK. Could you please have a look at it?
By default Andross uses [email protected] and [email protected], but upgrading to [email protected] and [email protected] still doesn't help.

FYR: I tried below and it works :

  • ReduxNavigation.js , export Middleware instead of just createReactNavigationReduxMiddleware
export const appNavigatorMiddleware = createReactNavigationReduxMiddleware(
  'root',
  (state) => state.nav
)
  • then import to CreateStore.js and modify as below.
import {appNavigatorMiddleware} from '../Navigation/ReduxNavigation'

/* ------------- Navigation Middleware ------------ */
  // const navigationMiddleware = createReactNavigationReduxMiddleware(
  //   'root',
  //   state => state.nav
  // )
  // middleware.push(appNavigatorMiddleware)
  middleware.push(appNavigatorMiddleware)

Superb. You saved me a thousand of minutes breaking this problem

mnzaki added a commit to jolocom/smartwallet-app that referenced this issue Jul 15, 2019
infinitered/ignite-andross#277
moaazsidat/react-native-qrcode-scanner#161
moaazsidat/react-native-qrcode-scanner#177
moaazsidat/react-native-qrcode-scanner#136
react-native-camera/react-native-camera#1797
react-native-camera/react-native-camera#1686
react-native-camera/react-native-camera#1686
react-navigation/redux-helpers#87
react-navigation/redux-helpers#60
react-navigation/redux-helpers#53
https://www.youtube.com/watch?v=CnQ8N1KacJc

Fixes #1326

The navigation actions were turned into ThunkActions and now directly
call react-navigation's navigator.dispatch, which handles state
internally

Usages of the navigationActions were fixed

Also QRcodeScanner was cleaned up a bit and hacked to properly re-enable
the camera by re-rendering it (so it gets remounted). Now camera works
again if you press back after scanning a QR code.
@flyskywhy
Copy link

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests