From 4f7f9679b77d9504ea7dcdbac4b2ce6f5a6a6bde Mon Sep 17 00:00:00 2001 From: Craig Harshbarger Date: Thu, 23 Jan 2025 16:29:19 -0600 Subject: [PATCH 1/5] Add a failing test --- src/tests/routeProps.browser.spec.ts | 53 ++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 src/tests/routeProps.browser.spec.ts diff --git a/src/tests/routeProps.browser.spec.ts b/src/tests/routeProps.browser.spec.ts new file mode 100644 index 00000000..2e316f80 --- /dev/null +++ b/src/tests/routeProps.browser.spec.ts @@ -0,0 +1,53 @@ +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 props = vi.fn().mockImplementation(() => ({ prop: 'foo' })) + const setup = vi.fn() + + const route = createRoute({ + name: 'test', + path: '/[param]', + component: defineComponent({ + setup() { + setup() + + return { props } + }, + render() { + return h('div', {}, 'test') + }, + }), + }, props) + + const router = createRouter([route], { + initialUrl: '/bar', + }) + + const root = { + template: '', + } + + 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: 'foo' }) + + expect(setup).toHaveBeenCalledTimes(1) +}) From b426f660d746b30d035ae6835b7684d961735418 Mon Sep 17 00:00:00 2001 From: Craig Harshbarger Date: Sat, 25 Jan 2025 17:04:16 -0600 Subject: [PATCH 2/5] Fix components remounting issue while preserving the ability to pass the wrapped component to the slot --- src/components/echo.ts | 4 +- src/components/routerView.vue | 67 +++++----------- src/compositions/useComponentsStore.ts | 13 ++++ src/services/component.browser.spec.ts | 15 ++-- src/services/component.ts | 101 ++++++++++++------------- src/services/createComponentsStore.ts | 51 +++++++++++++ src/services/createRouter.ts | 3 + 7 files changed, 146 insertions(+), 108 deletions(-) create mode 100644 src/compositions/useComponentsStore.ts create mode 100644 src/services/createComponentsStore.ts diff --git a/src/components/echo.ts b/src/components/echo.ts index 3ea2457d..0d1a38ed 100644 --- a/src/components/echo.ts +++ b/src/components/echo.ts @@ -1,7 +1,7 @@ import { defineComponent } from 'vue' -export default defineComponent(({ value }) => { - return () => value +export default defineComponent((props) => { + return () => props.value }, { props: { value: { diff --git a/src/components/routerView.vue b/src/components/routerView.vue index 11f9ad81..dac3a0db 100644 --- a/src/components/routerView.vue +++ b/src/components/routerView.vue @@ -1,31 +1,30 @@ - - diff --git a/src/compositions/useComponentsStore.ts b/src/compositions/useComponentsStore.ts new file mode 100644 index 00000000..5e5a5c9c --- /dev/null +++ b/src/compositions/useComponentsStore.ts @@ -0,0 +1,13 @@ +import { inject } from 'vue' +import { RouterNotInstalledError } from '@/errors/routerNotInstalledError' +import { ComponentsStore, componentsStoreKey } from '@/services/createComponentsStore' + +export function useComponentsStore(): ComponentsStore { + const store = inject(componentsStoreKey) + + if (!store) { + throw new RouterNotInstalledError() + } + + return store +} diff --git a/src/services/component.browser.spec.ts b/src/services/component.browser.spec.ts index 3f74e027..7a9ac320 100644 --- a/src/services/component.browser.spec.ts +++ b/src/services/component.browser.spec.ts @@ -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' @@ -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: '/', @@ -38,10 +37,8 @@ test('renders component with async props', async () => { const route = createRoute({ name: 'echo', path: '/echo', - component: component(echo, async () => { - return { value: 'echo' } - }), - }) + component: echo, + }, async () => ({ value: 'echo' })) const router = createRouter([route], { initialUrl: '/', @@ -109,8 +106,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: '/', diff --git a/src/services/component.ts b/src/services/component.ts index 00a783db..3a944462 100644 --- a/src/services/component.ts +++ b/src/services/component.ts @@ -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 @@ -13,83 +13,82 @@ export type ComponentProps = TComponent extends Co ? T : {} -type ComponentPropsGetter = () => MaybePromise> - /** * 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 + * @returns A component that expects `props` to be passed in as a single prop value */ -export function component(component: TComponent, props: ComponentPropsGetter): Component { +export function createComponentPropsWrapper(component: Component): Component { return defineComponent({ name: 'PropsWrapper', expose: [], - setup() { - const values = props() + props: ['props'], + setup(props: { props: unknown }) { const instance = getCurrentInstance() return () => { - if (values instanceof Error) { + if (props.props instanceof Error) { return '' } - if (isPromise(values)) { + if (isPromise(props.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(suspenseAsyncPropsWrapper(component, values)) + return h(SuspenseAsyncComponentPropsWrapper, { component, props: props.props }) } - return h(asyncPropsWrapper(component, values)) + return h(AsyncComponentPropsWrapper, { component, props: props.props }) } - return h(component, values) + return h(component, props.props) } }, }) } -/** - * Creates a component wrapper that binds async props which does not require suspense - */ -function asyncPropsWrapper(component: TComponent, props: Promise>): Component { - return defineComponent({ - name: 'AsyncPropsWrapper', - expose: [], - setup() { - const values = ref() +const AsyncComponentPropsWrapper = defineComponent((input: { component: Component, props: unknown }) => { + const values = ref() - ;(async () => { - values.value = await props - })() + watch(() => input.props, async (props) => { + values.value = await props + }, { immediate: true, deep: true }) - return () => { - if (values.value instanceof Error) { - return '' - } + return () => { + if (values.value instanceof Error) { + return '' + } - if (values.value) { - return h(component, values.value) - } + if (values.value) { + return h(input.component, values.value) + } - return '' - } - }, - }) -} + return '' + } +}, { + props: ['component', 'props'], +}) -/** - * Creates a component wrapper that binds async props which requires suspense - */ -function suspenseAsyncPropsWrapper(component: TComponent, props: Promise>): Component { - return defineComponent({ - name: 'SuspenseAsyncPropsWrapper', - expose: [], - async setup() { - const values = await props +const SuspenseAsyncComponentPropsWrapper = defineComponent(async (input: { component: Component, props: unknown }) => { + const values = ref() - return () => h(component, values) - }, - }) -} + values.value = await input.props + + watch(() => values.value, async (props) => { + values.value = await props + }, { deep: true }) + + return () => { + if (values.value instanceof Error) { + return '' + } + + if (values.value) { + return h(input.component, values.value) + } + + return '' + } +}, { + props: ['component', 'props'], +}) diff --git a/src/services/createComponentsStore.ts b/src/services/createComponentsStore.ts new file mode 100644 index 00000000..f210f2e3 --- /dev/null +++ b/src/services/createComponentsStore.ts @@ -0,0 +1,51 @@ +import { Component, InjectionKey } from 'vue' +import { createComponentPropsWrapper } from './component' +import { CreatedRouteOptions } from '@/types/route' +import { isWithComponent, isWithComponents } from '@/types/createRouteOptions' +import RouterView from '@/components/routerView.vue' + +export type ComponentsStore = { + getRouteComponents: (match: CreatedRouteOptions) => Record, +} + +export const componentsStoreKey: InjectionKey = Symbol() + +export function createComponentsStore(): ComponentsStore { + const store = new Map>() + + const getRouteComponents: ComponentsStore['getRouteComponents'] = (match) => { + const existing = store.get(match.id) + + if (existing) { + return existing + } + + const components = getAllComponentsForMatch(match) + + store.set(match.id, components) + + return components + } + + return { + getRouteComponents, + } +} + +function getAllComponentsForMatch(options: CreatedRouteOptions): Record { + if (isWithComponents(options)) { + return wrapAllComponents(options.components) + } + + if (isWithComponent(options)) { + return wrapAllComponents({ default: options.component }) + } + + return { default: RouterView } +} + +function wrapAllComponents(components: Record): Record { + return Object.fromEntries( + Object.entries(components).map(([name, component]) => [name, createComponentPropsWrapper(component)]), + ) +} diff --git a/src/services/createRouter.ts b/src/services/createRouter.ts index a1e6cfb6..efccf3b7 100644 --- a/src/services/createRouter.ts +++ b/src/services/createRouter.ts @@ -31,6 +31,7 @@ import { RouterReject } from '@/types/routerReject' import { EmptyRouterPlugin, RouterPlugin } from '@/types/routerPlugin' import { getRoutesForRouter } from './getRoutesForRouter' import { getGlobalHooksForRouter } from './getGlobalHooksForRouter' +import { componentsStoreKey, createComponentsStore } from './createComponentsStore' type RouterUpdateOptions = { replace?: boolean, @@ -84,6 +85,7 @@ export function createRouter< const getNavigationId = createUniqueIdSequence() const propStore = createPropStore() + const componentsStore = createComponentsStore() const visibilityObserver = createVisibilityObserver() const history = createRouterHistory({ mode: options?.historyMode, @@ -312,6 +314,7 @@ export function createRouter< app.provide(routerRejectionKey, rejection) app.provide(routerHooksKey, hooks) app.provide(propStoreKey, propStore) + app.provide(componentsStoreKey, componentsStore) app.provide(visibilityObserverKey, visibilityObserver) // We cant technically guarantee that the user registered the same router that they installed From 8f2d7208d984f32c872863ddad2ad90f346a475f Mon Sep 17 00:00:00 2001 From: Craig Harshbarger Date: Sat, 25 Jan 2025 22:12:09 -0600 Subject: [PATCH 3/5] Move all props logic out of router view and into the props wrapper --- src/components/routerView.vue | 25 +++++-------------------- src/services/component.ts | 22 ++++++++++++++-------- src/services/createComponentsStore.ts | 8 ++++---- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/components/routerView.vue b/src/components/routerView.vue index dac3a0db..b5a36783 100644 --- a/src/components/routerView.vue +++ b/src/components/routerView.vue @@ -1,19 +1,13 @@