Redux doc #939
Redux doc #939
Conversation
Gnito
force-pushed
the
redux-doc
branch
4 times, most recently
from
6a447ff
to
527bfed
Compare
There was a problem hiding this comment.
Good job, this is a great addition to the docs! Take the comments as advice and make changes with your best judgement.
One thing in my mind is that this is only documenting one "small" technical part of the big picture. The more interesting high level documentation that we don't have is about how the data flow within the whole FTW app. Redux is just a part of it. I've always seen the need for such a documentation. That can be a separate documentation, or parts of this could be within that documentation.
Some interesting things that could/should be there:
- How pages load their data (
loadData, routing, ducks) - How data from API is normalized and denormalized for the store and for components
- How incomplete data is rendered and should be wrapped with
ensureXfunctions - Where to draw the line between containers and presentational components (i.e. which components own the data)
This document would IMO be really useful at some point in understanding the whole FTW on a high level.
docs/redux.md
Outdated
| We have set up FTW so that pages are aware of Redux, but other components and forms are purely | ||
| presentational components. This makes it easier to customize UI components - you don't need to be | ||
| aware of the complexity related to Redux setup when you just want to touch the behavior of some | ||
| visual component. In those cases, you can just head to `scr/components/` or `src/forms/` folder and |
There was a problem hiding this comment.
I would always use "directory" instead of "folder":
https://stackoverflow.com/questions/5078676/what-is-the-difference-between-a-directory-and-a-folder
docs/redux.md
Outdated
| ## Containers: Pages + TopbarContainer | ||
|
|
||
| We have set up FTW so that pages are aware of Redux, but other components and forms are purely | ||
| presentational components. This makes it easier to customize UI components - you don't need to be |
There was a problem hiding this comment.
The "container" and "presentational" terms are not defined anywhere here and might be confusing. I don't think even the Redux basics docs mention them. Maybe link to Abramov's Medium post about them or add a simple explanation here. Those are quite crucial concepts to grok.
docs/redux.md
Outdated
|
|
||
| Naturally, there is a need for higher level components which fetch new data and define what props | ||
| are passed down to presentational components. In Redux terminology, those components are called | ||
| _Containers_. FTW has defined all the containers inside a folder called `src/containers/`. It |
There was a problem hiding this comment.
Ok, here is part of the explanation for "containers". Maybe add something earlier for both "presentational" and "containers"?
There was a problem hiding this comment.
I added a link to the first occurrence of "presentational component" and another "read more" pointer to the end of this paragraph.
| complex component and it is shared with almost every page. | ||
|
|
||
| The actual container setup of a page level component can be found from the bottom of the component | ||
| file. For example, `src/containers/TransactionPage/TransactionPage.js` connects itself to Redux |
There was a problem hiding this comment.
If all there dir/file refs would be links, that would help a lot!
There was a problem hiding this comment.
I'd rather avoid links leading outside of this directory before we have some docs/guide pages.
(These full paths to files are here to help people orient themselves in a new/strange repository.)
docs/redux.md
Outdated
| folder structure), FTW tries to keep pages isolated. Page specific actions, store updates, and data | ||
| fetches happen inside their respective folder - just look for a file which name follows a pattern: | ||
| `<ComponentName>.duck.js`. This pattern was first proposed by Erik Rasmussen and you can read more | ||
| about it from [his repository](https://github.com/erikras/ducks-modular-redux). |
There was a problem hiding this comment.
Some suggestions:
- folder -> directory
- isolated -> encapsulated
Maybe add a sentence that ducks are really a simple concept of just adding related things into a single file.
There was a problem hiding this comment.
Changed. (even though Rasmussen uses "isolated")
| * the state of the `ListingPage` can be found from `state.ListingPage` and | ||
| * the state of the global `user` object can be found from `state.user`. | ||
|
|
||
| ## Advanced Redux concepts: thunks |
There was a problem hiding this comment.
Not really "advanced", but ok. :P
There was a problem hiding this comment.
It is advanced since Redux docs say it's advanced :P
| example, fetching listing reviews can be done with a following thunk function: | ||
|
|
||
| ```js | ||
| export const fetchReviews = listingId => (dispatch, getState, sdk) => { |
There was a problem hiding this comment.
Should there be a note about Redux middleware? i.e. where the sdk comes from?
There was a problem hiding this comment.
Good point. I added this to the end of that example:
Note:
sdkparameter is also provided by Redux Thunk. We pass it to middleware in store.js:thunk.withExtraArgument(sdk)
I think these need their own document or at least bigger section that just didn't fit my document-something-on-Monday routine.
I think that writing something deeper about container vs presentational components is something that needs to be learnt from better sources than this repo. Our take is already mentioned here (i.e. pages + TopbarContainer). Of course, this document could be improved over time. |
We got feedback that Redux/duck files need some documentation.