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

Will it be exposed ReactReduxContext from react-redux? #935

Closed
budarin opened this issue Apr 23, 2018 · 37 comments
Closed

Will it be exposed ReactReduxContext from react-redux? #935

budarin opened this issue Apr 23, 2018 · 37 comments
Milestone

Comments

@budarin
Copy link

budarin commented Apr 23, 2018

will there be a new syntax of context available for using in my HOCs or should I using connect for accesing store and reinventing another one context?

@markerikson
Copy link
Contributor

You'll still be using connect() as normal - any changes to use the new context API will be an internal implementation detail. See #898 for an idea of what it may look like.

@budarin
Copy link
Author

budarin commented Apr 24, 2018

@markerikson
I need to have access to store\s methods and properties not data
connect provide me with state but I need its properties & methods

it would be safe to share ReactReduxContext.Consumer for such demands

@markerikson
Copy link
Contributor

What's your use case? Why do you need access to the store itself elsewhere in the component tree?

@budarin
Copy link
Author

budarin commented Apr 24, 2018

@markerikson
our bank app had a huge state so we have to dynamic add/remove reducers and sagas for app sections
at now we are using HOC with store context for these purposes
without access to the store object it's impossible

@budarin
Copy link
Author

budarin commented Apr 25, 2018

@markerikson
any suggestion?

we really need access to store.replaceReducer
should we reinvent new store context to have access to?

@timdorr
Copy link
Member

timdorr commented Apr 26, 2018

@budarin No changes have been made or will be made in the near future. We have no idea how things will actually shake out.

@budarin
Copy link
Author

budarin commented Apr 26, 2018

so we have to make an additional context that will duplicate the existing redux internal context - it's overhead

@budarin
Copy link
Author

budarin commented Apr 26, 2018

@timdorr
@markerikson

at now we have this ugly code

return (
        <Provider store={store}>
            <StoreContextProvider store={store}>
                   .....
            </StoreContextProvider>
        </Provider>
    );

@timdorr
Copy link
Member

timdorr commented Apr 26, 2018

The store is made available to your connected components. You don't need to add your own context.

@budarin
Copy link
Author

budarin commented Apr 26, 2018

@timdorr
in new versions of React with new Context API connected components will be able to get state of a store and dispatch method only, but we need an access to the the store to have ability to execute

store.replaceReducer(...) 

@jwugit
Copy link

jwugit commented Oct 25, 2018

Hi, im having similar issue trying to make redux code split reducers.

@budarin
did you ever found a solution to this issue ?

@timdorr @markerikson
Now that the new react context api is updated,
is there any consideration to support exposure to the store from connect or at least exposure to let use or call redux store replaceReducer ?

@markerikson
Copy link
Contributor

Our current WIP v6 PRs at #995 and #1000 both actually put both the store and the store state into context. I don't think we're going to officially support accessing it, but I don't want to intentionally block people from accessing it either.

Beyond that, please try out the PRs and give us feedback on how well they work (or don't work)!

@budarin
Copy link
Author

budarin commented Oct 25, 2018

@jwugit I've found on the internet a snippet from Dan Abramov but I lost the link
try to find it

@jwugit
Copy link

jwugit commented Oct 25, 2018

@markerikson
Ok thanks, i will give them a try tomorrow and see if it works out.

@budarin
I did found and looked up a lot of snippets,
This is not actually an issue if there is only one store in the app.
It can be imported from the store.js file again, since is a singleton, in any component to access the replaceReducer function.

But my specific uses case is having multiple store and i cannot use the singleton approach since i wouldnt be able to target the correct store instance.

@markerikson @budarin
if you want to know about my use case, i wrote up a question here.
https://stackoverflow.com/questions/52957552/how-to-replace-old-react-contexttypes-with-new-react-context-api-when-using-dyna

Initially i thought this would be a react problem since it is changing the legacy context to new context api. But Dan mention it's more of a react-redux decision of letting context be exposed or not in the upcoming updates. So here i am 😃 .
https://twitter.com/dan_abramov/status/1055445608104890368

@markerikson
Copy link
Contributor

@jwugit : can you give some specific examples of how and why you have multiple stores in your app, and what the component tree looks like (ie, which components are needing to pull data from which stores)?

@jwugit
Copy link

jwugit commented Oct 25, 2018

@markerikson
i have an app, Let say it has 2 pages,
1 with a very complicated todo list and 1 a calculator.
Normally it renders on 1 root element. But to re-use component and reducer logics,
I can render on it multiple roots. Let say for example 5 different todo list on the same web page.

Now i dont want the store data to be sync, other wise updating one will update all the other.
They need to be unqiue todo list.
I found 2 ways to do this,

  1. have 1 store by dynamically name each reducer differently each for each app.
    2, create a new store for each root.

For option 1, is easy with only 2 component pages, but what if i have 20+ pages. Then there will be too many reducers times the number of roots i have.

So we opt to try option 2. its cleaner to see in the redux dev tool also, and we name each generated one.

So basically each root level app needs to pull data from its own store.
But this is all generated from the same app codes.

`
const rootElements = document.querySelectorAll("[data-roots-app]");
rootElements.forEach(function(rootElement, index) {

// create unique store for each embed
const store = makeStore('app-' + index);

// extract defined page types here
const rootElementTarget = rootElement.getAttribute('data-page-target') || '';

// update name to the store
store.dispatch(defaultChangePageTarget(rootElementTarget));

// render the embed
ReactDOM.render(
    <ReduxProvider store={store}>
        <Router>
            <App />
        </Router>
    </ReduxProvider>
    , rootElement);

});
`

@jwugit
Copy link

jwugit commented Oct 25, 2018

i realize that with the updated react context api,
code splitting or lazy loading reducer wont work even if react-redux expose the store cotext.
Because it seems, i need to update the reducers before the connect mapStateToProps.
If connect runs before i trigger inject reducers, the states dont exist yet!
I'm feeling like this is a react context issue after all.

My flow:
-lazy load component
-when called to be used, inject reducer runs then connect component

app container

import FruitPage from "../containers/FruitPage/loadable";
<Route path="/fruit" component={FruitPage} />

loadable

export default Loadable({
    loader: () => import('./fruit'),
    loading: LoadingIndicator
});

fruit component

const mapStateToProps = (state) => {
    const reducer = state[reducerName];
    return {
        counter: reducer.counter
    };
};

export default withReducer(reducerName, fruitReducer)(connect(
    mapStateToProps,
    { fruitAddCount, fruitRemoveCount } 
)(Fruit));

withReducer

export const withReducer = (key, reducer) => WrappedComponent => {
    const Extended = (props, context) => {
        context.store.injectReducer(key, reducer);
        return <WrappedComponent {...props} />;
    };

    Extended.contextTypes = { store: PropTypes.object };

    return Extended;
};

Based on this legacy context example,
I just couldnt update it to work with new context api.

@budarin
Copy link
Author

budarin commented Oct 25, 2018

It's true that in a big enterprise app store should have an ability for dynamic injecting/rejecting some reducers djue to a large amount info stored in it.
But I think that some stores could not help and will not make it simple for development to support them

@jwugit
Copy link

jwugit commented Oct 26, 2018

@markerikson
any thoughts on my use case ?
or any consideration to add store.injectReducer access in the new version ?

@markerikson
Copy link
Contributor

@jwugit : we really haven't gotten that far. Give us some time to figure out how the rest of React-Redux v6 will behave, and hopefully we can address your concerns once the rest of the implementation is sorted out.

@markerikson markerikson reopened this Oct 26, 2018
@markerikson markerikson added this to the 6.0 milestone Oct 26, 2018
@timdorr
Copy link
Member

timdorr commented Nov 7, 2018

Yes:

export { Provider, connectAdvanced, ReactReduxContext, connect }

@timdorr timdorr closed this as completed Nov 7, 2018
@jwugit
Copy link

jwugit commented Nov 9, 2018

tested the beta and its working great!

@GuillaumeCisco
Copy link

@jwugit I have the same kind of problem. Not using Loadable, but react-universal-component, almost same philosophy. How did you succeed to make your connected component aware of the last update state with the injected reducer?

Currently, my only dirty patch is to call:

const st = ReactReduxContext._currentValue.store.getState();

In the mapStateToProps function for getting the very last updated state and have my app running correctly. As the state passed from mapStateToProps as the first argument is not updated.
Is there a way to get the very last updated state with the injectedReducer?

Thank you,

@markerikson
Copy link
Contributor

markerikson commented Dec 10, 2018

@GuillaumeCisco : that sounds very wrong. What are you actually trying to accomplish? Why are you needing to try to hack into the context like that? What does the rest of your code look like?

@jwugit
Copy link

jwugit commented Dec 10, 2018

@GuillaumeCisco i'm not sure if youre referring to the same issue i mentioned on this ticket.

but when i run replaceReducer to update the store, everything is recreated in the store,
i take the old state and pass it along.

this is the create store function in my proof of concept app.
Maybe it can help u. There should be multiple examples of this out there. I referenced most of it from elsewhere. dont recall where right now.

/** Store Creator **/
export const makeStore = (name) => {
    const store = createStore(
        reducers,
        initialState,
        enhancer(name)
    );

    // Inject the initial state for async loaded reducers
    function injectState(reducers, preloadedState = {}) {
        return Object.keys(reducers).reduce((finalReducers, key) => {
            if (typeof reducers[key] === "function") {
                finalReducers[key] = (state = preloadedState[key], action) => reducers[key](state, action);
            }
            return finalReducers;
        }, {});
    }
    store.asyncReducers = {};
    store.injectReducer = (key, reducer) => {

        store.asyncReducers[key] = reducer;
        console.log('hit');

        const combineReducersObj = combineReducers(injectState({
            ...store.asyncReducers,
            ...initialReducers
        }, initialState));

        store.replaceReducer(combineReducersObj);

        console.log(store.getState());
    };
    return store;
};

@GuillaumeCisco
Copy link

Thank you @jwugit . I'm not sure too now ^^" But there is a lot of similitude.

I use react-universal-component and redux-reducers-injector (I'm the maintainer).

Basically redux-reducers-injector does what you define in your store.injectReducer method, but expose an API with additional methods too. Code is very easy to read: https://github.com/GuillaumeCisco/redux-reducers-injector/blob/master/src/ReduxInjector.js.

react-universal-component has the same philosophy as react-loadable, it's a lazy loader component tool.

So basically, if I want to inject a Component in my app and inject its reducer too, I simply write:

import React from 'react';
import universal from 'react-universal-component';
import {injectReducer} from 'redux-reducers-injector';

const UniversalComponent = universal(import('./my_component.js'), {
    onLoad: (module, info, props, context) => {
        injectReducer('loadingReducer', module.loadingReducer); // will call replaceReducer
    }
});

export default UniversalComponent;

The file my_component.js looks like:

import React, {Component} from 'react';
import {connect} from 'react-redux';

// make loadingReducer available for being injected
export loadingReducer from './loadingReducer';

class MyComponent extends Component {
    render () {
        const {loading} = this.props;

       return loading ? <span>loading</span> : <div>my heavy content</div>;
    }
}

const mapStateToProps = (state, ownProps) => ({
    loading: state.loadingReducer.loading,
});

export default connect(mapStateToProps)(MyComponent);

This works perfectly with react-redux <= 5.1.1
But with react-redux >= 6.0.0, I got an error on the line loading: state.loadingReducer.loading, loadingReducer has not been injected in the state returned by the mapStateToProps method.
Dirty workaround to see what's going on is to call ReactReduxContext._currentValue.store.getState() for knowing the real actual state in the mapStateToProps method. And indeed, the loadingReducer has correctly been injected and is available.

As I read from react-redux 6.0.0 Behavior changes here:

Any library that attempts to access the store instance out of legacy context will break, because we now put the store state into a <Context.Provider> instead. Examples of this include connected-react-router and react-redux-subspace. (The current implementation does also put the store itself into that same context. While accessing the store in context is not part of our public API, we will still try to make it possible for other libraries to access it, with the understanding that this could break at any time.)

Si I don't know really how to be able to pass the last state to my component with its injected reducer available. Is there some documentation about that?

Furthermore, As I expected, the fourth parameter context in the onLoad: (module, info, props, context) from react-universal-component does not expose the store with react-redux >= 6.0.0. But this is not an issue for me right now.

Thank you,

@jwugit
Copy link

jwugit commented Dec 11, 2018

@GuillaumeCisco I'm not familiar with react-universal-component
but your injectReducer is called in the onLoad function.

Does this mean the inject happens after loading ?
If yes, then your approach is wrong.
Loading a component that depend on a reducer must have the reducer existing already.
So you need to inject, then lazy load.

@GuillaumeCisco
Copy link

Thank you @jwugit, It confirms what I understand about it.

But it seems according to the documentation of react-universal-component onLoad function, the injected reducer is called before loading the component.

See: https://github.com/faceyspacey/react-universal-component

onLoad is a callback function that receives the entire module. It allows you to export and put to use things other than your default component export, like reducers, sagas, etc. E.g:

onLoad: (module, info, props, context) => {
  context.store.replaceReducer({ ...otherReducers, foo: module.fooReducer })

  // if a route triggered component change, new reducers needs to reflect it
  context.store.dispatch({ type: 'INIT_ACTION_FOR_ROUTE', payload: { param: props.param } })
}
As you can see we have thought of everything you might need to really do code-splitting right (we have real apps that use this stuff). onLoad is fired directly before the component is rendered so you can setup any reducers/etc it depends on. Unlike the onAfter prop, this option to the universal HOC is only fired the first time the module is received. Also note: it will fire on the server, so do if (!isServer) if you have to. But also keep in mind you will need to do things like replace reducers on both the server + client for the imported component that uses new reducers to render identically in both places.

When reading carefully: onLoad is fired directly before the component is rendered so you can setup any reducers/etc it depends on NOT when the component is "loaded". I think this is the huge change with the new context store. What do you think.

Furthermore, looking at the sample code on this method, we understand very well it was created for handling this kind of behavior: injecting reducer/saga before rendering the component.
Unfortunately, it seems now that the state available in the mapStateToProps does not reflect the last one with the injected reducer.

I do not know how to fix this kind of issue, do you think it is now a react-universal-component issue?
Is is possible to lazy load a component and inject a reducer just before its loading?

Thank you,

@jwugit
Copy link

jwugit commented Dec 12, 2018

@GuillaumeCisco
Have you make sure the inject function finishes before the component is loaded?
ReplaceReducer is an asynchronous function.

@markerikson
Copy link
Contributor

@jwugit : no, store.replaceReducer() is synchronous:

  function replaceReducer(nextReducer) {
    if (typeof nextReducer !== 'function') {
      throw new Error('Expected the nextReducer to be a function.')
    }

    currentReducer = nextReducer
    dispatch({ type: ActionTypes.REPLACE })
}

@GuillaumeCisco
Copy link

Yes I can confirm the reducer is well injected.

@jwugit
Copy link

jwugit commented Dec 12, 2018

@markerikson thanks for clarifying.
I think i have mixed it up with the async nature of a component render with its state updates.
Doesn't React components cycles and render before the redux store update propagates down to the components usually?

@GuillaumeCisco
not sure where your code logic would have done something different.
this is my wrapper component. Maybe it can help give you some insights.

/**
 * This is a wrapper component to dynamically call injectReducer to add a reducer to the redux store
 *
 * */
import React from 'react';
import { ReactReduxContext } from "react-redux";

class ReducerLoader extends React.Component {
    constructor(props) {
        super(props);
        const { store, storeState, keyName, reducer } = props;

        if (!storeState[keyName]) {
            store.injectReducer(keyName, reducer);
        }
    }

    render() {
        const {keyName, storeState} = this.props;

        if (!storeState[keyName]) {
            return <div>loading</div>;
        } else {
            return this.props.children;
        }

    }
}

export const withReducer = (key, reducer) => WrappedComponent => {
    return (props) => {
        return (
            <ReactReduxContext.Consumer>
                {({ store, storeState }) => {
                    return <ReducerLoader keyName={key} reducer={reducer} store={store} storeState={storeState}>
                        <WrappedComponent {...props} />
                    </ReducerLoader>;
                }}
            </ReactReduxContext.Consumer>
        );
    };
};

usage sample:

import React from 'react';
import { connect } from "react-redux";
import { fruitAddCount, fruitRemoveCount } from '../../redux/actions/fruitAction';
import {withReducer} from "../../components/withReducer";
import {fruitReducer, name as reducerName} from "../../redux/reducers/fruitReducer";

import style from './style.module.scss';

class Fruit extends React.Component {

    render() {
        const { counter, fruitAddCount, fruitRemoveCount } = this.props;
        return (
            <div className={style.random}>

                <h1>
                    this is the Fruit Page {counter}
                </h1>
                <button onClick={() => {fruitAddCount(1)}}>ADD</button>
                <button onClick={() => {fruitRemoveCount(1)}}>REMOVE</button>

                <div className={style.fruits}>

                </div>
            </div>
        );
    }
}

const mapStateToProps = (state) => {
    const reducer = state[reducerName];
    return {
        counter: reducer.counter
    };
};

export default withReducer(reducerName, fruitReducer)(connect(
    mapStateToProps,
    { fruitAddCount, fruitRemoveCount } // injects increment and decrement
)(Fruit));

How you load it should not be an issue.
I used react-loadable but it is also tested and working with the new react lazy/suspense.
Should work fine with react-universal-component also.

@GuillaumeCisco
Copy link

Thank you @jwugit for your code. I tried to make it works with injectReducer from redux-reducers-injector without success. As the store was before passed by reference it was working, but now with the context behavior, it looks like redux-reducers-injector is now irrelevant.

I then created a new way to replace the reducers looking at your code. It still does not work.
Does your code really work? In your store.injectReducer(keyName, reducer); call, you correctly set a new reducer in the store (can confirm with store.getState()), but in the render of your ReducerLoader Component, the storeState is not updated! So you will always see a loading component.

Is there an easy way to update the storeState?

First of all, I think react-universal-component need to be updated for correctly injecting reducer in a lazy load way. Then, I'll try to update redux-reducers-injector for having an easy tool for replacing reducers.

Thanks,

@GuillaumeCisco
Copy link

Ok folks, after a lot of try and fail playing with the new React Context philosophy and the ReactReduxContext exposed, I have some questions.

Tell me if all my assertions are correct:

  • ReactReduxContext is a context that includes the store and storeState which is the last store.getState() call
  • Using store.replaceReducer will not update storeState, but store.getState() will give the last updated state with the right reducers.
  • Using mapStateToProps, the first parameter state is a reference to storeState.

With this in mind, how to create a HOC which can store.ReplaceReducer which will update the storeState i.e the first parameter of mapStateToProps to the last updated version of the state.

I understand now why my dirty patch was working: Using ReactReduxContext._currentValue.store.getState(); in the mapStateToProps gave me the last updated state.

I'm still learning how to use React Context correctly, but I don't see any way to correctly update the context, except than using setState, or maybe expose a method from ReactReduxContext to update it.

Thank you for your corrections,

@timdorr
Copy link
Member

timdorr commented Dec 13, 2018

@GuillaumeCisco You should dispatch an action like you would normally do with Redux. But if you're replacing the reducer, Redux will do that for you: https://github.com/reduxjs/redux/blob/master/src/createStore.js#L227

@GuillaumeCisco
Copy link

Thank you @timdorr .
This is what I do, replacing the Reducer.

Here is a draft for a HOC which should update the reducers:

import React, {Component} from 'react';
import {ReactReduxContext} from 'react-redux';
import {combineReducers} from 'redux';
import set from 'lodash/set';
import has from 'lodash/has';

const combine = combineReducers;

export function combineReducersRecurse(reducers) {
    // If this is a leaf or already combined.
    if (typeof reducers === 'function') {
        return reducers;
    }

    // If this is an object of functions, combine reducers.
    if (typeof reducers === 'object') {
        let combinedReducers = {};
        const keys = Object.keys(reducers);
        for (let i = 0; i < keys.length; i++) {
            const key = keys[i];
            combinedReducers[key] = combineReducersRecurse(reducers[key]);
        }
        return combine(combinedReducers);
    }

    // If we get here we have an invalid item in the reducer path.
    throw new Error({
        message: 'Invalid item in reducer tree',
        item: reducers,
    });
}

const LoadReducer = (store, storeState, keyName, reducer, props) => (WrappedComponent) => {

    class Consumer extends Component {

        constructor(props) {
            super(props);

            if (!storeState[keyName]) {
                if (!has(store.injectedReducers, keyName)) {
                    set(store.injectedReducers, keyName, reducer);
                }
                store.replaceReducer(combineReducersRecurse(store.injectedReducers)); //should dispatch an action and call mapStateToProps with new state
            }

            this.state = {
                store,
                storeState: store.getState(),
            }
        }

        render() {
            return !this.state.storeState[keyName] ? <div>loading</div> : <WrappedComponent {...props}/>
        }
    }

    return <Consumer/>
};

export const withReducer = (key, reducer) => (WrappedComponent) => {
    return (props) => {
        return (
            <ReactReduxContext.Consumer>
                {({store, storeState}) => {
                    return LoadReducer(store, storeState, key, reducer, props)(WrappedComponent);
                }}
            </ReactReduxContext.Consumer>
        );
    };
};

I call it like:

const mapStateToProps = (state) => ({
        item: state.item.name,
    });

export default withReducer('item', reducer)(connect(mapStateToProps)(Comp));

The first parameter state does not contains the item reducer.

@jwugit
Copy link

jwugit commented Dec 13, 2018

@GuillaumeCisco
here is my sample.
https://codesandbox.io/s/l2401r64qz

If you inspect the redux in dev tools, you will see that there are 2 reducers.
When i click on home page button, the example reducer will load some async data.

Then navigate to fruit page.
The fruit reducer will be injected.
If you inspect the reducers, there will be 3 of them now.
The previous loaded data is still there an unaffected after the inject.
If you trigger the fruit reducer actions to increase and decrease, they change properly.

So basically existing states is preserved, new reducer are injected before component is loaded.
Using new reducers cause no issue.

For me it works.

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

No branches or pull requests

5 participants