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

Mixin for watching stores #17

Closed
vesparny opened this issue Feb 10, 2015 · 12 comments
Closed

Mixin for watching stores #17

vesparny opened this issue Feb 10, 2015 · 12 comments

Comments

@vesparny
Copy link

I was trying to create a Mixin for automatiacally subscribe/unsubsribe Stores and use it like this:

mixins: [StoreListener(this.props.getStore('my-store')],

I don't know why is not working, just getting a blank page with no error in cosole. Am I missing somenthing?

I ended up declaring inside the component which needs the mixin a method returning the used stores

getStores(){
    return {
      'my-store': this.props.flux.getStore('my-store')
    };
  },

and here the mixin:

'use strict';

var StoreListenerMixin = {
  getInitialState() {
      return this.getStateFromStores(this.props);
    },

    componentDidMount() {
      Object.keys(this.getStores()).forEach(key =>
        this.getStores()[key].addListener('change', this.handleStoresChanged)
      );
      this.setState(this.getStateFromStores(this.props));
    },

    componentWillUnmount() {
      Object.keys(this.getStores()).forEach(key =>
        this.getStores()[key].removeListener('change', this.handleStoresChanged)
      );
    },

    handleStoresChanged() {
      if (this.isMounted()) {
        this.setState(this.getStateFromStores(this.props));
      }
    }
};

module.exports = StoreListenerMixin;

Everything is working like a charm, just wondering if there is a smarter way for passing the interested store from the component to the mixin using this.props.flux

@acdlite
Copy link
Owner

acdlite commented Feb 10, 2015

The reason this doesn't work

mixins: [StoreListener(this.props.getStore('my-store')],

is because mixins is a static property, so it doesn't have access to component props.

On a related note, the suggested way of passing the Flux instance is to use React context. The documentation needs to do a better job of explaining this #12.

I've just started work on a React mixin pretty similar to yours that should make all of this easier. I'll open up a separate issue to track that.

@vesparny
Copy link
Author

Ok thanks, you wrote the best flux library.
I just started to write a small mobile app and it works well.
Can't wait to move Morpheus over flummox.

anyway, why do you think is a better approach to use contexts?
Is it documented as a stable api on React docs?

@acdlite
Copy link
Owner

acdlite commented Feb 10, 2015

Thanks for the kind words!

Context is suggested instead of props because it exposes your Flux instance to arbitrarily deep components. It's currently an undocumented feature, but the project maintainers have assured us that it's safe to use: https://www.youtube.com/watch?v=EPpkboSKvPI&list=PLb0IAmt7-GS1cbw4qonlQztYV1TAW0sCr#t=529

React Router uses context heavily, too.

@martintietz
Copy link
Contributor

This is the mixin (or a first draft that is heavily inspired by https://github.com/gaearon/flux-react-router-example/blob/master/scripts/mixins/createStoreMixin.js) that I'm currently using. It presumes that you use React.withContext and that you define a function called getStateFromStores which returns the data you want to store in your state.

'use strict';

var React = require('react');

module.exports = function createStoreMixin(stores) {
    return {
        contextTypes: {
            flux: React.PropTypes.object.isRequired
        },

        getInitialState() {
            this.stores = {};
            this.flux = this.context.flux;

            if (typeof this.flux === 'undefined') {
                throw new Error("Print some error message...");
            }

            stores.forEach( (store) =>
                this.stores[store] = this.flux.getStore(store)
            );

            return this.getStateFromStores();
        },

        componentDidMount() {
            stores.forEach( (store) =>
                this.stores[store].addListener('change', this.handleStoresChanged)
            );

            this.setState(this.getStateFromStores());
        },

        componentWillUnmount() {
            stores.forEach( (store) =>
                this.stores[store].removeListener('change', this.handleStoresChanged)
            );
        },

        handleStoresChanged() {
            if (this.isMounted()) {
                this.setState(this.getStateFromStores());
            }
        }
    };
};

You'll use it like this: mixins: [createStoreMixin(['someStore', 'anotherStore'])]. After that you can use your stores via this.stores.someStore and the flux component via this.flux.

@SimonDegraeve
Copy link

I end up with something similar, however I do not understand why we should call the callback when removing the listener. Any thoughts?

@acdlite
Copy link
Owner

acdlite commented Feb 11, 2015

@SimonDegraeve Are you referring to removeListener('change', this.handleStoresChanged)? That doesn't call the this.handleStoresChanged; it just lets the event emitter (the store) know which listener to remove.

https://iojs.org/api/events.html#events_emitter_removelistener_event_listener

@SimonDegraeve
Copy link

Indeed, I just realised that working on my mixin ;-)

@acdlite
Copy link
Owner

acdlite commented Feb 11, 2015

@SimonDegraeve Do you want to post your mixin here so we can see it? I started writing one earlier today, but I'd love to take a look at everyone else's so we can gather all the best ideas.

@SimonDegraeve
Copy link

Yes, that's my plan.... but to be accurate it not a mixin as React defines it, but a method for ES6 class. Should be done in the next 30min.

@acdlite
Copy link
Owner

acdlite commented Feb 11, 2015

@SimonDegraeve Sweet. I'll finish mine up, too. I'm taking a slightly different approach, I think, so it will be good to compare.

@SimonDegraeve
Copy link

I think it is easier with a gist. You can check here.

It is inspired by Reflux connect mixin.

@acdlite
Copy link
Owner

acdlite commented Feb 11, 2015

Opened a pull request with my initial version. #18 Let's move this conversation over there

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

4 participants