feat: defer everything until the api config is loaded #926

Merged
konrad merged 27 commits from feature/ready-state into main 2021-11-13 19:49:03 +00:00
5 changed files with 180 additions and 149 deletions
Showing only changes of commit dc30de9176 - Show all commits

View File

@ -1,26 +1,27 @@
<template>
<div :class="{'is-touch': isTouch}" v-if="ready">
<div :class="{'is-hidden': !online}">
<!-- This is a workaround to get the sw to "see" the to-be-cached version of the offline background image -->
<div class="offline" style="height: 0;width: 0;"></div>
<top-navigation v-if="authUser"/>
<content-auth v-if="authUser"/>
<content-link-share v-else-if="authLinkShare"/>
<content-no-auth v-else/>
<notification/>
</div>
<div class="app offline" v-if="!online">
<div class="offline-message">
<h1>You are offline.</h1>
<p>Please check your network connection and try again.</p>
<ready>
<div :class="{'is-touch': isTouch}">
<div :class="{'is-hidden': !online}">
<!-- This is a workaround to get the sw to "see" the to-be-cached version of the offline background image -->
<div class="offline" style="height: 0;width: 0;"></div>
<top-navigation v-if="authUser"/>
konrad marked this conversation as resolved Outdated

Wrap in <template v-if="authUser">

Wrap in `<template v-if="authUser">`

Why?

Why?

to make clear that it mounts at the same time than the content-auth below.

to make clear that it mounts at the same time than the content-auth below.

Ah, that makes sense. Done.

Ah, that makes sense. Done.
<content-auth v-if="authUser"/>
<content-link-share v-else-if="authLinkShare"/>
<content-no-auth v-else/>
<notification/>
</div>
<div class="app offline" v-if="!online">
konrad marked this conversation as resolved Outdated

Didn't test, but just from reading:
might it be that the not offline check needs to be outside of <ready>?

Else the api might not be reached because we are offline. => slot is never exposed.

Didn't test, but just from reading: might it be that the not offline check needs to be outside of `<ready>`? Else the api might not be reached because we are offline. => slot is never exposed.

I think that's pretty much the case. Moved the offline overlay to the ready component.

I think that's pretty much the case. Moved the offline overlay to the ready component.
<div class="offline-message">
<h1>You are offline.</h1>
<p>Please check your network connection and try again.</p>
</div>
</div>
</div>
<transition name="fade">
<keyboard-shortcuts v-if="keyboardShortcutsActive"/>
</transition>
</div>
<vikunja-loading v-else/>
<transition name="fade">
<keyboard-shortcuts v-if="keyboardShortcutsActive"/>
</transition>
</div>
</ready>
</template>
<script>
@ -37,18 +38,18 @@ import ContentLinkShare from './components/home/contentLinkShare'
import ContentNoAuth from './components/home/contentNoAuth'
import {setLanguage} from './i18n'
import AccountDeleteService from '@/services/accountDelete'
import VikunjaLoading from '@/components/misc/vikunja-loading'
import Ready from '@/components/misc/ready'
export default defineComponent({
name: 'app',
components: {
VikunjaLoading,
ContentNoAuth,
ContentLinkShare,
ContentAuth,
TopNavigation,
KeyboardShortcuts,
Notification,
Ready,
},
beforeMount() {
this.setupOnlineStatus()
@ -57,7 +58,6 @@ export default defineComponent({
this.setupAccountDeletionVerification()
},
beforeCreate() {
this.$store.dispatch('loadApp')
setLanguage()
},
created() {
@ -73,7 +73,6 @@ export default defineComponent({
...mapState({
online: ONLINE,
keyboardShortcutsActive: KEYBOARD_SHORTCUTS_ACTIVE,
ready: 'vikunjaReady',
}),
...mapGetters('auth', [
'authUser',

View File

@ -26,7 +26,7 @@
<i18n-t keypath="apiConfig.signInOn">
<span class="url" v-tooltip="apiUrl"> {{ apiDomain }} </span>
</i18n-t>
<br />
<br/>
<a @click="() => (configureApi = true)">{{ $t('apiConfig.change') }}</a>
</div>
@ -46,9 +46,8 @@
</template>
<script>
import { parseURL } from 'ufo'
const API_DEFAULT_PORT = 3456
import {parseURL} from 'ufo'
import {checkAndSetApiUrl} from '../../helpers/checkAndSetApiUrl'
konrad marked this conversation as resolved Outdated

use @

use `@`

Done.

Done.
export default {
name: 'apiConfig',
@ -77,121 +76,23 @@ export default {
return
}
let urlToCheck = this.apiUrl
// Check if the url has an http prefix
if (
!urlToCheck.startsWith('http://') &&
!urlToCheck.startsWith('https://')
) {
urlToCheck = `http://${urlToCheck}`
}
urlToCheck = new URL(urlToCheck)
const origUrlToCheck = urlToCheck
const oldUrl = window.API_URL
window.API_URL = urlToCheck.toString()
// Check if the api is reachable at the provided url
this.$store
.dispatch('config/update')
.catch((e) => {
// Check if it is reachable at /api/v1 and http
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
.catch((e) => {
// Check if it has a port and if not check if it is reachable at https
if (urlToCheck.protocol === 'http:') {
urlToCheck.protocol = 'https:'
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
.catch((e) => {
// Check if it is reachable at /api/v1 and https
urlToCheck.pathname = origUrlToCheck.pathname
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
.catch((e) => {
// Check if it is reachable at port API_DEFAULT_PORT and https
if (urlToCheck.port !== API_DEFAULT_PORT) {
urlToCheck.protocol = 'https:'
urlToCheck.port = API_DEFAULT_PORT
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
.catch((e) => {
// Check if it is reachable at :API_DEFAULT_PORT and /api/v1 and https
urlToCheck.pathname = origUrlToCheck.pathname
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
.catch((e) => {
// Check if it is reachable at port API_DEFAULT_PORT and http
if (urlToCheck.port !== API_DEFAULT_PORT) {
urlToCheck.protocol = 'http:'
urlToCheck.port = API_DEFAULT_PORT
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
.catch((e) => {
// Check if it is reachable at :API_DEFAULT_PORT and /api/v1 and http
urlToCheck.pathname = origUrlToCheck.pathname
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return this.$store.dispatch('config/update')
}
throw e
})
checkAndSetApiUrl(this.apiUrl, () => this.$store.dispatch('config/update'))
.catch(() => {
// Still not found, url is still invalid
this.successMsg = ''
this.errorMsg = this.$t('apiConfig.error', {domain: this.apiDomain})
window.API_URL = oldUrl
})
.then((r) => {
if (typeof r !== 'undefined') {
// Set it + save it to local storage to save us the hoops
this.errorMsg = ''
this.successMsg = this.$t('apiConfig.success', {domain: this.apiDomain})
localStorage.setItem('API_URL', window.API_URL)
this.configureApi = false
this.apiUrl = window.API_URL
this.$emit('foundApi', this.apiUrl)
.then(url => {
if (url === '') {
return
}
// Set it + save it to local storage to save us the hoops
this.errorMsg = ''
konrad marked this conversation as resolved Outdated

Explain why we return here

Explain why we return here

Done.

Done.
this.successMsg = this.$t('apiConfig.success', {domain: this.apiDomain})
konrad marked this conversation as resolved Outdated

That seems like it should throw an error?

That seems like it should throw an error?

It does yeah. I've modified it so it shows an error with a "good" error message to the user instead of throwing one.

It does yeah. I've modified it so it shows an error with a "good" error message to the user instead of throwing one.
this.configureApi = false
this.apiUrl = url
this.$emit('foundApi', this.apiUrl)
})
},
},
konrad marked this conversation as resolved
Review

Explain why we return here

Explain why we return here
@ -200,15 +101,15 @@ export default {
<style lang="scss" scoped>
.api-config {
margin-bottom: .75rem;
margin-bottom: .75rem;
}
.api-url-info {
font-size: .9rem;
text-align: right;
font-size: .9rem;
text-align: right;
span.url {
border-bottom: 1px dashed $primary;
}
span.url {
border-bottom: 1px dashed $primary;
}
}
</style>

View File

@ -1,5 +1,11 @@
<template>
<section class="vikunja-loading">
<template v-if="ready">
<slot/>
</template>
<section v-else-if="error !== ''">
{{ error }}
</section>
<section class="vikunja-loading" v-else>
<img alt="Vikunja" :src="logoUrl" width="100" height="100"/>
<p>
<span class="loader-container is-loading-small is-loading"></span>
@ -10,14 +16,27 @@
<script>
import logoUrl from '@/assets/logo.svg'
import {setLanguage} from '@/i18n'
export default {
name: 'vikunja-loading',
name: 'ready',
data() {
return {
logoUrl,
error: '',
}
},
mounted() {
this.$store.dispatch('loadApp')
.catch(e => {
this.error = e
})
},
computed: {
ready() {
return this.$store.state.vikunjaReady
},
},
}
</script>
@ -29,14 +48,14 @@ export default {
height: 100vh;
width: 100vw;
flex-direction: column;
img {
margin-bottom: 1rem;
}
konrad marked this conversation as resolved Outdated

start loading in created

start loading in created

Done.

Done.
.loader-container {
margin-right: 1rem;
&.is-loading:after {
border-left-color: $grey-400;
border-bottom-color: $grey-400;

View File

@ -0,0 +1,111 @@
const API_DEFAULT_PORT = '3456'
export const checkAndSetApiUrl = (url: string, updateConfig: () => Promise<void>): Promise<string> => {
// Check if the url has an http prefix
if (
konrad marked this conversation as resolved Outdated

Since updateConfig is always () => dispatch('config/update'):
Shouldn't we just import he store in this file and call dispatch('config/update') directly?

Since `updateConfig` is always `() => dispatch('config/update')`: Shouldn't we just import he store in this file and call `dispatch('config/update')` directly?

Will that work outside of a vue component?

Will that work outside of a vue component?

If you import the store with:

import {store} from '@/store'

// then you can

store.dispatch('config/update')

Because it's JS ™️ (trying to steal reacts claim here)

If you import the store with: ``` import {store} from '@/store' // then you can store.dispatch('config/update') ``` Because it's JS ™️ (trying to steal reacts claim here)

That seems like a nice way to solve it. Done.

That seems like a nice way to solve it. Done.
!url.startsWith('http://') &&
!url.startsWith('https://')
) {
url = `http://${url}`
}
const urlToCheck: URL = new URL(url)
const origUrlToCheck = urlToCheck
const oldUrl = window.API_URL
window.API_URL = urlToCheck.toString()
// Check if the api is reachable at the provided url
return updateConfig()
.catch(e => {
// Check if it is reachable at /api/v1 and http
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
// Check if it has a port and if not check if it is reachable at https
if (urlToCheck.protocol === 'http:') {
urlToCheck.protocol = 'https:'
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
// Check if it is reachable at /api/v1 and https
urlToCheck.pathname = origUrlToCheck.pathname
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
// Check if it is reachable at port API_DEFAULT_PORT and https
if (urlToCheck.port !== API_DEFAULT_PORT) {
urlToCheck.protocol = 'https:'
urlToCheck.port = API_DEFAULT_PORT
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
// Check if it is reachable at :API_DEFAULT_PORT and /api/v1 and https
urlToCheck.pathname = origUrlToCheck.pathname
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
// Check if it is reachable at port API_DEFAULT_PORT and http
if (urlToCheck.port !== API_DEFAULT_PORT) {
urlToCheck.protocol = 'http:'
urlToCheck.port = API_DEFAULT_PORT
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
// Check if it is reachable at :API_DEFAULT_PORT and /api/v1 and http
urlToCheck.pathname = origUrlToCheck.pathname
if (
!urlToCheck.pathname.endsWith('/api/v1') &&
!urlToCheck.pathname.endsWith('/api/v1/')
) {
urlToCheck.pathname = `${urlToCheck.pathname}api/v1`
window.API_URL = urlToCheck.toString()
return updateConfig()
}
throw e
})
.catch(e => {
window.API_URL = oldUrl
throw e
})
.then(r => {
if (typeof r !== 'undefined') {
localStorage.setItem('API_URL', window.API_URL)
return window.API_URL
}
return ''
})
}

View File

@ -19,6 +19,7 @@ import attachments from './modules/attachments'
import labels from './modules/labels'
import ListService from '../services/list'
import {checkAndSetApiUrl} from '../helpers/checkAndSetApiUrl'
konrad marked this conversation as resolved Outdated

Use @

Use `@`

Done.

Done.
export const store = createStore({
strict: import.meta.env.DEV,
@ -143,7 +144,7 @@ export const store = createStore({
commit(CURRENT_LIST, currentList)
},
async loadApp({commit, dispatch}) {
await dispatch('config/update')
await checkAndSetApiUrl(window.API_URL, () => dispatch('config/update'))
await dispatch('auth/checkAuth')
commit('vikunjaReady', true)
},