reflux / refluxjs

A simple library for uni-directional dataflow application architecture with React extensions inspired by Flux
BSD 3-Clause "New" or "Revised" License
5.36k stars 330 forks source link

Any issues with using mapStoreToState to replicate Reflux.connect named state properties? #512

Closed geuis closed 7 years ago

geuis commented 7 years ago

Specifically, we're working with a codebase that still uses Reflux.connect in mixins to connect to stores. Reflux.connect had the requirement to provide a property to map the store to.

We've upgraded to the latest Reflux version and the old behavior is still supported. However, now that we're using ES6 classes and can't use mixins, all of the properties from the stores are just copied verbatim to the component state and doesn't seem to provide an ability to map them to a specific property like before. This was causing some problems.

Yesterday I tried using this.mapStoreToState in the constructor in the following manner to replicate the old behavior and it seems to be working on the surface. e.g.

this.mapStoreToState(userStore, (store) => this.state.users = store);

This isn't how mapStoreToState seems to be meant for use according to the documentation, but it is working.

Are there any problems with this technique that I'm not aware of?

EDIT UPDATE

I read the docs over again and I feel like the following might be the actual right way to do what I'm intending. Feedback is appreaciated.

this.mapStoreToState(userStore, (store) => {users: store});
BryanGrezeszak commented 7 years ago

mapStoreToState has the abilities to map property names that connect has, but it is not a replication of it. It is its own new fully functional feature that works in its own way.

Look at the example given for mapStoreToState:

    constructor(props)
    {
        super(props);

        this.mapStoreToState(MyStoreClass, function(fromStore){
            var obj = {};
            if (fromStore.color)
                obj.color = fromStore.color;
            if (fromStore.data && fromStore.data.classToUse)
                obj.class = fromStore.data.classToUse;
            return obj;
        });
    }

The fromStore is what was changed via setState in the store, and the object being returned from the function needs to be the object you want to send to the setState that you want to happen to the component.

So if you called this.setState({color:'blue'}) in your store, then the fromStore is going to be that {color:'blue'} object.

If color isn't relevant to your component, then simply return an empty object and your component will not re-render. If it is, then return an object that is the object you'd want it to send to this.setState in that component to handle such a change. Such as {theColor: fromStore.color}

geuis commented 7 years ago

@BryanGrezeszak Right, I get how its a filter mechanism. But my question is if its appropriate to use it in either of the 2 examples I mentioned as a way to replicate the previous connect() mixin behavior. Or is there a way to use connect with an ES6 class?

BryanGrezeszak commented 7 years ago

I don't really know what's in your store, or where you're trying to get that in your component. So I can't really give you a verbatim solution. I can only explain how mapStoreToState works for you.

BryanGrezeszak commented 7 years ago

Another note on this: also remember that some calls to setState in your store may not be changing any properties relevant to the component in question. So don't assume the presence of any given property on that fromStore object. Check if any relevant properties to map exist, and if there are none, then return an empty object and your component won't rerender.

It's actually quite similar to a reducer function concept, but for each individual component.

aziz commented 7 years ago

We're having the exact same issue that @geuis described. We're trying to upgrade from an old Reflux to the latest version. all of our stores are wirtten in Reflux.createStore style and are connected to components using the mixins:[Reflux.connect(EventStore, 'data')] style.

Here's the example

export default React.createClass({
  displayName: 'PageMyAccount',

  mixins: [
    Reflux.connect(EventStore, 'data'),
    Reflux.connect(appSettingsStore, 'appSettings')
  ],
  render () {...}

Now we have re-written our store to use the class notation (class EventStore extends Reflux.Store) but we need to be able to connect these stores to components under a specific key.

I can't find a way to do this properly? any idea?

I can't use this.store or this.stores cause the data will not be under the specific key that use to be defined as second parameter of Reflux.connect

mapStoreToState is also not working properly, although it renders fine initially, every setState in the store will totally destroy the whole state object using the below code:

constructor (props) {
  super(props);
  this.mapStoreToState(EventStore, (store) => ({ data: store }));
  this.mapStoreToState(appSettingsStore, (store) => ({ appSettingsStore: store }));
}
geuis commented 7 years ago

@aziz So we ended up writing a helper function:

connect = (instance, store, property) => {
  instance.mapStoreToState(store, () => ({
    [property]: store.getInitialState()
  }));
}

And its used like this in the class constructor:

class OurComponent extends Reflux.component {
  constructor (props) {
    super(props);

    connect(this, ourStore, 'ourstoreProp');
  }
}