[Docs Rewrite] New Page: Style Guide
markerikson opened this issue · 11 comments
This is a tracking issue to cover all work related to adding a new "Style Guide" page / section.
Tasks
- Outline desired topics
- Define recommendations
- Write content
Initial Style Guide Research Notes
The Redux docs have always been very unopinionated. The purpose of a "Style Guide" page is to specifically list our recommended approaches for various concepts, to provide guidance to the community on best practices.
I'm explicitly modeling this page on the Vue docs page at https://vuejs.org/v2/style-guide/ .
Vue Style Guide - Summary and Notes
Vue Rule Categories
- Priority A: Essential
- Prevents errors, so follow them all the time (only rare exceptions)
- Priority B: Strongly Recommended
- Improves readability and DX; justify exceptions
- Priority C: Recommended
- Multiple good options exist, but we recommend a default
- Prefer the community standard
- Priority D: Use with caution
- Potentially risky features or meant for edge cases
Vue Rules
Priority A
- Multi-word component names
data
must be a function- Detailed prop definitions (defaults, validation)
- keyed
v-for
- avoid
v-if
withv-for
- scope styles
- private property names
Priority B
- File per component
- Component filenames should be PascalCase or kebab-case
- Start base component names with
Base
- Singleton components start with
The
- Started coupled components with parent component name as prefix
- Words in names should be general to specific
- Self-close component tags
- Component name casing in templates and JSX
- Use full words in component names
- Prop names should be declared camel-cased, but kebab-cased in templates
- Spread multi-attribute elements across multiple lines
- Simple expressions in templates
- Split up complex computed properties into smaller ones
- Quote attribute values
- Use directive shorthands
Priority C
- Ordering for instance options and element attributes
- Empty lines in instance options
- Ordering in SFCs
Priority D
v-if
without key- Element selectors with
scoped
- Implicit parent-child communication
- Prefer VueX for global state management instead of an event bus
Observations
- A lot of the rules are truly "style"-related (naming, ordering, etc)
- Page is grouped by priority categories
- Rules show highlighted examples of "bad" and "good" usage
- Rule titles have additional priority tags at the end (like "Component Data Essential")
- Some rules have expandable "Detailed Explanation" sections
I'm curious how we can actually implement some of the highlights and formatting that exist in the Vue page using Docusaurus. @wgao19 , @endiliey , @yangshun : any ideas?
Redux Rules
This is an initial list of rules that I would want the docs to express an opinion on, with initial categories.
I am totally open to suggestions for improving this list: additional topics we should cover, specific points of emphasis, altering categories, etc.
Priority A (prevents errors):
- Do not mutate data
- No side effects in reducers (including generating random values like UUIDs or timestamps)
- Do not put non-serializable values in state, and only in actions if it will be stopped by a middleware before reaching the reducers.
- Only one store per app
Priority B
- Use Immer for immutable updates (preferably with RSK)
- Folder structure:
- Prefer use of feature folders or ducks, vs folder-by-type
- Suggest naming "duck files" as
someFeatureSlice.js
- Business logic location
- Prefer doing as much as possible in reducers
- Minimize "blind spreads/returns" like
return action.payload
orreturn {...state, ...action.payload}
- Try to treat reducers as state machines that only update if appropriate based on the current state.
- Action semantics
- Model actions as "events in the app", not "remote setters"
- Prefer dispatching one action handled by multiple reducers
- Minimize multi-dispatch sequences (batch if necessary)
- Define many meaningful actions for a readable history log, vs just a
"SET_DATA"
or"UPDATE_STORE"
action
- Action type strings:
- Prefer
"domain/eventName"
- Prefer
- Use the FSA action format
- May model actions with separate error types vs
error: true
- May model actions with separate error types vs
- Evaluate each piece of state to determine whether it should live in Redux or not
- Keep form state outside Redux by default
- Use static typing
- If using
connect
, use the "object shorthand" formapDispatch
- If using hooks, create multiple smaller
useSelector
s, and don't bind action creators - Connect/select from the store in more components at a more granular level
- Normalize complex/relational data in the store
Priority C
- Use action creators for consistency
- Async middleware
- Default to thunks; add sagas or observables if you have truly complex async workflows
- Use selector functions to read store state
- Use Reselect
- But, find a middle ground for granularity - don't define selectors for every field
- Prefer putting more complex sync/async logic outside the component (thunks, etc)
Thoughts?
I took a quick look at the Vue Style Guide. Having an official battle-tested opinion would be a great addition to the docs.
Having a background for the code might be harder to do in D1 but it's very easy in D2. Maybe we could port the website to D2 while rewriting the docs. The Docusaurus team could help with that.
@yangshun : yep, I figure we can migrate RSK -> RR -> Redux core, in that order. But, I don't want to hold up getting new content in place waiting for a revised publishing approach. We can always format stuff better later. (Or, if necessary, add some manual HTML tags to the Markdown to get formatting.)
Besides, this is probably one of the first pages I want to put together.
I love this. One thing that I personally would add to the "Evaluate each piece of state to determine whether it should live in Redux or not", but don't really know your opinion on:
API Cache state.
Personally, I believe that in most cases this should not be handled by redux, as it will people start at zero/almost at zero every time, leaving them with problems like
- cancellation
- cache expiration (which is tightly coupled with the information if there are components mounted that currently use that cache
- normalization
- in the future: most likely some problems with concurrent mode
- explosion of code as usually one api endpoint <-> one slice
Honestly, I haven't seen one implementation in the wild that does all of those right.
Either we should be able to recommend a good middleware for that (if there is one I don't know it), or a few libraries that live outside of the redux space (as for most people, those will suffice and leave redux for the business logic).
What's your take on this?
@phryneas : I definitely don't want to address that kind of topic in the style guide page. It may be worth discussing it in a separate page under the "Real World Usage" category, but tbh those topics are pretty well out of my area of expertise.
@phryneas : I definitely don't want to address that kind of topic in the style guide page. It may be worth discussing it in a separate page under the "Real World Usage" category, but tbh those topics are pretty well out of my area of expertise.
Good call. It's loosely related to the form-advice, so I was a bit triggered there, but it shouldn't be in a "best practice" style guide I guess.
If you want to add a page like that somewhere else, feel free to ping me in the issue for that page. I've got loads of opinion on that topic ;)
Sure. Added another task entry to #3598 for a "Data Fetching and Caching" page.
If you'd like, go ahead and create an issue specifically to start planning the contents for that page.
I've put up an initial outline for the "Style Guide" page.
For now, I'm keeping it as one single page, but I'm totally open to splitting it up into multiple sections somehow. I've also got the recommendations grouped by priority (same as Vue), but could see good arguments for grouping them by concept somehow (especially if the list was split into pages by priority).
I also borrowed the "flair"-type styling from the Vue docs page for marking each rule header as "ESSENTIAL", "STRONGLY RECOMMENDED", or "RECMMENDED".
Next step is to actually fill out the contents of each rule entry.
I'm interested in feedback on the list of topics and recommendations, the content (once it's written), and the structure.
It's a great idea to put this together but also a challenging one. Thanks for undertaking it anyway :) I'm not sure if this is the correct place for this kind of detailed discussion but let's try anyway.
Normalize complex/relational data in the store
I think we should distinguish two things: normalizing data and having a single source of truth. We don't need to have the first one to fulfil the second one and the latter is much more important. I saw quite a bit redux apps that became unnecessarly complex just because they tried to normalize the whole state (especially the one from the server) when they would have been fine with the more colocated state which is much easier to keep consistent during updates (and with Immer we don't have issues with updating it in an immutable way) and easier to select/read (we don't need complex selector to glue together many pieces of data). What we should strive for is to split our state into aggregates what is my next point :)
Prefer use of feature folders or ducks, vs folder-by-type
I think when we talk about slices
or ducks
, we just rediscover aggregates. Instead of reinventing the wheel we could try to apply what is already a standard in IT and is well defined. I'm not saying that we should change the naming, but at least we could mention and promote the original concept which could help with better understanding how to slice our state to avoid "overnormalization" and "undernormalization". In my opinion, it is the single most important aspect when working with Redux (and any other data storage) and most problems come from not following principles behind aggregates
.
Connect/select from the store in more components at a more granular level
For me, it is more like an optimization than good practice. Connecting components to redux come with a great cost. We sacrifice reusability, testability and readability. Redux is sill a global state and the less we tangle our app with it the better. Clear boundaries between what is connected and what not are really helpful when come to understanding how data flows through our app.
@wand3r : thanks, appreciate the feedback.
I'll agree that with Immer it's a lot easier to update nested state than it used to be. But, there's still benefits for keeping more state normalized, especially when the UI tries to read that data (particularly being able to easily extract a single item from the state based on its ID).
I'm very hesitant to introduce any "new" terminology that isn't already being used with Redux. As it is, the "slice" term is something we sort of came up with ourselves, but it's been in the docs for a while. I get what you're saying about similarities with terms from outside of Redux, but given that there's already a ton of terms in the docs, I'd rather avoid throwing any more terminology in there.
As for the "connect many components" thing, it's a tradeoff. The early docs suggested you should only connect once at the root of your app, but at that point you're forcing the entire app to re-render on every update, which kills perf. If you use connect
many times, it's better for perf, but you've got lots of wrapper components. If you use useSelector
many times, you drop the wrapper components, but you're embedding the dependency on the store. So, there's a balance to be had, and it's up to the user to decide what that is. I talked about this in my post Thoughts on React Hooks, Redux, and Separation of Concerns, and my ReactBoston 2019 talk on "Hooks, HOCs, and Tradeoffs".
Aaaaand we're live!
https://redux.js.org/style-guide/style-guide
I made some initial progress on the outline and the first couple entries over the weekend, and was able to crank out the rest of it tonight.
We can always make further edits down the road, but this should hopefully be a valuable addition to the docs.