refactor: Unify generator store slices creation #39
Conversation
| name: generateNewName(), | ||
| setName: (name: string) => | ||
| set((state) => { | ||
| state.name = name | ||
| }), |
There was a problem hiding this comment.
Perhaps these could also be in their own slice, something like metadataSlice. Seems a bit excessive though
| allowList: [], | ||
| filteredRequests: [], |
There was a problem hiding this comment.
filteredRequests is essentially a computed state, which means storing both allowList and filteredRequests is a bit error prone. Perhaps we can use a memoized selector instead?
There was a problem hiding this comment.
I think this might be a little complicated 🤔
When saving the State to the file we do no care about filteredRequests.
The interface for State will need validation so it might become a zod type object.
Should we have the State and the extra State (filteredRequests..) as separated state objects or will it make things even more complex 😕
There was a problem hiding this comment.
We may need to write some adapter functions that will turn the state into a file: it's not just that the state has some runtime properties (like filteredRequests or showAllowListDialog), but also some slices has extra properties (like load profile)
There was a problem hiding this comment.
I think there will be 2 separate types of state: State for app use and GeneratorFile type, which won't include internal props like selectedRunId. And 2 functions to convert between them.
As for filteredRequests - agree that it should be a memoized selector 👍
| interface State { | ||
| rules: TestRule[] | ||
| selectedRuleId: string | null | ||
| } | ||
|
|
||
| interface Actions { | ||
| createRule: (type: TestRule['type']) => void | ||
| updateRule: (rule: TestRule) => void | ||
| cloneRule: (id: string) => void | ||
| deleteRule: (id: string) => void | ||
| selectRule: (id: string) => void | ||
| } | ||
|
|
||
| export type RulesSliceStore = State & Actions |
There was a problem hiding this comment.
I feel like having internal State and Actions improves readability, while exporting a store type means we can use it elsewhere as before
| selectRule: (id: string) => void | ||
| } | ||
|
|
||
| export type RulesSliceStore = State & Actions |
There was a problem hiding this comment.
RulesSliceStore vs RulesSliceState vs RulesSliceType - what's your choice?
There was a problem hiding this comment.
we might have to discuss the requirements more before deciding 🤔
There was a problem hiding this comment.
What do you think about moving store hooks to a different folder? src/store?
| allowList: [], | ||
| filteredRequests: [], |
There was a problem hiding this comment.
I think this might be a little complicated 🤔
When saving the State to the file we do no care about filteredRequests.
The interface for State will need validation so it might become a zod type object.
Should we have the State and the extra State (filteredRequests..) as separated state objects or will it make things even more complex 😕
| import { ProxyData } from '@/types' | ||
| import { ImmerStateCreator } from '@/utils/typescript' | ||
|
|
||
| interface State { |
There was a problem hiding this comment.
any specific reason for dropping the prefix to the name ?
As it will be used outside it might make sense to keep it 🤔
There was a problem hiding this comment.
It's not exported, so the prefix is dropped for readability
| selectRule: (id: string) => void | ||
| } | ||
|
|
||
| export type RulesSliceStore = State & Actions |
There was a problem hiding this comment.
we might have to discuss the requirements more before deciding 🤔
No description provided.