Chaplin is too complicated

[akre54]

The code base for Chaplin is overgrown and needs some pruning. It makes it very hard to bring on a new team member to a project because of how complicated it is.

I'd like to discuss paring down on some of the features that detract from the otherwise well-designed framework:

1. Remove most of the global events and reliance on mediator.

2. Remove imperative events created with mediator.setHandler. This is an abuse of the events pattern. Events are an inversion of control, used to describe something that has happened rather than something that should happen as in the case of a method invocation. At the moment there is a rats nest of logic in dispatcher, router and composer to fit this backwards pattern and makes it hard to trace logic for what the method is trying to do. Use direct references to the methods instead.

3. Use adapter pattern for View instead of checking for the existence of $. Allow simple methods to be overwritten by an adapter if $ is not desired. Will clean up View and CollectionView greatly.

   3a. Chaplin requires $ to be set after Backbone is loaded but before Chaplin itself is, and it can't be changed during the lifetime of the application. This is frustrating but not a deal-breaker.

4. Work out a pattern for plugins or mixins for edge case features. Chaplin is not very extensible at the moment.

5. Compose needs to be simplified. Composer redesign #779 is a good start.

6. Fix memory management in Controller lifecycles to be easy to follow. Composer is one of Chaplin's best features. Give it space to grow.

7. Fail early in methods that don't have easy args checking, but don't throw errors unless absolutely necessary (let the JS runtime handle problematic arguments)

8. Figure out how to make querystring parsing work with the latest Backbone and reduce the shims that Chaplin.History requires to override Backbone.History.

9. Fix documentation. In lieu of that, make source clearer to reason about.

Lastly, the tests are a mess. They test implementation details more than they do behavior, and are currently very brittle when making changes.

Chaplin has some really nicely designed features, but they're obscured by some bad patterns. I'm happy to discuss more and help make changes where needed.

[molily]

Thanks for the detailed critique! I’m just starting with one bit of it:

Remove most of the global events and reliance on mediator.
Remove imperative events created with mediator.setHandler. This is an abuse of the events pattern.

I’m not sure if it’s clear for everyone what the different purposes of the mediator are and why Chaplin came to this solution in the first place. In short, it’s the only global singleton object, a Publish/Subscribe event bus, a Request/Response implementation. And most importantly it’s a solution for solving most dependency issues, both inside of Chaplin core and application parts. It’s the counterpart of the basic rule of Chaplin that no m/v/c/router/… instance is globally available. This has a lot of consequences, for example testability since the mediator is the only dependency and it behaves like a mock object.

Regarding Pub/Sub and Req/Resp, these are quite proven and tested patterns that are present in other Backbone-based architectures.

There are several other ways to solve the dependency problem. “Make everything global” is still the most popular solution for Backbone apps. Alternatives are singletons, or more complex dependency injection with factories/services/etc, or nested, non-isolated scopes. Other libraries solve these problems with more code and tougher conventions, for example, module declaration conventions.

I’m fine with any solution that meets these requirements and improves the situation.

[akre54]

I hope these notes come off as more constructive than critical. I consistently argue for Chaplin in projects over Ember, Angular, Marionette, and plain Backbone because of its distinguishing features. It just becomes a harder sell when the stakeholders try to dig into the library code or when we need to bring new people on.

To your points:

Regarding Pub/Sub and Req/Resp, these are quite proven and tested patterns that are present in other Backbone-based architectures.

Definitely. But in the best circumstances they tend to be scoped down and act as a channel only to the module or modules that require knowledge of the event. Why should region:show be a global event anyway? The global event bus is the biggest Backbone pattern with the potential for abuse, for the same reasons any global scope is a bad idea.

And why are all the handler methods named in the imperative (i.e. telling the application to do something) instead of the active infinitive (i.e. alerting that something is happening or has happened) as the rest of Backbone's events are? Dubbing them "handlers" doesn't change the fact that we have multiple functions registered to the same action, which is a frankenstein amalgam of parts of the Events pattern mixed with traditional imperative programming. In Chaplin as in most programming, names of methods that do things tend to begin with imperative, active-case verbs (i.e. executeBeforeAction()). When you make these Events, it muddies their intent and usefulness.

the basic rule of Chaplin that no m/v/c/router/… instance is globally available.

Sure, but you're just shifting responsibility to the mediator to handle global state changes rather than the individual parts of the Application instance, which is for all intents and purposes the primary object in a Chaplin app.

Why is the mediator the global singleton that defines the events in your app? Why not the Application instance? Why don't we have a single Application instance as the event channel, and directly call the Dispatcher and Composer singletons already attached to it?