Conversation
Split the store into: - auth-store: OIDC init, JMAP client, connection state - email-store: mailboxes, message list (vue-query), detail, delta sync - compose-store: compose state, identities, send logic
MelissaAutumn
left a comment
MelissaAutumn
left a comment
There was a problem hiding this comment.
No dom manip in stores please.
They're storage containers for things like state. They should store a raw-ish state, and using computed turn that state into something more processed.
If you need to act on that state there should be functions in the store that do it for you.
That's not to say Appointment/Accounts does this 100%, but it's generally what stores are for. I tend to shove any convenience functions that are related to the state as well.
Given the state of this code, you should probably write these from scratch based on what you need to do from the main app with those concepts in mind.
Also these need comments, written by humans pls.
| const client = ref(null) | ||
| const connected = ref(false) | ||
| const status = ref('Not connected.') | ||
| const error = ref('') |
| } catch (e) { | ||
| status.value = 'Failed.' | ||
| error.value = | ||
| e.message + |
| }) | ||
| await client.value.fetchSession() | ||
| connected.value = true | ||
| document.body.classList.add('connected') |
There was a problem hiding this comment.
This can be inferred based on the oidc status enum on whatever is consuming this store. No dom manip in stores!
| const status = ref('Not connected.') | ||
| const error = ref('') | ||
| const oidcReady = ref(false) | ||
| const oidcLoading = ref(true) |
There was a problem hiding this comment.
Are these states independant of each other? You can merge it into an enum. (You're not using typescript but you can still create a object with the values.)
There was a problem hiding this comment.
ex/
const OIDC_LOADING_STATE = {
NOT_CONNECTED: 'not_connected',
FAILED: 'failed',
CONNECTING: 'connecting',
READY: 'ready',
};Set it in your code like so:
oidcLoadingStatus.value = OIDC_LOADING_STATE.CONNECTING;
Then in your l10n you can simply have a computed that returns return l10n(`status_codes.${oidcLoadingStatus}`).
| export const useAuthStore = defineStore('auth', () => { | ||
| const client = ref(null) | ||
| const connected = ref(false) | ||
| const status = ref('Not connected.') |
There was a problem hiding this comment.
These can be inferred based on an enum state I mentioned below.
|
|
||
| const identities = ref([]) | ||
| const sending = ref(false) | ||
| const composeStatus = ref('') |
There was a problem hiding this comment.
Should be an enum like in auth-store.
There was a problem hiding this comment.
Bundle this with a composeLastError for extra points or something.
| } | ||
| } catch {} | ||
| return copy | ||
| } |
| } | ||
|
|
||
| function sanitizeForDebug(obj) { | ||
| const copy = JSON.parse(JSON.stringify(obj)) |
There was a problem hiding this comment.
| ) | ||
| } | ||
| } catch {} | ||
| return issues.join('\n') |
There was a problem hiding this comment.
Please flatten by early short-circuiting.
| if (role === 'drafts' || mailboxName === 'drafts') return 'Drafts' | ||
| if (role === 'archive' || mailboxName === 'archive' || mailboxName === 'archives') return 'Archives' | ||
| if (role === 'inbox' || mailboxName === 'inbox') return 'Inbox' | ||
| return m.name || 'Mailbox' |
2 participants
Split the store into: