mulesoft-labs / mule-match

A simple swapping app to use as a Reference app for React applications.
MIT License
6 stars 2 forks source link

List of questions #1

Open yanivtal opened 8 years ago

yanivtal commented 8 years ago

Things are looking real good!

nanovazquez commented 8 years ago

Interesting choice to put reducers, actions, selectors in a folder called domains. We've been including separate folders for actions, reducers, and domain. The idea is that domain includes all the base domain logic that is required elsewhere (including selectors). Is there a reason you prefer your approach? The dependency graph is as follows: domain -> actions domain -> reducers domain -> providers actions -> providers reducers -> providers This creates an acyclic graph with domain at the root. Room for improvement: It might be nice to be able to define models that describe structure, initial state, and optionally additional methods that can be used by reducers.

That is one of the big differences between what we've been doing and this approach: the domains folder gathers all the files that work with the (Redux) global state. This approach was originally inspired by this post, but after some discussions I decided to move the actions/reducers/selectors elsewhere (the domains folder) in a way we can reuse this logic in more than one component. Notice that this approach also follows Redux suggestions for separating presentational logic and state manipulation, aka dumb & smart components respectively (see here).

Organizing the code this way so far seems to make the daily work easier, because you can know find all the files manipulating a particular node in the state tree in the same folder, instead of everywhere. I've asked around and learnt that for React newcomers the learning curve is greatly reduced this way, making them to focus in more architectural decisions like designing the state tree properly or avoid adding extra code where it does not belong to (e.g. do a lot of state-related work inside components).

What's the rationale for having proptypes defined in a separate file?

Just to reduce file length and separate responsibilities per file. This is a common pattern across all the repo and the general idea is have 1 responsability, 1 file (when it makes sense, of course).

Let's incorporate CSS variables from https://github.com/mulesoft/anypoint-styles for colors, typography, and spacing. Integrate Anypoint Core 2.0. See https://github.com/mulesoft/exchange-ui/blob/develop/src/routes/ExchangeRoutes.js

Those are private repos and this one is public. Nevertheless, creating a new sample with those repos is straighforward. I will do that after finishing this one.

Add tests with Enzym and Ava

Working on that (see https://github.com/mulesoft-labs/mule-match/pull/2). I did it in a rush, meaning that I used enzyme + mocha + chai + karma and haven't seen that you suggested Ava, sorry about that. I'll dig into that repo later, looks promising.

Is there a reason you prefer semi-colons? We've been building new UI projects with the standard linter https://github.com/feross/standard

For the node.js projects we are using a combination of jscs and jslint. I'm experimenting with eslint with the airbnb preset and some custom configurations to allow, for instance, experimental ES7 (object rest/spread, async/await, etc.). Works really good so far, and yes, it requires semicolons. We can probably merge the best rules between both linterns, but the general idea should be avoiding using different linterns for backend and frontend.