chaplinjs / chaplin

HTML5 application architecture using Backbone.js
http://chaplinjs.org
Other
2.85k stars 231 forks source link

Advanced syntax for declarative listeners #800

Open jeron-diovis opened 10 years ago

jeron-diovis commented 10 years ago

For now, declarative listeners only allows to subscribe to single event of single target. So when logic becomes more complex, all it quickly becomes very verbose or requires to refuse using declarative approach.

It's for discussion. I propose to allow following:

listen:
  "add,remove collection": "refreshCounter"
listen:
  "sync model,collection": "render"
listen:
  mediator:
    "event1": "handler1"
    "event2": "handler2"
    ...
    "eventN": "handlerN"

I will implement any of these points - need to know with which ones you agree.

molily commented 10 years ago

See #333 and #334 for the initial discussion of listen. Grouping and listening to custom properties was also discussed there.

I agree there’s room for improvement here. Actually I find the Backbone’s {'event targetOrSelector': 'method'} syntax confusing enough. (Why put event and target in one string separated by space and the method name in another? Why only one handler? This is because the handler can be a function, something I don’t find practical.)

I hesitate to overload this with more logic by introducing commas as another separator. If we deviate from Backbone here, we should probably come up with a better syntax instead of adding more logic to the existing syntax, which is fine for simple cases, but not extensible. (Of course there is backwards-compatibility.)

In events, the selector can already contain spaces (and should for readability): 'ev .sel1, .sel2': 'handler1' With several events, it would look like: 'ev1,ev2 .sel1, .sel2': 'handler1'. I find it confusing that there must not be a space after the first comma but there might be a comma in the CSS selector.

jeron-diovis commented 10 years ago

Oh, I missed that selectors can contain commas. It was a bad idea, ok. Let's leave events alone.

As for listen. I read that epic thread and found questions still open.


Custom events channels

It was shown, that it is easy to pass them to view, and you do not seem to have any objections. Also, note, that now the shown way is the only possible, as in Backbone 1.1.0 View::_configure was removed - so there are no more "hacks" with overriding it and doing @listenTo @options.myLittlePony .... All seems to be clear and obvious - which channel came from.

And also I agree with @akre54's opinion - listening to such local "channels" is not so rare issue to redefine _delegateListener only in some particular view. The first thing we will do when first time we will have such need will be redefining _delegateListener in our very base view, to do not return to it never again.

So I think that using only "reserved" channel names didn't prove to be practical.


Multiple events

I hesitate to overload this with more logic by introducing commas as another separator

we should probably come up with a better syntax instead of adding more logic to the existing syntax

But what another solution could be here at all? Anyway we have just a string and all we can do is to introduce more special symbols to it. Or just refuse this feature.

Taking additional advantage on last word in the string is not an option - because, even with fixed channel names, it still looks confusing.

So this question is about "whether to introduce additional separator". I don't see another solution for declarative syntax.


Grouping targets

In that thread you propose approach based on modelEvent properties. Not sure, but for now it seems to be not suitable, as single listen hash is already widely used.

You say that

For me the most important information is the mediator/model/collection part, and the structure should expose this. The code is more readable if this information is on the top level so the code can be read in this order: which target, DOM or model or collection or mediator? > which event type? > which handler function?

It's ok, but why do you think that nesting in listen does not represent that information in the best way?

listen:
  mediator: #  which target?
    "event1": "handler1" # which event type? : which handler function?
    "event2": "handler2"
  model:
    "event3": "handler3"
    "event4": "handler4"

We read this exactly in order you describe. What can be more readable?

Also, to understand what view listens to it is easier to just look what happens in single listen property, than to scroll through class definition searching for multiple *Events properties, placed in random order (because, let's be realistic - not everyone will thoroughly care about placing them one-by-one).

The only confusing thing here is - again - listening to view's own events. It seems to be not suitable to introduce additional keyword for this, because by this we will force to use nesting and break old syntax listen: { "event": "handler" }. And so we came to Thorax's approach, with one important difference - no mixing DOM and application events in single property. We have only application events in listen - no surprises, no implicit substitutions.

Of course, then someone could write something like this:

listen:
  mediator: 
    "event1": "handler1"
  "event2 mediator": "handler2"

But would it be really problem of Chaplin?


Well... it's a huge comment. I hope the sense is still clear.

akre54 commented 10 years ago

I do like the listen: -> target: -> 'event': 'handler' syntax where target refers to this[target], except in the mediator case where it refers to the global mediator.

jeron-diovis commented 10 years ago

So this question is about "whether to introduce additional separator". I don't see another solution for declarative syntax.

Ok, I thought about it some more, and there is another idea: multiple events can be allowed inside groups. As in this case target will be already separated, we can still split events only by spaces.

But in this case will be no ability to listen multiple events from view itself.

akre54 commented 10 years ago

But in this case will be no ability to listen multiple events from view itself.

You're saying something like this, right?

class Button extends View
  listen:
    model:
      'change': 'render'
    'update eventa eventb': 'update'

I don't see a reason why triggering eventa or eventb on a view instance wouldn't also work here. It'll map pretty cleanly to Backbone's this.on.

jeron-diovis commented 10 years ago

I don't see a reason why triggering eventa or eventb on a view instance wouldn't also work here.

It is only if we will deny to specify targets "inline", that is, refuse old syntax "event mediator": "handler". Such solution will break backward compatibility, isn't it?

akre54 commented 10 years ago

Personally I don't care about backwards compatibility for this case (and the Chaplin devs have proven they're willing to break bc for any good reason).

It just means that you won't be able to use mediator, collection or model as your event names. Not a huge deal IMO.

jeron-diovis commented 10 years ago

It just means that you won't be able to use mediator, collection or model as your event names.

Very well, if bc is not so big deal, then I like this approach too. So in this case we have grouping, and multiple events per target "by design".

And also idea about multiple targets for same events becomes impossible, as targets are strictly separated. Thinking about it - perhaps, it's even better, because in real world such need is very rare, and in such case, as usual, "you should probably rethink you architecture".

So, is there some another objections? @molily ?

paulmillr commented 10 years ago

It just means that you won't be able to use mediator, collection or model as your event names.

we can't break backwards compat for 1.x

jeron-diovis commented 10 years ago

Then what about mixed approach from source comment?

akre54 commented 10 years ago

because in real world such need is very rare, and in such case, as usual, "you should probably rethink you architecture".

Definitely not. I use space-separated events all the time (and Chaplin's CollectionView uses them to listen on add/remove/sort events).

On Tue, May 27, 2014 at 3:59 PM, Jeron Diovis notifications@github.comwrote:

It just means that you won't be able to use mediator, collection or model as your event names.

Very well, if bc is not so big deal, then I like this approach too. So in this case we have grouping, and multiple events per target "by design".

And also idea about multiple targets for same events becomes impossible, as targets are strictly separated. Thinking about it - perhaps, it's even better, because in real world such need is very rare, and in such case, as usual, "you should probably rethink you architecture".

So, is there some another objections? @molily https://github.com/molily?

— Reply to this email directly or view it on GitHubhttps://github.com/chaplinjs/chaplin/issues/800#issuecomment-44327147 .

molily commented 10 years ago

we should probably come up with a better syntax instead of adding more logic to the existing syntax

But what another solution could be here at all?

Backbone’s object approach makes sense for simple key-value combinations. There are (at least) two ways to deal with more complex combinations:

1 nest objects

'dimension1':
  'dimension2':
    'dimension3'

2 use flat lists

['dimension1', 'dimension2', 'dimension3']

Example for version 2:

listen: [
  ['event1 event2 eventN', 'target1 target2 targetN', 'handler1 handler2 handlerN']
  …
]

(Alternative order: targets, events, handlers)

akre54 commented 10 years ago

I like the nested object syntax the best, because it implies a hierarchy of properties on the view. The array syntax is confusing IMO.