this.emit('change') vs this.emitChange()

[macalinao]

It seems odd to have this one event not use a constant when every other emitted event should be using constants. I think that there should be this method in stores to prevent typos.

[BinaryMuse]

I've been thinking about this quite a bit since #58, but I'm hesitant to make it part of the base store API.

It's likely that an event named "change" is probably the most common event used, especially since StoreWatchMixin uses it, but it's certainly far from the only one I've used (an app I have in production emits several different events), and users are free to use whatever event name(s) they want.

I've considered a method called changed() that does the same thing that makes the whole system feel a bit more cohesive (e.g. you don't really have to know that you're emitting an event, as the changed method and the StoreWatchMixin take care of it for you), but I'm leery of "too much magic"—my goal is convenience, but without hiding the underlying power that the system is based on (e.g. the fact that you have an entire EventEmitter at your disposal).

The typo argument is valid, but I think a bit weak, as one could easily define a constant somewhere and use it for event emissions:

var CHANGE = "change";
// ...
this.emit(CHANGE);

I'm also fond of the idea of mixins (see #37), and while the proposed PR adds a considerable amount of complexity to Fluxxor's stores, version 2 of the Fluxxor API will have the ability to use completely custom stores if you so wish.

In the meantime, if there are methods you want to use in multiple stores, you can use Object.assign (or the object-assign module or extend from Underscore/assign from Lo-Dash) to mix them in to a spec:

// store_mixin.js
module.exports = {
  emitChange: function() { this.emit("change"); },
  // maybe other methods here
};

// my_store.js
var assign = Object.assign || require("object-assign"),
    storeMixin = require("./store_mixin");

module.exports = Fluxxor.createStore(assign({}, storeMixin, {
  handleEvent: function() {
    // ...
    this.emitChange();
  }
}));

I'm going to leave this open for now to foster discussion.

[davejacobs]

One thing that I really like about this.emit("change") is that it encourages you to think about whether custom events make sense for your particular app. Without the explicit string, it might be easy to assume that custom events are bad practice.

[macalinao]

I mean, the change event is enforced by StoreWatchMixin. Custom events
are definitely fine, but an event that's enforced by the library imo should
have a dedicated function (or constant) for this.

On Sun, Jan 11, 2015 at 2:50 PM, David Jacobs notifications@github.com
wrote:

One thing that I really like about this.emit("change") is that it
encourages you to think about whether custom events make sense for your
particular app. Without the explicit string, it might be easy to assume
that custom events are bad practice.

—
Reply to this email directly or view it on GitHub
#91 (comment).