Skip to content
This repository has been archived by the owner on Jul 4, 2021. It is now read-only.

Collection.parse is called 3 times #246

Open
baer opened this issue Aug 18, 2014 · 10 comments
Open

Collection.parse is called 3 times #246

baer opened this issue Aug 18, 2014 · 10 comments

Comments

@baer
Copy link
Contributor

baer commented Aug 18, 2014

I'm seeing parse called 3 times for paginated collections. Can't yet figure out why but it does seem to be related to backbone.paginator. I've verified this by using the following code.

var Invoice = Backbone.Model.extend({
  parse: function (response) {
    console.log(response.id);
  }
});

var Invoices = Backbone.Collection.extend({
  model: Invoice,
});

var PageableInvoices = Backbone.PageableCollection.extend({
  mode: 'client',
  model: Invoice,

  state: {
    pageSize: 15
  }
});

var invoiceLines = [{
  id:1,
  message: "hi"
},
{
  id:2,
  message: "hello"
}];

// Logs 1 time per ID
new Invoices(invoiceLines, {parse: true});

// Logs 3 times per ID
new PageableInvoices(invoiceLines, {parse: true});
@wyuenho
Copy link
Member

wyuenho commented Aug 20, 2014

Is there an actual problem that this causes?

@baer
Copy link
Contributor Author

baer commented Aug 20, 2014

I can only tell you what problems this has caused me - there may be others. In my parse method I transform the data that the api gives me into something that is more useful to my application. Below are a few examples of things that get converted.

Specifically, the issue I ran into was when I was converting booleans. In the code below if response.isNew === 'true' then on the first pass that expression will evaluate to true but on the second pass it will evaluate to false!

parse: function (response) {
  response.quantity = parseFloat(response.quantity);
  response.dateCreated = moment(response.dateCreated);
  response.isNew = response.isNew === "true" ? true : false;
  return response;
}

I have solved this by not mutating the original object but the behavior was surprising and is probably worth correcting. Maybe I can have a look if you can point me in the right direction.

@wyuenho
Copy link
Member

wyuenho commented Aug 20, 2014

First of all, the signature of Collection:parse is to return an array of model attribute hashes. This parse implementation will only work on the vanilla backbone collection when you use parse during singular add ops.

Second of all, the implementation of parse should not contain any side-effects. This is the only sensible thing to do and AFAIK, the convention in the backbone community. A transformation function that contains side-effects often causes surprises like what you discovered above when used inside Backbone.Collection subclasses. There's no guarantee you can make to how often parse gets called (or much of anything really when it comes to subclassing).

Lastly, parse is only called multiple times when under client or infinite mode, for the simple reason that the collection is created by first invoking the superclass' constructor, and then a shadow collection fullCollection is made, and then finally the current page is reset.

@wyuenho wyuenho closed this as completed Aug 20, 2014
@wyuenho wyuenho reopened this Aug 20, 2014
@wyuenho
Copy link
Member

wyuenho commented Aug 20, 2014

BTW, this should be easy fix if you want to send over a PR.

@baer
Copy link
Contributor Author

baer commented Aug 20, 2014

I've changed my parse methods such that they don't have side-effects I sometimes forget to do that when I'm working with an options hash or in this case with server data but yes hard lesson learned I should always make sure there are no side-effects.

I'll see if I can get a PR out this week or next to shore this up. While you're right about side-effects it's quite wasteful to call a method 3 times when you only need to call it once if you can help it :)

Thanks for working on this plugin - I've been using it since the original Osmani version.

@wyuenho
Copy link
Member

wyuenho commented Aug 20, 2014

While you're right about side-effects it's quite wasteful to call a method 3 times when you only need to call it once if you can help it :)

You are right! But I've never had an issue, so... besides, the fix should be pretty easy. Just need to muck with the hashes and models references a bit

@wyuenho
Copy link
Member

wyuenho commented Aug 29, 2014

@baer would you like to send over a PR?

@baer
Copy link
Contributor Author

baer commented Aug 29, 2014

Yes I would like to :) I haven't had time this week. I'm headed across the country to visit family this weekend so I'll see if I can get to it one of these evenings. If you want to get this out quickly I don't have a branch yet so if you know right where it is you won't be stepping on toes if you fix it. Up to you.

@zeet2020
Copy link

hi @wyuenho / @baer

am kind of facing the similar issues, execution happen 3 times or model + 1 times in client mode. issues seems to be a years old. wanted to know whether issue is fixed or what was the solution @baer come across.

@baer
Copy link
Contributor Author

baer commented Oct 28, 2015

@zeet2020 - Sorry, I never got the PR but the "fix" above is still working.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants