Collection.create() with wait: true triggers Collection.parse() #2824

Closed
thrar opened this Issue Oct 22, 2013 · 8 comments

Comments

Projects
None yet
3 participants
@thrar

thrar commented Oct 22, 2013

Since version 1.1, calling

myCollection.create( {} , { wait: true } );

triggers a call to myCollection.parse() once the server's response is returned. According to the documentation, it looks like parse should only be called in reaction to fetch().

Additionally, the parameters to this parse() call seem to be different: When calling parse() via fetch(), the first parameter contains the raw data of the response, i.e. an array of JS objects. When parse() is triggered from create(), the first parameter contains the newly created model.

As a workaround, it's possible to set parse: false when calling create(), this will prevent the call to parse().

@j03w

This comment has been minimized.

Show comment Hide comment
@j03w

j03w Oct 22, 2013

Contributor

collection.create shouldn't call parse unless you specifically set parse: true when you call the function. code here. As you see, collection.create will call collection.add which simply call collection.set. If you don't pass in parse: true then this should not get called

model.parse get called though since collection.create calls model.save

Am I missing something?

Contributor

j03w commented Oct 22, 2013

collection.create shouldn't call parse unless you specifically set parse: true when you call the function. code here. As you see, collection.create will call collection.add which simply call collection.set. If you don't pass in parse: true then this should not get called

model.parse get called though since collection.create calls model.save

Am I missing something?

@thrar

This comment has been minimized.

Show comment Hide comment
@thrar

thrar Oct 22, 2013

The call to parse() only happens when you set wait: true.

thrar commented Oct 22, 2013

The call to parse() only happens when you set wait: true.

@thrar

This comment has been minimized.

Show comment Hide comment
@thrar

thrar Oct 22, 2013

I understand what you're trying to point out, but unfortunately the actual behavior seems to be different:

class MyCollection extends Backbone.Collection
  url: "some/url/here" # this has to be a valid URL with a readable server response, since we're waiting for it

  parse: -> console.log "called parse"

new MyCollection().create {}, wait: true

This results in "called parse" printed to the log.
Note that without wait: true, parse isn't called.

You can use the Coffeescript compiler from http://coffeescript.org/ to compile the above to JS.

thrar commented Oct 22, 2013

I understand what you're trying to point out, but unfortunately the actual behavior seems to be different:

class MyCollection extends Backbone.Collection
  url: "some/url/here" # this has to be a valid URL with a readable server response, since we're waiting for it

  parse: -> console.log "called parse"

new MyCollection().create {}, wait: true

This results in "called parse" printed to the log.
Note that without wait: true, parse isn't called.

You can use the Coffeescript compiler from http://coffeescript.org/ to compile the above to JS.

@j03w

This comment has been minimized.

Show comment Hide comment
@j03w

j03w Oct 22, 2013

Contributor

You're right, options.parse is being set to true in model.save apparently. Although, I think it has been working like that since 0.9.10 from this commit

Contributor

j03w commented Oct 22, 2013

You're right, options.parse is being set to true in model.save apparently. Although, I think it has been working like that since 0.9.10 from this commit

@thrar

This comment has been minimized.

Show comment Hide comment
@thrar

thrar Oct 22, 2013

Not sure if I'm reading the code correctly, but doesn't that refer to Model.save() calling Model.parse()?
On model level that's correct as far as I know because there we actually have raw model data that we can parse.

On collection level, when creating a model, using Collection.create(), all we have is raw data for that one model. Collection.parse() expects raw collection data, i.e. typically an array. When creating a model, we don't have any such data.

Calling Collection.parse() is new behavior. I've been using Backbone 1.0 for months and it was never called during create(). As mentioned before, it seems that Collection.parse() in this situation is actually called with the model as the first parameter, not with any kind of raw data as it would normally be.

thrar commented Oct 22, 2013

Not sure if I'm reading the code correctly, but doesn't that refer to Model.save() calling Model.parse()?
On model level that's correct as far as I know because there we actually have raw model data that we can parse.

On collection level, when creating a model, using Collection.create(), all we have is raw data for that one model. Collection.parse() expects raw collection data, i.e. typically an array. When creating a model, we don't have any such data.

Calling Collection.parse() is new behavior. I've been using Backbone 1.0 for months and it was never called during create(). As mentioned before, it seems that Collection.parse() in this situation is actually called with the model as the first parameter, not with any kind of raw data as it would normally be.

@j03w

This comment has been minimized.

Show comment Hide comment
@j03w

j03w Oct 22, 2013

Contributor

You are right about it is being updated on 1.1… commit

If you have wait: true then the model is being added to collection in success callback. That success callback is being called from inside model.save callback. At that point options.parse will have been set to true if it isn't set to anything before.

Doesn't seem like an intentional behaviour to me.

I think it is more intuitive to have options preserved in collection.create scope right?

options.success = function(model, resp)

in Collection.prototype.create should fix the issue I think

Contributor

j03w commented Oct 22, 2013

You are right about it is being updated on 1.1… commit

If you have wait: true then the model is being added to collection in success callback. That success callback is being called from inside model.save callback. At that point options.parse will have been set to true if it isn't set to anything before.

Doesn't seem like an intentional behaviour to me.

I think it is more intuitive to have options preserved in collection.create scope right?

options.success = function(model, resp)

in Collection.prototype.create should fix the issue I think

@thrar

This comment has been minimized.

Show comment Hide comment
@thrar

thrar Oct 23, 2013

Makes sense to me. Any actions taken on collection level as a consequence of Collection.create() (i.e. add() and the success callback) should receive the same options as the call to create() itself.

Your proposed change fixes the issue for me.

thrar commented Oct 23, 2013

Makes sense to me. Any actions taken on collection level as a consequence of Collection.create() (i.e. add() and the success callback) should receive the same options as the call to create() itself.

Your proposed change fixes the issue for me.

@jashkenas

This comment has been minimized.

Show comment Hide comment
@jashkenas

jashkenas Nov 6, 2013

Owner
options.success = function(model, resp)

Ok -- makes sense. Anyone else have an opinion on this proposed fix?

Owner

jashkenas commented Nov 6, 2013

options.success = function(model, resp)

Ok -- makes sense. Anyone else have an opinion on this proposed fix?

@jashkenas jashkenas closed this in 9280d85 Nov 9, 2013

@thrar thrar referenced this issue in backbone-paginator/backbone-pageable Feb 1, 2014

Closed

`totalRecords` must be a finite integer #146

jridgewell added a commit to jridgewell/backbone that referenced this issue May 30, 2015

jridgewell added a commit to jridgewell/backbone that referenced this issue May 30, 2015

jridgewell added a commit to jridgewell/backbone that referenced this issue May 30, 2015

jridgewell added a commit to jridgewell/backbone that referenced this issue May 31, 2015

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