-
Notifications
You must be signed in to change notification settings - Fork 19
docs: Add Zustand development conventions to coding guidelines (resolves #289). #332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1d34a5e
a8e70ec
4598dc9
8c8be4d
22526a6
2159be2
f92f375
56a46b5
678c4cf
8f96f8e
f5f00b7
f5822d6
d02feb6
08e9a07
19c7959
3dbb7ad
c97fa29
da469a6
eb38e38
e5842c4
31faf0a
515406c
edfbb8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -53,6 +53,190 @@ To avoid including a state variable in a React Hook's dependency array, you can | |||||||||||||||||||||||
| the state variable with an additional `Ref` suffix. E.g., `logEventNumRef` is the reference variable | ||||||||||||||||||||||||
| that corresponds to the `logEventNum` state variable. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Zustand | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Zustand is a state management tool, which allows creating global state stores that can be accessed | ||||||||||||||||||||||||
| from anywhere. Please follow the guidelines below when using Zustand stores. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## File naming conventions | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| When creating Zustand stores, we follow these naming conventions: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| * Simple stores: `{name}Store.ts` (camelCase) - single file containing all state and actions. | ||||||||||||||||||||||||
| * Large stores: create `{name}Store/` folder with: | ||||||||||||||||||||||||
| * `create{Name}{Feature}Slice.ts` - individual feature slices (e.g., `createQueryConfigSlice.ts`). | ||||||||||||||||||||||||
| * `index.ts` - main store file that combines slices and exports the main store hook. | ||||||||||||||||||||||||
| * `types.ts` - defines all types / interfaces for the store. | ||||||||||||||||||||||||
Henry8192 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
Henry8192 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
| ## Store conventions | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Guidelines for defining types, default values, and action naming conventions in Zustand stores. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Type definitions | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Split store types into three interfaces: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| * `{Name}Values`: state variables | ||||||||||||||||||||||||
| * `{Name}Actions`: action functions (methods that update state) | ||||||||||||||||||||||||
| * `{Name}State`: union of values and actions | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| For large stores, split `{Name}State` into feature-specific slices (`{Name}{Feature}Slice`) and | ||||||||||||||||||||||||
| combine them back into `{Name}State` using TypeScript intersection types. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
Henry8192 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
| :caption: Example: Log export store types | ||||||||||||||||||||||||
| :emphasize-lines: 1,5,9 | ||||||||||||||||||||||||
| interface LogExportValues { | ||||||||||||||||||||||||
| exportProgress: Nullable<number>; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| interface LogExportActions { | ||||||||||||||||||||||||
| setExportProgress: (newProgress: Nullable<number>) => void; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| type LogExportState = LogExportValues & LogExportActions; | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Default values | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| * Create an object for initial state values using the `{Name}Values` interface for type safety. | ||||||||||||||||||||||||
| * Use uppercase constant naming with `_DEFAULT` suffix | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: Log export store default values | ||||||||||||||||||||||||
| const LOG_EXPORT_STORE_DEFAULT: LogExportValues = { | ||||||||||||||||||||||||
| exportProgress: null, | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Action naming | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Use clear, consistent naming patterns: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| * `set{Property}` - simple state updates that directly assign a new value. | ||||||||||||||||||||||||
| * `update{Property}` - complex logic involving API calls, multiple state updates, or asynchronous | ||||||||||||||||||||||||
| operations. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: User store actions | ||||||||||||||||||||||||
| :emphasize-lines: 2,5 | ||||||||||||||||||||||||
| const useUserStore = create<UserState>((set, get) => ({ | ||||||||||||||||||||||||
| setName: (name) => { | ||||||||||||||||||||||||
| set({name}); | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| updateProfile: async (data) => { | ||||||||||||||||||||||||
| set({isLoading: true}); | ||||||||||||||||||||||||
| const result = await api.updateProfile(data); | ||||||||||||||||||||||||
| set({ | ||||||||||||||||||||||||
| profile: result, | ||||||||||||||||||||||||
| isLoading: false | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Feature-based slicing | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| When a Zustand store grows too large, split it into slices based on features such as functional | ||||||||||||||||||||||||
| areas. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| :::{warning} | ||||||||||||||||||||||||
| Follow the principle of separation of concerns: | ||||||||||||||||||||||||
| - Do: Slice by feature. e.g., query configuration, query results, or query controller. | ||||||||||||||||||||||||
| - Don't: Slice by type. e.g., one file for all values or one file for all actions | ||||||||||||||||||||||||
| ::: | ||||||||||||||||||||||||
|
Comment on lines
+143
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Fix list style and spacing inside admonition (MD004, MD032) Use asterisks to match the rest of the doc and add a blank line before the list. -:::{warning}
-Follow the principle of separation of concerns:
-- Do: Slice by feature. e.g., query configuration, query results, or query controller.
-- Don't: Slice by type. e.g., one file for all values or one file for all actions
-:::
+:::{warning}
+Follow the principle of separation of concerns:
+
+* Do: Slice by feature — e.g., query configuration, query results, or query controller.
+* Don't: Slice by type — e.g., one file for all values or one file for all actions.
+:::📝 Committable suggestion
Suggested change
🧰 Tools🪛 markdownlint-cli2 (0.17.2)145-145: Unordered list style (MD004, ul-style) 145-145: Lists should be surrounded by blank lines (MD032, blanks-around-lists) 146-146: Unordered list style (MD004, ul-style) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Each slice should be self-contained and represent a coherent unit of application functionality. | ||||||||||||||||||||||||
Henry8192 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Store access patterns | ||||||||||||||||||||||||
|
Comment on lines
+149
to
+151
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Call out the known exception (viewStore) to avoid confusion Earlier decisions favoured suppressing max‑lines for a tightly coupled viewStore rather than slicing. Add a short note so readers don’t over‑split cohesive stores. Each slice should be self-contained and represent a coherent unit of application functionality.
+
+:::{note}
+Exception: for highly cohesive stores (e.g., `viewStore`) where functions are tightly coupled,
+prefer suppressing the ESLint `max-lines-per-function` rule over forced slicing.
+:::If you want, I can add the exact link to the prior decision for traceability. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| There are three ways to access Zustand stores, each with its own use cases. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Inside store creation | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Use `get()` and `set()` to access the store's own states: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: View format store access - inside store slice creation | ||||||||||||||||||||||||
| :emphasize-lines: 3,5,10 | ||||||||||||||||||||||||
| const createViewFormattingSlice: StateCreator< | ||||||||||||||||||||||||
| ViewState, [], [], ViewFormattingSlice | ||||||||||||||||||||||||
| > = (set, get) => ({ | ||||||||||||||||||||||||
| updateIsPrettified: (newIsPrettified: boolean) => { | ||||||||||||||||||||||||
| const {isPrettified} = get(); | ||||||||||||||||||||||||
| if (newIsPrettified === isPrettified) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| // ... | ||||||||||||||||||||||||
| set({isPrettified: newIsPrettified}); | ||||||||||||||||||||||||
| // ... | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Inside React components | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| There are two access patterns depending on whether the access should be reactive or non-reactive. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #### State values | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Choose access pattern based on usage: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| *Reactive access* - when the value is used in JSX or hook dependency arrays, causing re-renders: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: Log export store value access - reactive | ||||||||||||||||||||||||
| const exportProgress = useLogExportStore((state) => state.exportProgress); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // The progress should be printed when `exportProgress` updates. | ||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||
| console.log(exportProgress); | ||||||||||||||||||||||||
| }, [exportProgress]); | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| *Non-reactive access* - when the value should not trigger re-renders or hook re-runs, | ||||||||||||||||||||||||
| typically for one-time reads: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: Log export store value access - non-reactive | ||||||||||||||||||||||||
| // The progress should be printed only once when the component mounts. | ||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||
| const {exportProgress} = useLogExportStore.getState(); | ||||||||||||||||||||||||
| console.log(exportProgress); | ||||||||||||||||||||||||
| }, []); | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #### Actions | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Actions usually do not change after initialization, so always access them non-reactively: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: Log export store action access - non-reactive | ||||||||||||||||||||||||
| const handleExportButtonClick = useCallback(() => { | ||||||||||||||||||||||||
| const {exportLogs} = useLogExportStore.getState(); | ||||||||||||||||||||||||
| exportLogs(); | ||||||||||||||||||||||||
| }, []); | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
Henry8192 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Outside React components | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Always use non-reactive access since reactive subscriptions do not work for outside components. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```{code-block} ts | ||||||||||||||||||||||||
| :caption: Example: An error handler that accesses the Notification store outside of any component | ||||||||||||||||||||||||
| :emphasize-lines: 3,4,9 | ||||||||||||||||||||||||
| const handleErrorWithNotification = (e: unknown) => { | ||||||||||||||||||||||||
| // ... | ||||||||||||||||||||||||
| const {postPopUp} = useNotificationStore.getState(); | ||||||||||||||||||||||||
| postPopUp({ | ||||||||||||||||||||||||
| level: LOG_LEVEL.ERROR, | ||||||||||||||||||||||||
| message: message, | ||||||||||||||||||||||||
| timeoutMillis: DO_NOT_TIMEOUT_VALUE, | ||||||||||||||||||||||||
| title: "Action failed", | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| [eslint-config-mjs]: https://github.com/y-scope/yscope-log-viewer/blob/main/eslint.config.mjs | ||||||||||||||||||||||||
| [vite-worker-query-suffix]: https://vite.dev/guide/features.html#import-with-query-suffixes | ||||||||||||||||||||||||
| [yscope-guidelines]: https://docs.yscope.com/dev-guide/contrib-guides-overview.html | ||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.