Skip to content

Instantly share code, notes, and snippets.

@abdullin
Last active October 25, 2017 08:01
Show Gist options
  • Save abdullin/5953ab4f5eae0a7fc8f9 to your computer and use it in GitHub Desktop.
Save abdullin/5953ab4f5eae0a7fc8f9 to your computer and use it in GitHub Desktop.

This is a response to Bill Fisher regarding experience with Flux:

@abdullin Also, can you clarify what you mean by "solution structure"? I am thinking about revising the examples soon.

Currently all flux samples (that I've seen) group files into folders based on technical similarity. For example, stores go with stores, action creators reside in the same folder shared with the other action creators.

This pattern works quite well for smaller projects. It feels especially good for the sample projects of various MVC frameworks, when you have just a bunch of controllers, models and views.

However, as we discovered on some production projects, such approach doesn't scale well. At some point you end up with dozens of technically similar files per folder and logically messy solution.

  1. Doesn't scale well.
  2. While you work on a component, you have to suffer from extra context switching overhead (jumping between various folders).
  3. Solutions become more "entangled" than needed.
  4. More complex IDE features are needed to support the workflow (e.g. context-aware navigation and completion).
  5. More merge conflicts, since it is harder to bound work by a feature (and communicate that).

We discovered that aligning solution with the domain model leads to a better design in the long run. E.g. we try to group files by their functionality or feature. This would mean, that different technical elements of a single feature could be squashed into one folder or onto one file. For example, for Flux we are considering to have a single folder for "News feed", which would contain all related action creators, fetchers and stores. Of course, there could be some other components that the this chat page would use (e.g. avatars, like buttons or user stores), such components would reside in their own folders. On the overall, such component decomposition is requires more effort than simply grouping files by class type. However, we value both the process (it leads to a deeper insight) and the outcome (solution that is more simple to think and reason about).

Of course, this is just something that seems to work only in a subset of cases I've been exposed to. There can easily be a deeper pattern which I fail to recognize.

@fisherwebdev
Copy link

Hi Rinat,

This treatise is wonderfully thorough and quite thought-provoking. I loved reading through it and comparing your approach to our own. Many parts of this resonated with me, but of course there are details where we differ. Some of those details are more important than others.

modules

I initially stumbled on some of the terminology you have used here. For example, at Facebook we use the term module to signify a CommonJS module, which has its own closure. This has a very specific JavaScript-oriented meaning for us. But I see now that you are talking about directories used purposefully and meaningfully to support a specific way of thinking about an application.

I think your approach to module-directories could work quite well, given how you have described it. We try to do the same thing with naming conventions, but I would be interested to try the same conventions made more concrete in the directory structure. Like you said, it might help to focus how we think about the various domains in the application.

events, actions and dispatches

More significantly, I am concerned about the use of the term event being used liberally here to describe a number of different processes. While I understand that you might be talking about events in a more abstract sense, I would not want to overlook that Flux dispatches very intentionally avoid being JavaScript events, which are uncontrolled, asynchronous processes.

The mechanics of the dispatcher are vital to how we've been able to keep our applications resilient as they grow, especially the waitFor() method, which allows the stores to declaratively depend on one another. JavaScript promises are better than simple events, but we've found they still don't offer us enough control to ensure our code doesn't become a tangled mess. The dispatcher was our solution to the problem. The dispatcher is the core of what makes Flux different from other approaches. It is the mechanism that enforces the unidirectional data flow, helps us manage the complexity of dependencies between stores, and allows the stores to be completely in control of themselves and their data domain.

stores: limiting reuse and duplicating code

This is an interesting, somewhat radical idea. But I'm having trouble imagining how different parts of the application that rely on the same data stay in sync with each other over time. I understand that we could duplicate the code in multiple places, but we would need to maintain that duplication, and I can't imagine that would happen without a small failure rate. Perhaps a small amount of duplication is not terrible, however, and like you said, it can provide a way to avoid dependencies between stores.

the past

I loved this:

[Actions/events] are carefully crafted to reflect meaningful outcomes of some real-world processes. ... They are generally named in the past tense to emphasize that: UserRegistered, PaymentProcessed, MessageSent (you can't change the past, you just have to deal with it).

This really speaks to me. I will always name my actions in the past tense from now on!

Also:

Put all actions (type constants and probably constructors) into a single module that is easy to discover and explore. That is the API.

This is what I have been doing recently too, but we use simple action creators that are nothing more than a helper function that sends an action through the dispatcher. Recent projects of mine have been putting them all in one file. The large applications I've seen at Facebook have taken a more domain-driven approach to this, and I haven't yet been part of a large project that put them all in one file. I'm sure the number of the action creators would eventually get to the point where we would need to split them up. On the other hand, why break these into separate files until we really need to do it? It's much nicer to have them all in one place.

testing

It sounds like you test in very similar way to the way we test. If interested, I recently wrote a blog post about testing Flux stores with our automocking unit test framework, Jest.

Thanks again for writing such a thoughtful response. I'll be thinking it over for quite a while, and I'm sure I'll experiment with some of these ideas in my future projects.

@tcoopman
Copy link

Hi Rinat, Bill,

Thanks for these insights, they are very useful.

I was recently thinking more about actions/events in the past. Because I do love the idea the actions are outcomes of real-word processes and thus should be named in the past because they can't change. But I have some concerns about this, and I was wondering what you think about it.

Validating user input

At the moment my stores are responsible for the validation and I do something like this:

// action -> MESSAGE_SUBMITTED

messageStore.messageSubmitted(message) {
  if (invalid(message)) {
    setErrorNotification()
  } else {
    // do things in the store
    // deleteErrorNotifcation()
  }

  emitChange()
}

So it's possible that a message is not valid so submitted seems wrong here. It's only after the validation that the message really is submitted.

Failed on the server

An action can fail on the server, so, again, submitted seems wrong.

Possible solution

It's the responsability of the api (actionCreator) to call the server, and maybe also to validate the messages, so it would be possible to handle all things that can fail in the api and change my api interface like this:

for validation:

api.messageSumbit {
  if (invalid(message)) {
    dispatch('MESSAGE_INVALID_SUBMITTED')
  } else {
    dispatch('MESSAGE_VALID_SUBMITTED')
  }
}

with submitting to the server:

api.messageSumbit {
  if (invalid(message)) {
    dispatch('MESSAGE_INVALID_SUBMITTED', message)
    // Invalid messages are not send to the server
  } else {
    dispatch('MESSAGE_VALID_SUBMITTED.SEND', message)
    sendMessageToServer(message).then(function(result) {
      dispatch('MESSAGE_VALID_SUBMITTED.RESOLVED', message)
    }).catch(function(error) {
      dispatch('MESSAGE_VALID_SUBMITTED.REJECTED', message)
    });
  }
}

The inspiration for my solution came mostly from here: http://www.code-experience.com/async-requests-with-react-js-and-flux-revisited/

This has the advantage that all actions can be in the past again, but it makes the solution more complex. Furthermore, as soon as localstorage is added, that could fail too and that must also be taken into account.

On the other hand, syncing between localstorage and the server is not trival, so dealing with it explicitly wouldn't be so bad.

Is this the way you would handle this, or do you do this differently?

@fisherwebdev
Copy link

Hi Thomas,

Yes, this is essentially how I would do validation, but only if my validation was somewhat simple, like a regex to validate that the text looks like an email address.

However, if the validation requires me to examine the state of the stores, then I would want to move all of this to the store. If I was concerned about race conditions with the resolution of the API call vs. the current dispatch, I would manage that with a check to Dispatcher.isDispatching() in the action creator called from the response handler.

Keeping the validation and the API call in the action creator has the advantage of keeping action creators out of the store. The advantage of having the validation and API call in the store, however, is that application logic resides in the store along with state management.

See also: http://stackoverflow.com/questions/27363225/tracking-ajax-request-status-in-a-flux-application/27459732#27459732

Semantic tangent:
In our action types, we often don't have an agent or other details. They primarily have the form of OBJECT_VERB. There is no agent. In this case, the user has actually submitted the message to the application. They pressed the enter key on their keyboard. And this is what the action is saying about something that happened in the real world. The application has not submitted the message to the store or the API, but this is an internal detail. The results of an XHR may not seem like something happening in the real world, but I would argue that they are -- we become painfully aware of this while developing for the mobile web.

@tomkis
Copy link

tomkis commented Apr 21, 2015

Hello Bill and Rinat,

I would love to read more enriching discussions like this. I have to admit that since the first time I saw Flux, it was really familiar to DDD & EventSourcing (which is a great concept in my opinion). This is my output for the discussion:

  1. Store is something called bounded context in DDD terms, therefore it's quite common to duplicate state between multiple stores
  2. Action represents interaction (be that a user's interaction or API callback) with the application and therefore it's something that happened in the past and there should not be any way to reverse it. (Rinat you have already mentioned it)
  3. Action naming should always reflect what actually happened rather than expose implementation detail

I wish more developers would be aware of those points before they start using Flux in production, as it would lead to much cleaner design. Flux is really powerful weapon but can harm badly if it's not used properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment