Separate fetch/save api into plugin or external module

[taburetkin]

It would be great if in the second version of the library the model and collection got rid of the built-in backend API. In my opinion, the backend API should be an external module, although it is possible to provide a base implementation. Today, you constantly have to look for workarounds if you want to destroy a model that does not imply synchronization, but has the specified Id. And in general, if we are talking about the model-view concept, in my opinion it would be better if the library will be not aware of the backend Besides, this separation will simplify the process of getting rid of jQuery if such an idea arises.

[jgonggrijp]

Hey @taburetkin, thanks for sharing your thoughts!

It would be great if in the second version of the library the model and collection got rid of the built-in backend API. In my opinion, the backend API should be an external module, although it is possible to provide a base implementation.

If you mean the CRUD methods of Backbone.Collection and Backbone.Model, then I think it makes a ton of sense to have those on board by default, as base implementations as you put it. I don't think there are many serious stateful web applications that never talk to a backend. These methods already abstract over the means of synchronization thanks to Backbone.sync and Backbone.ajax. However, I somewhat agree that Backbone.ajax need not default to jQuery.ajax and that this could be deferred to a plugin instead, especially now that fetch is built into most browsers.

Today, you constantly have to look for workarounds if you want to destroy a model that does not imply synchronization, but has the specified Id.

I don't really see your point here. What's the difference from collection.remove(id)? If there is actually a difference, why doesn't overriding Backbone.sync suffice?

And in general, if we are talking about the model-view concept, in my opinion it would be better if the library will be not aware of the backend

What do you mean by that? I don't think Backbone in its current form is aware of anyone's particular backend.

Besides, this separation will simplify the process of getting rid of jQuery if such an idea arises.

Honestly, as far as synchronization is concerned, this is very easy to achieve now already because you only need to override Backbone.ajax. I don't see why "getting rid of jQuery" would ever be a goal in itself, though. A lot of people seem to hate jQuery nowadays, for no apparent reason other than the library being old. As I see it, old is gold.

[taburetkin]

Hey @taburetkin, thanks for sharing your thoughts!

It would be great if in the second version of the library the model and collection got rid of the built-in backend API. In my opinion, the backend API should be an external module, although it is possible to provide a base implementation.

If you mean the CRUD methods of Backbone.Collection and Backbone.Model, then I think it makes a ton of sense to have those on board by default, as base implementations as you put it. I don't think there are many serious stateful web applications that never talk to a backend. These methods already abstract over the means of synchronization thanks to Backbone.sync and Backbone.ajax. However, I somewhat agree that Backbone.ajax need not default to jQuery.ajax and that this could be deferred to a plugin instead, especially now that fetch is built into most browsers.

Yes, i mean crud. In my opinion there should be something else for crud operation, some kind of store with base implementation

Model and Collection should not have any crud related properties and methods like url, save and fetch, the destroy method should not perform DELETE. all this logic should be implemented in entity store.

I do not mean that there should not be CRUD at all i mean that this sync logic should be extracted to some EntityStore and there should be one built-in by default

Today, you constantly have to look for workarounds if you want to destroy a model that does not imply synchronization, but has the specified Id.

I don't really see your point here. What's the difference from collection.remove(id)? If there is actually a difference, why doesn't overriding Backbone.sync suffice?

Just try this

let menuItemModel = new Model({ id: 'menuItem1', label: 'bla-bla-bla' });
menuItemModel.destroy();

link to codepen: https://codepen.io/dimatabu/pen/MWOqVGY

And in general, if we are talking about the model-view concept, in my opinion it would be better if the library will be not aware of the backend

What do you mean by that? I don't think Backbone in its current form is aware of anyone's particular backend.

oh, thats because of my pure eng, i was telling not about particular backend but about crud operations. i think we can skip this.

Besides, this separation will simplify the process of getting rid of jQuery if such an idea arises.

Honestly, as far as synchronization is concerned, this is very easy to achieve now already because you only need to override Backbone.ajax. I don't see why "getting rid of jQuery" would ever be a goal in itself, though. A lot of people seem to hate jQuery nowadays, for no apparent reason other than the library being old. As I see it, old is gold.

actualy any thing is very easy to achieve. and the talk here not about jquery itself. just imagine that you want to use backbone in application where you communicate with the backend not through http and there is no need in jquery in application. still possible today, but its just not very comfortable.

dom manipulation is subject of user choice (there is a good try in marionette about this) backend data manipulation is subject of user choice too

perfect minimum application setup should looks like

<head>
  <script src="cdn://backbone" />
</head>

okay, i can accept next one as semi-perfect

<head>
  <script src="cdn://underscore" />
  <script src="cdn://backbone" />
</head>

And I don't insist, I just share my thoughts

[jgonggrijp]

Hold on, I think we have a misunderstanding about Model#destroy. As far as I know, the very purpose of this method is to send a DELETE request to the backend. If you do not need to send a DELETE request to the backend, there is no point in calling destroy in the first place. Why would you ever need to run the code in your example?

[taburetkin]

no.

    // Destroy this model on the server if it was already persisted.
    // Optimistically removes the model from its collection, if it has one.
    // If `wait: true` is passed, waits for the server to respond before removal.
    destroy: function(options) {
      options = options ? _.clone(options) : {};
      var model = this;
      var success = options.success;
      var wait = options.wait;

      var destroy = function() {
        model.stopListening();
        model.trigger('destroy', model, model.collection, options);
      };

      options.success = function(resp) {
        if (wait) destroy();
        if (success) success.call(options.context, model, resp, options);
        if (!model.isNew()) model.trigger('sync', model, resp, options);
      };

      var xhr = false;
      if (this.isNew()) {
        _.defer(options.success);
      } else {
        wrapError(this, options);
        xhr = this.sync('delete', this, options);
      }
      if (!wait) destroy();
      return xhr;
    },

the main thing of destroy is

      var destroy = function() {
        model.stopListening();
        model.trigger('destroy', model, model.collection, options);
      };

1. clear listeners
2. trigger the destroy hook which effectively removes model from collection and all that things.

[jgonggrijp]

You quote the source code of the destroy method, which is 20 sloc, and then highlight 3 of those lines and declare that those are "the main thing". I hope you can agree that this is a rather subjective way of choosing "the main thing" about a method. ;-)

Let me suggest a more objective way of identifying the purpose of the destroy method. The first sentence of the doc comment that you quoted, as well as the first three sentences of the documentation on the website, clearly convey that the method is about deleting the model from the underlying datastore:

Destroys the model on the server by delegating an HTTP DELETE request to Backbone.sync. Returns a jqXHR object, or false if the model isNew. Accepts success and error callbacks in the options hash, which will be passed (model, response, options).

If cleaning up is all you need to do, you can do model.stopListening().trigger('destroy', model). It's a oneliner. If you do this a lot and you want to remove the repetition, you can also add your own method to Backbone.Model.prototype or a subclass. You could call it remove.

Is this type of cleaning up really helpful, by the way? I mean, does it actually help in garbage collection, like remove does for views? In that case, I wouldn't be against adding a remove method to Backbone.Model.

[taburetkin]

You quote the source code of the destroy method, which is 20 sloc, and then highlight 3 of those lines and declare that those are "the main thing". I hope you can agree that this is a rather subjective way of choosing "the main thing" about a method. ;-)

Let me suggest a more objective way of identifying the purpose of the destroy method. The first sentence of the doc comment that you quoted, as well as the first three sentences of the documentation on the website, clearly convey that the method is about deleting the model from the underlying datastore:

Destroys the model on the server by delegating an HTTP DELETE request to Backbone.sync. Returns a jqXHR object, or false if the model isNew. Accepts success and error callbacks in the options hash, which will be passed (model, response, options).

If cleaning up is all you need to do, you can do model.stopListening().trigger('destroy', model). It's a oneliner. If you do this a lot and you want to remove the repetition, you can also add your own method to Backbone.Model.prototype or a subclass. You could call it remove.

Is this type of cleaning up really helpful, by the way? I mean, does it actually help in garbage collection, like remove does for views? In that case, I wouldn't be against adding a remove method to Backbone.Model.

I must agree that backbone view has remove method in a view and implementation of this method looks like some kind of dispose pattern design. I am not agree with the remove name of this method - destroy will be much better.

but lets discuss the main thing of model's destroy. Imagine some abstract method:

function someMethod() {
   let action = () => {
     doThis();
     doThat();
   }

   if (conditionA) {
      callAnotherMethod();
      action();
   } else {
      doAsyncJob().then(() => action());
   }

}

in this method action will be definitely executed, and all other things will be executed only by condition i think that the main thing of the method is thing which will be definitely executed, others are optional the local destroy is such definitely executed thing and sending request to the backend is optional.

I belive that model.destroy and view.remove are examples of dispose pattern implementation, even if it was not originally intended that way.

[jgonggrijp]

By that reasoning, the local destroy is still not "the main thing", because it will not definitely be executed. Consider the case where model.isNew() is false, options.wait is true and the backend request fails. In fact, the destroy method as a whole has no single part that will always be executed, except for the overarching decision making logic.

I can agree that Model#destroy includes cleanup behavior, even if it is not its sole purpose. I can also agree that there is currently no other Model method that provides cleanup behavior, so I understand how you may have arrived at using destroy for cleanup purposes. Still, I think that backend deletion is very clearly also a purpose of destroy and that most users expect this behavior.

Now, I can think of three ways we could take this:

1. Reduce Model#destroy to a method that does cleanup only, as you're suggesting, and move backend deletion to a new, separate facility. This would be a very disruptive change and in my opinion, "destroy" implies more than just cleaning up.
2. Keep Model#destroy as is, but add a new Model#remove which does only the cleanup. I think remove is a good name because it is consistent both with Collection#remove and View#remove, but the new method could potentially also get a different name.
3. Do nothing. This would be defensible in my opinion, if we can determine that the cleanup by itself is not strictly necessary.

[taburetkin]

I think that any decision will be a decision. i am here only to point some things. and i suppose i have a success in delivering my thoughts about that, so we can stop at the moment.

ps. about model's local destroy: the case you described is only valid for a model which explicitly must be synchronized. and ofcourse if backend request fails then destroy should not happens.

the main thing of destroy (not in backbone but in general) is that after success destroy any use of instance must throw.

[ogonkov]

CRUD is already abstracted from collection/models, via #sync alias. If you don't want any network activity even possible, you can instantiate your own base models with sync method mocked.

class BaseModel extends Backbone.Model {
  sync() {
    // always resolve anytning
  }
}

[Rayraz]

I guess to some extent this is a question of what the responsibility of Backbone.Model should be:

• Is it supposed to be merely a data structure?
• Is it's responsibility to facilitate some reactivity to changes to local data?
• Is it's responsibility to facilitate communication with a persistence layer?
• Is it's responsibility to be kind of a two-way binding with a remote copy of it's data?

I would say 'local data' in this context essentially comes down to application state, while 'remote data' refers to some storage layer. Whether this is a remote server or localStorage or whatever.

It seems to me what @taburetkin might be aiming for is to remove the responsibility of communicating with a persistence layer from the model entirely?

If the model is meant to be a sort of two-way binding with a remote copy of it's data, then it makes sense for communication with the persistence layer (even if indirectly through some layer of abstraction like "feel free to implement your own sync method") to be an inherent responsibility of the model.

When it comes to event listeners and such, there is a conceptual difference between 'the value of this attribute of the model changed' and 'a copy of the data contained in this model has just been persisted to/deleted from a storage layer'.

With regards to remove/destroy. I would say there is at least potentially a difference between things like cleaning up event listeners, or telling a persistence layer to store or delete some data. Splitting this off into two separate methods might not necessarily be a bad idea, but it really kind of depends on what the model's responsibilities are.

Edit: We could actually treat some of these responsibilities as optional, and if all persistence is passed through sync (is it currently?) then @ogonkov's suggestion of turning sync into a no-op might be good enough to nuke any interactions with a persistence layer.

[jgonggrijp]

@Rayraz

Edit: We could actually treat some of these responsibilities as optional, and if all persistence is passed through sync (is it currently?)

Yes, it is.

then @ogonkov's suggestion of turning sync into a no-op might be good enough to nuke any interactions with a persistence layer.

Yes, this is something that is currently already possible.

[Rayraz]

I found some time to re-familiarize myself a little more and indeed, what @ogonkov suggested seems to be all that's needed to remove the responsibility of talking to a persistence layer entirely.

I can see there might maybe be one detail to possibly clean up. The method name 'destroy' might imply it's a counterpart to the constructor method. You could argue tearing down the class instance and removing some data from some storage layer should be separate responsibilities. Currently, calling Model.destroy() means both "delete this data" and "tear down this class instance".

Of course it make sense to tear down the class instance once it's data has been deleted, but maybe it sometimes makes sense t tear it down without deleting the data from your storage layer?

Currently the only cleanup that's happening in a standard Backbone model is to call Model.stopListening(). So tearing down the model would come down to simply calling myModelInstance.stopListening(). However, if you extend the standard Model you might want to add some additional cleanup?

If I'm not mistaken, currently you can only do so by extending destroy and adding your own cleanup before calling a the original Model.destroy method:

var MyFancyModel = Backbone.Model.extend({
    destroy: function() {

        /* ... some additional cleanup code ... */
 
        Backbone.Model.prototype.destroy.apply(this, arguments);
    }
})

But this doesn't allow you to wait until after the data has been deleted. If you need to wait until after the data has been deleted, you'd need to listen for the destroy event and then do your cleanup, but then your cleanup has become external to the model itself.

Perhaps something as simple as spinning off a teardown method would solve this? It would clearly separate the two responsibilities, it'd allow you to execute your additional cleanup behavior after data has been deleted yet before the destroy event is triggered and it'd allow you to do all your cleanup at once with one function call, without deleting any data:

teardown: function() {
    this.stopListening();
},
destroy: function(options) {
      options = options ? _.clone(options) : {};
      var model = this;
      var success = options.success;
      var wait = options.wait;

      var destroy = function() {
        model.teardown();
        model.trigger('destroy', model, model.collection, options);
      };

      options.success = function(resp) {
        if (wait) destroy();
        if (success) success.call(options.context, model, resp, options);
        if (!model.isNew()) model.trigger('sync', model, resp, options);
      };

      var xhr = false;
      if (this.isNew()) {
        _.defer(options.success);
      } else {
        wrapError(this, options);
        xhr = this.sync('delete', this, options);
      }
      if (!wait) destroy();
      return xhr;
},

The only thing I'm not really sure about is if this is something you'd actually ever need or if I'm just overthinking...

[jgonggrijp]

Perhaps something as simple as spinning off a teardown method would solve this?

That's basically what I've been proposing above as "option 2", except that I called it remove instead of teardown and that I think this method should also imply removal from all collections that the model might be a member of.

The only thing I'm not really sure about is if this is something you'd actually ever need

That's exactly what I'm unsure about, too.

[Rayraz]

Hmm... yeah looking at it from that perspective it's kinda similar to what View.remove does.

The option 2 style remove method doesn't really do anything that would prevent you from re-using the model again and neither does View.remove keep you from re-using the view. It would simply make Model clean up after itself the way View does. From this perspective the name remove is actually better than teardown.

Meanwhile, Model.destroy and View.remove are very different things. The default behavior of destroy is much more permanent..

I also notice View doesn't do anything to notify other application components that it's has been detached from the DOM and by design it doesn't make any assumptions about how it'll be attached to the DOM. Simplicity and flexibility are clearly the main considerations here.

Perhaps splitting off remove from destroy does fit in with that particular philosophy? It doesn't introduce much if any complexity in terms of understanding the code, but it could be argued it anticipates the fact that different people might want to use Model in very different ways.

All in all, it's still probably mostly bike-shedding, but at the same time it does feel nice and clean.

[taburetkin]

sorry for being absent in this discussion in first, i just put it here: https://en.wikipedia.org/wiki/Dispose_pattern and in second, i want to say that current naming convention makes me ill.

.net: (one of my backend languages is .net) dispose pattern is implemented via dispose method (its just for example. not better or worse than teardown). marionette: (my primary fe framework) dispose pattern is implemented via destroy method. backbone: a dispose pattern implemented via remove for a view and destroy for a model.

I feel that method name should be the same for every class within the framework. But backbone view has natural dispose method - remove, which exactly disposes the view and model has some kind of hidden dispose inside of destroy method. So, lets make a remove our dispose method for everything in framework. Wait! But there is a remove method as part of collection api which is definitely not about disposing. So if we call our dispose method remove then we will end with situations when model.remove() and collection.remove() will means different things and if you have to implement dispose pattern for a collection, this method will have to be called differently. - That's why I'm against remove.

btw, destroy - seems to me more natural. model.destroy(), collection.destroy(), view.destroy()

[taburetkin]

CRUD is already abstracted from collection/models, via #sync alias. If you don't want any network activity even possible, you can instantiate your own base models with sync method mocked.

class BaseModel extends Backbone.Model {
  sync() {
    // always resolve anytning
  }
}

yes, this will definitely works.

const BoneMarrowModel = Backbone.Model.extend({
  sync: () => {},
  save: () => throw new Error('synchronizing is not supported'),
  fetch: () => throw new Error('synchronizing is not supported'),
  destroy() {
    this.stopListening();
    this.trigger('destroy', this);
  }
});

for getting just more backboner model :)

[taburetkin]

teardown: function() {
    this.stopListening();
},
destroy: function(options) {
      options = options ? _.clone(options) : {};
      var model = this;
      var success = options.success;
      var wait = options.wait;

      var destroy = function() {
        model.teardown();
        model.trigger('destroy', model, model.collection, options);
      };

      options.success = function(resp) {
        if (wait) destroy();
        if (success) success.call(options.context, model, resp, options);
        if (!model.isNew()) model.trigger('sync', model, resp, options);
      };

      var xhr = false;
      if (this.isNew()) {
        _.defer(options.success);
      } else {
        wrapError(this, options);
        xhr = this.sync('delete', this, options);
      }
      if (!wait) destroy();
      return xhr;
},

actually stopListening is not the only thing. look at local destroy there is model.trigger('destroy', model, model.collection, options); which effectively removes destroyed model from every collection.

let collectionA = new Collection();
let collectionB = new Collection();
let model = new Model();
collactionA.add(model);
collactionB.add(model);
model.destroy();

https://github.com/jashkenas/backbone/blob/0d455df4e15d814ed9c3f24be1ab4ea23c5c630d/backbone.js#L1213