Collection parse

[jridgewell]

Fixes #3636, #2824.

Reverts #1407, so breaking change.

Why? To mirror Model. Model#set doesn’t parse for you, only instantiation, #fetch, and #save parse.

• We do it on instantiation because we have to have an instance to call #parse (you can't do it before hand because you don't have an instance)
• We do it on #fetch and #save because they return data from the server (the reason we have a #parse!)

If you need to parse your Collection's data before #set, #reset, or #add, do it beforehand (you already have an instance):

• collection.set(collection.parse(models));
• collection.reset(collection.parse(models));
• collection.add(collection.parse(models));

Or, you can parse on instantiation if you don't have an instance, just like Model:

• new Collection(models, { parse: true });

And of course, Collection#fetch parses at the collection level like it always has.

[akre54]

_.defaults?

[jridgewell]

No, because they can specify { parse: false } if they want to. And, we don't want to mutate, their options object (that, and _.defaults(null, {parse: true}) will return null).

[akre54]

cool, thanks

[jashkenas]

Mind explaining the behavior of this — I don't want to guess incorrectly.

Collections should be parsing whenever data comes down from the server ... but if you set data directly on the client side, it shouldn't parse by default. Right?

[jridgewell]

Collections should be parsing whenever data comes down from the server ... but if you set data directly on the client side, it shouldn't parse by default. Right?

Yup.

[jridgewell]

Wait, are we going to releasing this with v1.2.1? I thought the consensus was this would be released in v2?

[jashkenas]

This looks like a bugfix, doesn't it? We shouldn't be parsing your models with data set on the client-side.

[jridgewell]

This looks like a bugfix, doesn't it?

Yah, but it's fixing it by changing core functionality. I think it'd be better to label it as a known bug (with an easy patch) that's fixed in the next major.

[jridgewell]

We could actually pull the data instanceof Model check into v1.2.1, and leave this for v2:

# L795
      options = _.defaults({}, options, setOptions);
-     if (options.parse) models = this.parse(models, options);
+     if (options.parse && !this._isModel(models)) models = this.parse(models, options);
      var singular = !_.isArray(models);

[jashkenas]

Who do you imagine (What use case do you imagine —) that we'd be breaking by simply including this fix now?

[jridgewell]

Well, my job's codebase uses it extensively (not that we'll be updating anytime soon...).

I understand the rationale that people want to parse their collection when setting (I just think it's wrong), so I do think this will be a pretty major change for users. Espescially in a patch release.

[jashkenas]

Okay, fine ... but it makes me sad that parsing-while-set'ing landed in first place.

[jridgewell]

daf6892#diff-0d56d0d310de7ff18b3cef9c2f8f75dcR804 😛

Don't worry, I'll open another PR so we can remove it in v2.