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

[1st Attempt] Fix route components remounting rather than being updated when props change #439

Closed
wants to merge 3 commits 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
4 changes: 2 additions & 2 deletions src/components/echo.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { defineComponent } from 'vue'

export default defineComponent(({ value }) => {
return () => value
export default defineComponent((props) => {
return () => props.value
}, {
props: {
value: {
Expand Down
36 changes: 15 additions & 21 deletions src/components/routerView.vue
Original file line number Diff line number Diff line change
@@ -1,28 +1,27 @@
<template>
<template v-if="component">
<slot name="default" v-bind="{ route, component, rejection }">
<component :is="component" />
<template v-if="rejection">
<component :is="rejection.component" />
</template>
<template v-else>
<ComponentPropsWrapper :component="component" :props="props" />
</template>
</slot>
</template>
</template>

<script lang="ts">
/**
* @ignore
*/
</script>

<script lang="ts" setup>
import { Component, UnwrapRef, VNode, computed, provide, resolveComponent } from 'vue'
import { usePropStore } from '@/compositions/usePropStore'
import { useRejection } from '@/compositions/useRejection'
import { useRoute } from '@/compositions/useRoute'
import { useRouterDepth } from '@/compositions/useRouterDepth'
import { component as componentUtil } from '@/services/component'
import { RouterRejection } from '@/services/createRouterReject'
import { RouterRoute } from '@/services/createRouterRoute'
import { CreateRouteOptions, isWithComponent, isWithComponents } from '@/types/createRouteOptions'
import { depthInjectionKey } from '@/types/injectionDepth'
import { ComponentPropsWrapper } from '@/services/component'

const { name = 'default' } = defineProps<{
name?: string,
Expand All @@ -45,29 +44,24 @@

provide(depthInjectionKey, depth + 1)

const component = computed(() => {
if (rejection.value) {
return rejection.value.component
}

const props = computed(() => {
const match = route.matches.at(depth)

if (!match) {
return null
}

const component = getComponent(match)
const props = getProps(match.id, name, route)
return getProps(match.id, name, route)
})

const component = computed(() => {
const match = route.matches.at(depth)

if (!component) {
if (!match) {
return null
}

if (props) {
return componentUtil(component, () => props)
}

return component
return getComponent(match)
})

function getComponent(match: CreateRouteOptions): Component | undefined {
Expand Down
15 changes: 7 additions & 8 deletions src/services/component.browser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { flushPromises, mount } from '@vue/test-utils'
import { expect, test } from 'vitest'
import echo from '@/components/echo'
import { component } from '@/services/component'
import { createRoute } from '@/services/createRoute'
import { createRouter } from '@/services/createRouter'
import { h, defineComponent, getCurrentInstance } from 'vue'
Expand All @@ -10,8 +9,8 @@ test('renders component with sync props', async () => {
const route = createRoute({
name: 'echo',
path: '/echo',
component: component(echo, () => ({ value: 'echo' })),
})
component: echo,
}, () => ({ value: 'echo' }))

const router = createRouter([route], {
initialUrl: '/',
Expand All @@ -38,9 +37,9 @@ test('renders component with async props', async () => {
const route = createRoute({
name: 'echo',
path: '/echo',
component: component(echo, async () => {
return { value: 'echo' }
}),
component: echo,
}, async () => {
return { value: 'echo' }
})

const router = createRouter([route], {
Expand Down Expand Up @@ -109,8 +108,8 @@ test('renders component with async props using suspense', async () => {
const route = createRoute({
name: 'home',
path: '/',
component: component(echo, () => promise),
})
component: echo,
}, () => promise)

const router = createRouter([route], {
initialUrl: '/',
Expand Down
136 changes: 64 additions & 72 deletions src/services/component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable vue/require-prop-types */
/* eslint-disable vue/one-component-per-file */
import { AsyncComponentLoader, Component, FunctionalComponent, defineComponent, getCurrentInstance, h, ref } from 'vue'
import { MaybePromise } from '@/types/utilities'
import { AsyncComponentLoader, Component, FunctionalComponent, defineComponent, getCurrentInstance, h, ref, watch } from 'vue'
import { isPromise } from '@/utilities/promises'

type Constructor = new (...args: any) => any
Expand All @@ -13,83 +13,75 @@ export type ComponentProps<TComponent extends Component> = TComponent extends Co
? T
: {}

type ComponentPropsGetter<TComponent extends Component> = () => MaybePromise<ComponentProps<TComponent>>

/**
* Creates a component wrapper which has no props itself but mounts another component within while binding its props
*
* @param component The component to mount
* @param props A callback that returns the props or attributes to bind to the component
* @returns A component
*/
export function component<TComponent extends Component>(component: TComponent, props: ComponentPropsGetter<TComponent>): Component {
return defineComponent({
name: 'PropsWrapper',
expose: [],
setup() {
const values = props()
const instance = getCurrentInstance()

return () => {
if (values instanceof Error) {
return ''
}

if (isPromise(values)) {
// @ts-expect-error there isn't a way to check if suspense is used in the component without accessing a private property
if (instance?.suspense) {
return h(suspenseAsyncPropsWrapper(component, values))
}

return h(asyncPropsWrapper(component, values))
}

return h(component, values)
}
},
})
}

/**
* Creates a component wrapper that binds async props which does not require suspense
*/
function asyncPropsWrapper<TComponent extends Component>(component: TComponent, props: Promise<ComponentProps<TComponent>>): Component {
return defineComponent({
name: 'AsyncPropsWrapper',
expose: [],
setup() {
const values = ref()

;(async () => {
values.value = await props
})()

return () => {
if (values.value instanceof Error) {
return ''
}

if (values.value) {
return h(component, values.value)
}

return ''
export const ComponentPropsWrapper = defineComponent((input: { component: Component, props: unknown }) => {
const instance = getCurrentInstance()

return () => {
if (input.props instanceof Error) {
return ''
}

if (isPromise(input.props)) {
// @ts-expect-error there isn't a way to check if suspense is used in the component without accessing a private property
if (instance?.suspense) {
return h(SuspenseAsyncComponentPropsWrapper, { component: input.component, props: input.props })
}
},
})
}

/**
* Creates a component wrapper that binds async props which requires suspense
*/
function suspenseAsyncPropsWrapper<TComponent extends Component>(component: TComponent, props: Promise<ComponentProps<TComponent>>): Component {
return defineComponent({
name: 'SuspenseAsyncPropsWrapper',
expose: [],
async setup() {
const values = await props

return () => h(component, values)
},
})
}
return h(AsyncComponentPropsWrapper, { component: input.component, props: input.props })
}

return h(input.component, input.props)
}
}, {
props: ['component', 'props'],
})

const AsyncComponentPropsWrapper = defineComponent((input: { component: Component, props: unknown }) => {
const values = ref()

watch(() => input.props, async (props) => {
values.value = await props
}, { immediate: true, deep: true })

return () => {
if (values.value instanceof Error) {
return ''
}

if (values.value) {
return h(input.component, values.value)
}

return ''
}
}, {
props: ['component', 'props'],
})

const SuspenseAsyncComponentPropsWrapper = defineComponent(async (input: { component: Component, props: unknown }) => {
const values = ref()

values.value = await input.props
Copy link
Contributor

Choose a reason for hiding this comment

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

So the suspense version is different because it uses top-level await for props, right?

And we only use this version when we know that the current instance has a Vue Suspense component above?


watch(() => values.value, async (props) => {
values.value = await props
}, { deep: true })

return () => {
if (values.value) {
return h(input.component, values.value)
}

return ''
}
}, {
props: ['component', 'props'],
})
48 changes: 48 additions & 0 deletions src/tests/routeProps.browser.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { vi, test, expect } from 'vitest'
import { createRoute } from '@/services/createRoute'
import { createRouter } from '@/services/createRouter'
import { defineComponent, h } from 'vue'
import { mount } from '@vue/test-utils'

test('components are not remounted when props change', async () => {
const setup = vi.fn()

const route = createRoute({
name: 'test',
path: '/[param]',
component: defineComponent({
setup,
render() {
return h('div', {}, 'test')
},
}),
}, () => ({ prop: 'foo' }))

const router = createRouter([route], {
initialUrl: '/bar',
})

const root = {
template: '<RouterView />',
}

mount(root, {
global: {
plugins: [router],
},
})

await router.start()

await router.route.update({ param: 'foo' })

expect(setup).toHaveBeenCalledTimes(1)

await router.route.update({ param: 'bar' })

expect(setup).toHaveBeenCalledTimes(1)

await router.route.update({ param: 'baz' })

expect(setup).toHaveBeenCalledTimes(1)
})
Loading