jashkenas / backbone

Give your JS App some Backbone with Models, Views, Collections, and Events
http://backbonejs.org
MIT License
28.1k stars 5.39k forks source link

#4185 Model 'change' event is not triggered after Exception. #4186

Closed vinaymor002 closed 1 year ago

vinaymor002 commented 6 years ago

PR for #4185

paulfalgout commented 1 year ago

Yeah.. if this is even an issue worth fixing, it's everywhere in this library.. pretty much a side-effect of the trigger/on/listen code.. if you have model.set({ foo: 1, bar, 2 }); and your on('change:foo' throws, you won't see the change:bar or the change. I think collection.add would be similar with the add event.

Not that Backbone should follow Marionette, but if you throw inside the onBeforeRender event it'll stop the render.. along with any other handler. But again that's largely because we utilize the backbone event code.

Really if this is to be fixed it probably needs to be within triggerEvents and even then it'd be inside those whiles I'd think? And.. I dunno what the perf hit would be for that..

jgonggrijp commented 1 year ago

Good point! I'm now strongly leaning towards slapping a "wontfix" on this, but I'll leave it open for a day or so in case somebody else wants to reply.

GammaGames commented 1 year ago

I agree. Exceptions in the handler should ideally be handled and prevented by the dev, and consistent handler behavior is more useful overall than trying to take into account the edge cases this would have + applying them to the other handler functions in the library.