Stubbing out Flux stuff. Refactored Location component

chadxz commented 9 years ago

This looks good @syjulian. A couple of points, but I'm not gonna block the merge on them... they can be cleaned up later as needed.

object-assign package is not being used. Instead you are using the ES6 Object.assign(). Since we are using babel to transpile, this should just work as long as babel-core is being pulled into the loader. We just need to remove the object-assign package from the package.json
@tessellator and i had discussed using the alt flux implementation for this project, but since you have this setup already i say we just use this instead.
there are a couple spots with unnecessary empty lines or missing spaces but those are just minor nitpicky things. I'm not able to comment on lines of code right now since I'm on my phone or i would have pointed then out.

I'm gonna add you as a contributor Julian, so feel free to merge. Otherwise I'll do it when I'm back from vacation on Thursday and have had a chance to test it

chadxz commented 9 years ago

One last thing. It's not a good idea to use internal implementation details of another library like you are doing with keymirror. Use this instead: https://www.npmjs.com/package/keymirror

See https://github.com/facebook/react/issues/1639

syjulian commented 9 years ago

How is alt flux different from normal flux?

The current store has getters but no setters. I'm assuming async calls go into stores instead of actions to make stores the bigger logical part of the app. Also, I apologize for the messy use of ES6 modules. I mostly based my changes on facebook's flux todomvc example.

chadxz commented 9 years ago

alt is meant to reduce the amount of boilerplate involved with implementing flux. That's pretty much it.

I don't think there is a canonical right way to do async calls in flux. Doing it in the stores probably has some drawbacks, but we can address anything that comes up at that time.

chadxz commented 9 years ago

Oh, alt also supports universal flux, so you can do server rendering of your app.

syjulian commented 9 years ago

I just checked out the github page for it. It looks like a good idea to use alt. I'm going to update my pull request to using alt after work.

codeforhuntsville / Frontier

Stubbing out Flux stuff. Refactored Location component #56