Skip to content
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

move prefetch logic out of useLink and into composables #288

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 18 additions & 56 deletions src/compositions/useLink.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import { ComputedRef, MaybeRefOrGetter, computed, toRef, toValue, watch } from 'vue'
import { usePropStore } from '@/compositions/usePropStore'
import { ComputedRef, MaybeRefOrGetter, computed, toRef, toValue } from 'vue'
import { usePrefetchedComponent } from '@/compositions/usePrefetchedComponent'
import { usePrefetchedProps } from '@/compositions/usePrefetchedProps'
import { useRouter } from '@/compositions/useRouter'
import { InvalidRouteParamValueError } from '@/errors/invalidRouteParamValueError'
import { RouterResolveOptions } from '@/services/createRouterResolve'
import { isWithComponent, isWithComponents } from '@/types/createRouteOptions'
import { PrefetchConfig, PrefetchConfigs, getPrefetchOption } from '@/types/prefetch'
import { PrefetchConfig, PrefetchConfigs } from '@/types/prefetch'
import { RegisteredRoutes, RegisteredRoutesName } from '@/types/register'
import { ResolvedRoute } from '@/types/resolved'
import { RouterPushOptions } from '@/types/routerPush'
import { RouterReplaceOptions } from '@/types/routerReplace'
import { RouteParamsByKey } from '@/types/routeWithParams'
import { Url, isUrl } from '@/types/url'
import { AllPropertiesAreOptional } from '@/types/utilities'
import { isAsyncComponent } from '@/utilities/components'

export type UseLink = {
/**
Expand Down Expand Up @@ -78,9 +77,6 @@ export function useLink(
const sourceRef = toRef(source)
const paramsRef = computed<Record<PropertyKey, unknown>>(() => isUrl(sourceRef.value) ? {} : toValue(paramsOrOptions))
const optionsRef = computed<UseLinkOptions>(() => isUrl(sourceRef.value) ? toValue(paramsOrOptions) : toValue(maybeOptions))
const { getPrefetchProps, setPrefetchProps } = usePropStore()

let props: Record<string, unknown> = {}

const href = computed(() => {
if (isUrl(sourceRef.value)) {
Expand All @@ -103,8 +99,21 @@ export function useLink(
const isExactMatch = computed(() => !!route.value && router.route.matched === route.value.matched)
const isExternal = computed(() => router.isExternal(href.value))

const prefectConfig = computed<PrefetchConfigs>(() => {
const { prefetch: routerPrefetch } = router
const { prefetch: linkPrefetch } = optionsRef.value

return {
routerPrefetch,
linkPrefetch,
}
})

const { commitPrefetchedProps } = usePrefetchedProps(route, prefectConfig)
usePrefetchedComponent(route, prefectConfig)

const push: UseLink['push'] = (options) => {
setPrefetchProps(props)
commitPrefetchedProps()

return router.push(href.value, { ...optionsRef.value, ...options })
}
Expand All @@ -113,25 +122,6 @@ export function useLink(
return push(options)
}

watch(route, route => {
if (!route) {
return
}

const { prefetch: routerPrefetch } = router
const { prefetch: linkPrefetch } = optionsRef.value

prefetchComponentsForRoute(route, {
routerPrefetch,
linkPrefetch,
})

props = getPrefetchProps(route, {
routerPrefetch,
linkPrefetch,
})
}, { immediate: true })

return {
route,
href,
Expand All @@ -141,32 +131,4 @@ export function useLink(
push,
replace,
}
}

function prefetchComponentsForRoute(route: ResolvedRoute, { routerPrefetch, linkPrefetch }: PrefetchConfigs): void {

route.matches.forEach(route => {
const shouldPrefetchComponents = getPrefetchOption({
routePrefetch: route.prefetch,
routerPrefetch,
linkPrefetch,
}, 'components')

if (!shouldPrefetchComponents) {
return
}

if (isWithComponent(route) && isAsyncComponent(route.component)) {
route.component.setup()
}

if (isWithComponents(route)) {
Object.values(route.components).forEach(component => {
if (isAsyncComponent(component)) {
component.setup()
}
})
}
})

}
35 changes: 35 additions & 0 deletions src/compositions/usePrefetchedComponent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { computed, MaybeRef, ref, watch } from 'vue'
import { isWithComponent, isWithComponents } from '@/types/createRouteOptions'
import { getPrefetchOption, PrefetchConfigs } from '@/types/prefetch'
import { ResolvedRoute } from '@/types/resolved'
import { isAsyncComponent } from '@/utilities/components'

export function usePrefetchedComponent(route: MaybeRef<ResolvedRoute | undefined>, prefetch: MaybeRef<PrefetchConfigs | undefined>): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MaybeRefOrGetter is preferred over MaybeRef just because its more flexible.

const routeRef = ref(route)
const prefetchRef = ref(prefetch)
Comment on lines +8 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: using toValue inside the matchesToPrefetchComponents is preferred


const matchesToPrefetchComponents = computed(() => {
const matches = routeRef.value?.matches ?? []

return matches.filter(route => getPrefetchOption({
...prefetchRef.value,
routePrefetch: route.prefetch,
}, 'components'))
})

watch(matchesToPrefetchComponents, routes => {
routes.forEach(route => {
if (isWithComponent(route) && isAsyncComponent(route.component)) {
route.component.setup()
}

if (isWithComponents(route)) {
Object.values(route.components).forEach(component => {
if (isAsyncComponent(component)) {
component.setup()
}
})
}
})
}, { immediate: true })
}
30 changes: 30 additions & 0 deletions src/compositions/usePrefetchedProps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { MaybeRef, Ref, ref, watch } from 'vue'
import { usePropStore } from '@/compositions/usePropStore'
import { PrefetchConfigs } from '@/types/prefetch'
import { ResolvedRoute } from '@/types/resolved'

export type UsePrefetchedProps = {
prefetchProps: Ref<Record<string, unknown>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need to expose this right?

commitPrefetchedProps: () => void,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: kinda like how simple "commit" would be here.

Suggested change
commitPrefetchedProps: () => void,
commit: () => void,

}

export function usePrefetchedProps(route: MaybeRef<ResolvedRoute | undefined>, prefetch: MaybeRef<PrefetchConfigs | undefined>): UsePrefetchedProps {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another spot to use MaybeRefOrGetter

One thing I did in my version of this was may this a single object so you could do this

usePrefetchedProps(() => ({
  route: route.value,
  routerPrefetch: router.prefetch,
  linkPrefetch: options.prefetch
}))

Don't feel strongly, just felt nice when I was playing with it.

const routeRef = ref(route)
const prefetchRef = ref(prefetch)
const store = usePropStore()
const prefetchProps = ref<Record<string, unknown>>({})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't reactive so we should just use a let here. Also, we're in a prefetch props composition. We could probably simplify some of these names.

Suggested change
const prefetchProps = ref<Record<string, unknown>>({})
const props: Record<string, unknown> = {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you are exposing it. I guess I wouldn't expose it and I wouldn't make it reactive. That would be my suggestion.


watch([routeRef, prefetchRef], ([route, prefetch]) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single object works nicely here just because you end up with this

watch(option, ({ route, ...configs }) => {
  ...
})

if (!route) {
return
}

prefetchProps.value = store.getPrefetchProps(route, prefetch ?? {})
}, { immediate: true })

function commitPrefetchedProps(): void {
store.setPrefetchProps(prefetchProps.value)
}
Comment on lines +25 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the "commit" idea ❤️ I struggled with what to call this.


return { prefetchProps, commitPrefetchedProps }
}
Loading