backbone

backbone copied to clipboard

@jridgewell it's done. Is it okay that I made this revert commit? I mean if you do squash merge at the end it wont matter.

Oct 12 '16 20:10 unclechu

No, we can squash merge it. Still need the link to #3842. And someone else should look over the wording. @captbaritone? 😉

Oct 12 '16 21:10 jridgewell

@jridgewell

Still need the link to #3842

Do you mean I should add #3842 to the commit message?

Oct 12 '16 21:10 unclechu

To the comment, not the commit message.

Oct 12 '16 21:10 jridgewell

@jrburke how about now?

Oct 12 '16 22:10 unclechu

Rather than "makes sense" can you explain the behavior that it actually avoids?

Oct 12 '16 22:10 captbaritone

@captbaritone I added line about it helps to avoid conflicts with Object.prototype properties.

Oct 12 '16 22:10 unclechu

I'm not familiar with this issue, and those who are reading this comment probably won't be either. In what way does it "make sense"? Does it preserve attributes which have been explicitly set to undefined? Does it overwrite them?

Oct 12 '16 22:10 captbaritone

It has two steps.

1. We have to create an object that contains all properties, attrs taking precedence over defaults. We clobber prototype inheritance here on purpose.
   • _.extend({}, defaults, attrs)
2. Anything in Step 1 that is undefined should be overridden with its defaults value. Since we clobbered prototype inheritance, we know for certain that if something is undefined, it was set specifically to undefined in attrs. (Or, if it was set in defaults, it will still be undefined after this step)
   • _.defaults(_, defaults)

Oct 12 '16 23:10 jridgewell

Superseded by #4267.

Jul 20 '23 21:07 jgonggrijp