Ej/zustand store #738
Ej/zustand store #738
Conversation
|
ok looks like versioning might be better handled in the conf store? looking at the migrations section here |
|
added another commit with some types on the store. Not sure they're any good though, Zustand's typescript docs are a bit vague unfortunately... |
|
Ok I'm slightly more confident in my typings now but still need someone to look over them |
|
after a little back and forth with @gmaclennan on slack we've decided to replace |
|
lastly, since we now have a means of persisting state through zustand I suggest removing the I haven't been able to confirm it's fully compatible with existing persisted configs but the only thing we're saving is the active tab so at worst user won't see their last tab once when opening the next update which I'd argue is an acceptable trade-off |
|
That sounds good! Thanks @lightlii |
…createPersistedState
There was a problem hiding this comment.
I set up a meeting to figure out your pre-commit steps that don't seem to be working
In order to utilize typescript, you need to add "//@ts-check" to the first line of every file. store.js has some ts errors that you are not seeing currently.
I think store.js should be stored in the folder hooks since it just returns hooks
src/renderer/store.js
Outdated
| * @returns {Promise<string | null>} | ||
| */ | ||
| getItem: async key => { | ||
| return (await store.get(key)) || null |
There was a problem hiding this comment.
there is a typing issue here. Typescript is expecting string or null. store.get return unknown. You will have to cast a type here
There was a problem hiding this comment.
Not 100% sure how casting works in jsdoc, I've pushed something. Can you take a look and confirm that what you meant?
src/renderer/store.js
Outdated
| /** | ||
| * @type {import('zustand/middleware').StateStorage} | ||
| */ | ||
| const storage = { |
There was a problem hiding this comment.
We should probably set a schema to storage
There was a problem hiding this comment.
Not sure what that would look like or how best implement it... We can chat about that later though
src/renderer/components/Home.js
Outdated
| import SyncView from './SyncView' | ||
| import { STATES as updateStates, UpdaterView, UpdateTab } from './UpdaterView' | ||
| import useUpdater from './UpdaterView/useUpdater' | ||
| import Loading from './Loading' | ||
| import buildConfig from '../../build-config' | ||
| import { usePersistedUiStore } from '../store' |
There was a problem hiding this comment.
Don't forget to update the import!
Contributor checklist:
Script and updated it if necessary.
npm install --package-lockDescription
Adds zustand state store with persistence using our current conf store. Versioning is achieved with a
STORE_VERSIONconstant applied to the store, not sure if this is the best way, open to feedback.closes #735