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

init in StoreMethods #184

Closed iofjuupasli closed 7 years ago

iofjuupasli commented 9 years ago
Reflux.StoreMethods.init = function () {
    console.log('global');
};
Reflux.createStore({
    init: function () {
        console.log('local');
    }
});

it logs only local to console. But I expected both, global and local The problem is here: https://github.com/spoike/refluxjs/blob/master/src/createStore.js#L55 It overrides methods from Reflux.StoreMethods with methods from definition Reflux.StoreMethods is like defaults for definition. But It's unobvious. For comparison, in mixins both init will call. As it works in React for livecycle methods (e.g. componentWillMount, getInitialState). So, maybe it'll be better if Reflux.StoreMethods will works as global mixin rather than defenition defaults

For example it can be helpful for hydrate stores in it's init method. Just for example, something like:

Reflux.StoreMethods.init = function () {
    this.data = window.app.payloads[this.name];
};

Or using for hook into preEmit or shouldEmit for some reason. One another issue here, is only one possible global method. What if we have two separated modules, that use init method in all stores.

Reflux.StoreMethods.init = function () {
    this.data = window.app.payload[this.name];
};
Reflux.StoreMethods.init = function () {
    console.log('was initiated: ', this.name);
};

Second assignment will override first. Instead it'll be better to use method for adding new functionality:

Reflux.StoreMethods.add({ init: function () {} });

That looks like mixin for all stores:

var LogMixin = {
    init: function () {
        console.log('was initiated: ', this.name);
    }
};
Reflux.StoreMethods.add(LogMixin);

As we already have mixins, probably this feature will not greatly increase code size.

spoike commented 9 years ago

Yeah. I agree that StoreMethods doesn't need to be a "defaults" object. It should mix itself in, and merge function calls. Unless anyone objects I can't see why we shouldn't have this changed.

influx6 commented 9 years ago

Am in agreement!

ConradIrwin commented 9 years ago

Would be great!

BryanGrezeszak commented 7 years ago

With the ES6 patterns this is not very relevant any more. Closing unless someone thinks there's a good reason to pursue it.