f / delorean

An Agnostic, Complete Flux Architecture Framework
749 stars 40 forks source link

"You are attempting to create more than one dispatcher." #89

Closed nigelwtf closed 9 years ago

nigelwtf commented 9 years ago

Yo @darcyadams - I've just updated the NPM package in my project and noticed I'm now getting this: "You are attempting to create more than one dispatcher. DeLorean is intended to be used with a single dispatcher. This latest dispatcher created will overwrite any previous versions."

Seems like a bit of an anti pattern to me, and IMHO I would expect to use multiple dispatchers to effectively separate any concerns:

A bit of a rigmarole for me, as I have to make sure this is running smoothly on everybody's machines. Will have to rollback to a previous version, and make sure we stay with that.

This seems like a deal breaker :( Could you run me through the design decisions for this?

darcyadams commented 9 years ago

delorean was previously using an internal react property _.owner to recursively traverse up the component tree to find the first dispatcher prop. However, React killed _.owner in version 0.13, leaving (AFAIK) no way to traverse up the tree. This left us with requiring the developer to pass the dispatcher property all the way down the component tree to anywhere it was needed (a huge hassle in larger apps) or settle on enforcing a single dispatcher and creating in internal reference to that dispatcher when it is created. We obviously opted for the latter approach. As I wrote in PR https://github.com/deloreanjs/delorean/pull/85, it is not my favorite approach, and am open to other ideas for how to attach the dispatcher (or dispatchers) to components. However, I will defend it in the sense that flux is really intended to be built around a single dispatcher. I get your point about separating responsibilities, but I would argue this can be done through modularizing your action files, and feeding them to the dispatcher when you app starts up. EG, you have CartActions and SearchActions files, and they both get required by your dispatcher file, concatenated and fed to createDispatcher.

from the flux architecture overview on facebook's site ...

When a user interacts with a React view, the view propagates an action through a central dispatcher, to the various stores that hold the application's data and business logic, which updates all of the views that are affected. This works especially well with React's declarative programming style, which allows the store to send updates without specifying how to transition views between states.

hope this helps, and like I said, I'm open to proposals and PRs for a better approach that would allow the flexibility you want to create multiple dispatchers, if that is the approach that works best for you.

Personally, I use delorean in a huge application, with just a single dispatcher and the multiple action files I described above, and it is working great. So let me know if you want some more detail about how that setup works.

nigelwtf commented 9 years ago

Ah cheers for that. I guess I'm sticking with the 0.9.3 package and passing the dispatcher down manually!

I get what you mean with separating the actions. Would be good to see an implementation though ;)

darcyadams commented 9 years ago

Here is what my dispatcher file looks like (sorry if coffeescript is not your thing, added comments to explain the gist)...

Essentially the idea is to separate your monolithic dispatcher into smaller files, which are much easier to work with and understand. Also, implement a system to warn developers of any naming conflicts that may occasionally occur when combining many files into a single class.

It took me a bit to reconcile with the idea of a massive singleton at the center of my project, but the simplicity and reliability of the flux approach has won me over. This workaround at least allows me to break down the various portions of my application as far as development is concerned.

With all stores wired to a single dispatcher It's very nice for any store to be able to respond to a single dispatched action, eg all stores can do their own cleanup after a logout action is dispatched.

{Flux} = require 'delorean'

Global = require './stores/global'
System = require './stores/system'
UserStore = require './stores/loggedin_user'
Patients = require './stores/patients'
PopoverModal = require './stores/popover_modal'
Chart = require './stores/chart'

# Create a baseline object to feed to createDispatcher
dispatcher = 

  viewTriggers: {}

  getStores: ->
    {
      global: Global
      system: System
      loggedInUser: UserStore
      patients: Patients
      popoverModal: PopoverModal
      chart: Chart
    }

# This function takes and action file, and adds it's methods (actions) and viewTriggers to the baseline object above
# it console.warns when a name conflict is detected
processActions = (actionModule) ->
  for actionName, action of actionModule
    if actionName is 'viewTriggers'
      for triggerName, triggerMethod of action
        if dispatcher.viewTriggers[triggerName]? then console?.warn "Duplicate View Trigger: #{triggerName}. #{triggerName} will not be applied to the dispatcher viewTriggers hash."
        else dispatcher.viewTriggers[triggerName] = triggerMethod
    else
      if dispatcher[actionName]? then console?.warn "Duplicate Dispatcher Action: #{actionName}. #{actionName} will not be applied to the dispatcher."
      else dispatcher[actionName] = action

# Require action modules inside of this Array
# This goes through the separate action files and runs them through the processActions function above, which adds them to the dispatcher
processActions(actionModule) for actionModule in [
  require './actions/system'
  require './actions/loggedin_user'
  require './actions/patients'
  require './actions/popover_modal'
  require './actions/chart'
  require './actions/global'
]

module.exports = Flux.createDispatcher dispatcher
nigelwtf commented 9 years ago

That's really helpful! Cheers!