wix / react-native-navigation

A complete native navigation solution for React Native
https://wix.github.io/react-native-navigation/
MIT License
13.04k stars 2.67k forks source link

[V2] remx HOC problem #3322

Closed yusufyildirim closed 6 years ago

yusufyildirim commented 6 years ago

Issue Description

I'm trying remx in one of my projects and have a problem with it. Whenever I wrap my component with connect() function, remx is works fine but react native navigation spesific events or functions like onNavigationButtonPressed(), componentDidAppear(), static get options() stops triggering. I looked remx's codes a bit and when we use connect, on the background remx wraps our component with HOC and returns a new component (as expected). This problem is probably remx's fault but I wanted to ask you guys here. There may be a integration problem.

Steps to Reproduce / Code Snippets / Screenshots

Environment

guyca commented 6 years ago

@Niryo @yogevbd Wasn't this fixed recently?

DanielZlotin commented 6 years ago

@yusufyildirim can you please create a very small reproducible example project? Here we have some simple integration tests with remx

Niryo commented 6 years ago

@DanielZlotin Please speak with @yogevbd, we already came across this issue and yogev even added a PR to remx.

guyca commented 6 years ago

A workaround to this issue was pushed to v1-v2 adapter, we need to address is either in Remx or in RNN before graduating the adapter.

yusufyildirim commented 6 years ago

Do you guys still need example project @DanielZlotin ?

DanielZlotin commented 6 years ago

@yusufyildirim no thanks, I pushed a fix, close the issue if resolved

yusufyildirim commented 6 years ago

@DanielZlotin I checked, still it's not working. Is this test which you added really passes? I checked my react-native-navigation package folder to be sure I get your code and yea, I have latest version of the branch.

DanielZlotin commented 6 years ago

I can't seem to reproduce this. @yusufyildirim can you create a minimal reproducible repo for me to play with?

nickofthyme commented 6 years ago

Having the same issue. @DanielZlotin I haven't had time to create a sample repo to illustrate the issue but in the meantime, here is a solution that works for me.

Modify the redux HOC to ref the WrappedComponent like so...

// @screens/withRedux.js
import React, { PureComponent, createRef } from 'react';
import { Provider } from 'react-redux';

const withRedux = (store) => (WrappedComponent) => () => class ReduxWrapper extends PureComponent {
    constructor(props) {
        super(props);

        this.componentRef = createRef();
    }

    hasMethod(method) {
        return this.componentRef &&
            this.componentRef.current &&
            this.componentRef.current[method] &&
            this.componentRef.current[method].apply;
    }

    onNavigationButtonPressed(...args) {
        if (this.hasMethod('onNavigationButtonPressed')) {
            this.componentRef.current.onNavigationButtonPressed(...args);
        }
    }

    // add other callbacks here in the same way as done with onNavigationButtonPressed above

    render () {
        return (
            <Provider store={store}>
                <WrappedComponent ref={this.componentRef} {...this.props} />
            </Provider>
        );
    }
};

export default withRedux;

Then register your screen like so...

// @screens/index.js
import { Navigation } from 'react-native-navigation';

import withRedux from '@screens/withRedux';
import MyScreen from '@screens/MyScreen';

// register screens
export const registerScreens = (store) => {
    const withReduxStore = withRedux(store);

    Navigation.registerComponent(
        'myScreen',
        withReduxStore(MyScreen),
    );
}

Then in your screen component you will have access to the callbacks (or at least the ones to added to the withRedux HOC).

// @screens/MyScreen.js
import React, { PureComponent } from 'react';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { View, Text } from 'react-native';

class MyScreen extends PureComponent {
    onNavigationButtonPressed(id) {
        if (id === 'cancel') {
            // This will now actually fire like intended (with redux)
            Navigation.dismissModal(this.props.componentId);
        }
    }

    render() {
        return (
            <View>
                <Text>This connects redux with callbacks</Text>
            </View>
        );
    }
}

const mapStateToProps = ({ auth }) => ({
    isLoggingIn: auth.isLoggingIn,
});

const mapDispatchToProps = (dispatch) => bindActionCreators({
    login: AuthActions.login,
}, dispatch);

export default connect(mapStateToProps, mapDispatchToProps)(MyScreen);

Update: After testing with a connected component the callback does not fire 😞

guyca commented 6 years ago

I believe the reported issue and the issues with connect callback are both fixed in remx 2.0.48

ferrannp commented 6 years ago

@nickofthyme does not seem to work.. I have several HOCs including withRedux. I am guessing this is a pretty common case. Is there any solution documented @guyca ?

sapjax commented 6 years ago

connect(mapStateToProps, mapDispatchToProps, null, { 'withRef': true })(MyScreen).

should add {withRef: true}

guyca commented 6 years ago

@drorbiran Can you please share your insights on integrating redux with v2?

ferrannp commented 6 years ago

At the end I went away of HOC(s) here. Basically, I have parent component will all providers you need, also the parent controls static options and lifecycle events that can be passed down.

drorbiran commented 6 years ago

It's the same issue as #1642

thame commented 6 years ago

I ended up doing the same thing as @ferrannp. I had previously been using a modified version of the HOC developed here but was unable to access the wrapped component for some reason. I'm using Mobx so I just reverted to registering my components normally and importing and observing the store into components that needed it.

sinxwal commented 5 years ago

@thame Take a look at https://github.com/alexwasner/react-native-mobx-firebase-starter repo.