Fixing 'maximum call stack size exceeded' errors

[nuragic]

Hi,

I made some changes that solves the following issues:
#180
#210
#224
#233

Basically, I replaced underscore with lodash because it provides a single method called isPlainObject(), that is very helpful in this context.

What do you think?

Thanks!

[chiefGui]

I think your solution is cool, but changing the whole library? Audacious.

Anyway, all the tests passed?

[blikblum]

-1 to add dependency on lodash

[nuragic]

Thank you @chiefGui ! Yep, all the tests passed without change any line of it... Audacious? Why? Lodash is basically the same thing as underscore, but is better... :)

@blikblum Please, can you argue your rejection? Note that I've not just added one dependency more, but I replaced it.

Detailed info about the differences between lodash and underscore here:
http://stackoverflow.com/questions/13789618/differences-between-lodash-and-underscore

[blikblum]

All projects that depends on backbone.validation will be forced to use lodash.
Now, the dev is free to choose between underscore and lodash.

[chiefGui]

@blikblum is right, all projects dependent of Backbone.Validation will also be dependent of lodash for the front-end, which means another library — very similar to underscore — to be loaded and this way increasing the final weight of the files distributed to our clients.

Lodash is basically the same thing as underscore, but is better... :)

Basically, yes, but say that it is better I think it's too much. I mean, I know it has its strong points, but it also has some weak points which Underscore don't.

Anyway, I need more opinions to merge this. But people, please, argue about.

[nuragic]

Mmmh ok, so, the alternative is to borrow _.isPlainObject(). Sounds better? :) Underscore allows you to extend it with your own custom functions through the _.mixin() method...

[blikblum]

@nuragic borrow _.isPlainObject() or improve flatten.
Can you post an working example that crashes, so i can take a look?

[blikblum]

I used $.isPlainObject and worked fine. Tested against a jquery selector, a model, a date

[nuragic]

@blikblum I've noticed the same thing... Jquery has the isPlainObject method so... I will revert the lodash switch and commit that change very soon! 👍

[chiefGui]

Awesome! Waiting for you! 👍

[nuragic]

After trying to add jquery as dependency I realized that it was a terrible mess because it requires a document object to work properly in NodeJS. I finally replaced the isPlainObject() method for:

!!val && typeof val === 'object' && val.constructor === Object

(kindly borrowed from https://github.com/jonschlinkert/is-plain-object/blob/master/index.js)

Seems that works like a charm! 🎉 😄

[blikblum]

Looks good to me

[chiefGui]

Awesome, @nuragic. Could you pull it as 0.9.2?

[nuragic]

@chiefGui Yep, no problem. I have to modify only the package.json or even the README.md?

[chiefGui]

• package.json
• bower.json
• README.md

and also create a new tag on your branch then squash/pull it.

[nuragic]

bower.json ❓ 😄 I can't see that... It would be nice to have anyway!

[chiefGui]

Wow, man, my bad. I thought I accepted #251, but I don't. Nevermind, I'm merging your PR.

Thank you man! 👍 💯

[nuragic]

No worries! Thanks to you guys for the awesome work!