Refresh not working as expected in several cases

[daviscabral]

Some small tweaks in the refresh method and a few new callbacks to allow some work around navigation events in order to get it updated.

Components are mounted at once - from what I could see - still checking it.

This PR will solve #3236, #3218, #3206, #3197, #3188.

[aksonov]

LGTM (for WIP)

[aksonov]

@daviscabral Is it ready to merge?

[aksonov]

I think no, could you tell me what issues/difficulties are?

[daviscabral]

Just lack of time - helping a friend with some stress test in his server right now - after finishing it I'll push the changes.

But what I am doing is to move the logic of onEnter from componentDidMount to the event listener willFocus. And the onExit to the didBlur. But I want to be sure that is really going to fix the issue - and also not break anything.

I am also extending the test suite - so it's possible to require that every new PR that fixes a bug comes in companion of a test that helps it to come back (and also in future upgrades).

[dextermb]

😲cant wait to see everything functional again, @daviscabral @aksonov

[aksonov]

@daviscabral But how it will solve problems with passing params?

[daviscabral]

@aksonov yes - that's going to fix that's as well. sorry - misunderstood the question. Some of the errors I saw in the issues opened were related with people relying in the regular life-cycle from the components, that now are not like before.

It's not actually directly attached - I am going to finish the changes and show to you here, not sure if this should exactly be merged, as it's probably not just extending/facilitating the use, but might be changing behavior.

[daviscabral]

@aksonov after this - could we get a new version? I tried the last time but due the lack of permissions I was not able to publish - so I reverted a few things. Ended doing a little mess here while you was not around.

[aksonov]

@daviscabral Sure, I'll publish new version once this PR is finished. Any ETA?

[daviscabral]

Should be there in a few hours - just finishing a few tests here.

[dextermb]

@daviscabral you've managed to resolve the param issue?

[jaredkove]

Can't wait to get this update! Thanks @daviscabral

[daviscabral]

Please, do not merge yet - I noticed a bug in the isStatelessComponent call - making it to pass a ref to the wrong kind of component.

I've updated the examples to some of the cases that people complained - but to hide or pass props - it's like @aksonov said before - wrap=false might be needed - or you will have to provide the right scene key to refresh or pop.

Below the tab bar toggle working:

[daviscabral]

I would appreciate some help testing these changes by the people that were having the problems before we proceed. We might have to update docs too. Any voluntary?

[dextermb]

@daviscabral I will test it now and see if it works in my testcase

[dextermb]

@daviscabral Attempted to see if it worked with the example I posted a while ago (challenge). But it didn't seem to get passed down 🤕

[dextermb]

@daviscabral I've created a gist with the example I gave, along with the routes structure.

[daviscabral]

Can you try this? (just to test if I am correct about the key reference that we need to keep now. If thta's the case - like it happened with React Navigation - it will require some work from everyone to adapt to it.

onPress={() => Actions.pop(
        {
          ...props,
          refresh: { challenge: challenges[i] },
          key: 'root'
        }
      )}

I believe you are defining the params but they are not passed from top to bottom like before. Also, check this:

onPress={() => {
   Actions.ChallengesDetails();
   Actions.refresh(
        {
          ...props,
          refresh: { challenge: challenges[i] },
          key: 'root'
        }
      )}

[dextermb]

@daviscabral Can do, but after installing this branch then swapping back to master my app doesn't load the initial scene anymore. Trying to debug that 😨

[dextermb]

I've been trying to debug it myself from reading through code, but wouldn't the issues be happening with:

      if (!this[key]) {
        this[key] = new Function(
          'actions',
          'props',
          'type',
          `return function ${
            key.replace(/\W/g, '_') // eslint-disable-line no-new-func
          }(params){ actions.execute(type, '${key}', props, params)}`,
        )(this, { error: '', ...commonProps, ...props }, type);
      }

---

Source

[daviscabral]

I use this to help with the libs I help to maintain:

function rn-clean() {
  rm -rf android/build
  rm -rf ios/build
  watchman watch-del-all
  rm -rf $TMPDIR/react-*
  rm -rf $TMPDIR/haste-*
  rm -rf $TMPDIR/metro-*
  yarn start --reset-cache
}

$ cp [lib-path:react-native-router-flux(in this case)]/src/*.js [app-path]/node_modules/react-native-router-flux/src/
$ yarn install # in the project root 
$ rn-clean

For your case - probably worth also remove node_modules.

[daviscabral]

This is the piece that magically generates the methods with scene names. But it calls execute - with type jump for tabs - or push for everything else.

[dextermb]

@daviscabral Alright, ive cloned down my app again. npm i then going to build and test with Expo

[dextermb]

@daviscabral Isn't that magic function the point where it degregates props into the other functions then down to react-navigation?

[daviscabral]

You can place a console.log inside the execute - and you will see that the params get there. The problem seems to be that is not possible to put a lot of nested scenes like before without wrapping it with a navigator to manage this. This is what I am trying to confirm.

[dextermb]

@daviscabral 😛 im trying to confirm it for you but my app doesn't want to play nice after changing versions

[dextermb]

Was unable to get the app working again after trying different things, so i'm transplanting things into an application that uses React Navigation for now

[daviscabral]

I am getting this in - I can't reproduce that - but it fixed a few other cases and add more examples.