Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Differences in object construction #51

Closed
justin808 opened this issue Sep 7, 2014 · 7 comments
Closed

Differences in object construction #51

justin808 opened this issue Sep 7, 2014 · 7 comments

Comments

@justin808
Copy link
Contributor

Any information on why the difference between:

var TodoStore = merge(EventEmitter.prototype, {

and

var AppDispatcher = copyProperties(new Dispatcher(), {

Maybe we don't want to call the constructor of EventEmitter? It doesn't seem to do much either way.

It would be nice to simplify the setup.

@justinwoo
Copy link
Contributor

I think the only documentation is in the source code:

https://github.com/facebook/react/blob/8cb2812cff6a6cb54bb673254f1170281ce0790a/src/vendor/core/merge.js#L22

https://github.com/facebook/react/blob/8cb2812cff6a6cb54bb673254f1170281ce0790a/src/vendor/core/copyProperties.js#L20

tl;dr merge returns a fresh object merge of the operands, copyProperties mutates the hell out of the first operand and returns it.

@justin808
Copy link
Contributor Author

That makes sense why copyProperties must be called with "new".

Any reason TodoStore could not be created the same way? I dug into the code a bit and it seems like merge has some specific semantics regarding "merging" objects like arrays, whereas with copyProperties, the second object will overwrite the propery in the first object.

Anyway, this differece would be worth a comment in the code, or if there is no difference, then one technique should be picked.

@zpao
Copy link
Contributor

zpao commented Sep 7, 2014

copyProperties has mostly been replaced by merge in new code, but there is still a decent amount of older code that hasn't been updated because it works. merge now being really just an alias for Object.assign means we may even deprecate those callers and just call it directly. Either way, yes one should be picked but the example app was written months ago and hasn't been updated much.

@justin808
Copy link
Contributor Author

@fisherwebdev @zpao A related question is this declaration in ThreadSection.react.js, line 21

function getStateFromStores() {
  return {
    threads: ThreadStore.getAllChrono(),
    currentThreadID: ThreadStore.getCurrentID(),
    unreadCount: UnreadThreadStore.getCount()
  };
}

I'm wondering if there was a reason that it was not declared like:

var getStateFromStores = function() {
....

@zpao
Copy link
Contributor

zpao commented Sep 30, 2014

Because we wanted to. In general we prefer that style and it does have semantic differences in JS. Here's a deeper dive into the differences: http://javascriptweblog.wordpress.com/2010/07/06/function-declarations-vs-function-expressions/. There are times where your latter example is appropriate.

@justin808
Copy link
Contributor Author

@zpao Very helpful article for a reforming CoffeeScripter-Rails-I-don't-use-modules-nor-Javascript developer! Do you have a link to any style guidelines, especially one that has some "whys"?

@zpao
Copy link
Contributor

zpao commented Sep 30, 2014

https://github.com/airbnb/javascript is quite good from what I remember but it's been a while since I looked closely.

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

4 participants