diff --git a/RESULT-optimization-vue-blocker-subscription.md b/RESULT-optimization-vue-blocker-subscription.md new file mode 100644 index 0000000000..fe71c6e131 --- /dev/null +++ b/RESULT-optimization-vue-blocker-subscription.md @@ -0,0 +1,89 @@ +# Shared Vue blocker subscription + +## Principle + +When two public entry points implement the same private state machine, keep the +semantic differences at their boundaries and share the identical interior. + +`useBlocker` and `` previously duplicated location conversion, resolver +state, history registration, cleanup, and navigation settlement. They now use +one private implementation. Boundary adapters preserve the hook's fixed option +snapshot and callback receiver while `` continues to read reactive props +and resubscribe. + +## Scope and compatibility + +- Public exports, overloads, props, and return types are unchanged. +- `useBlocker` still captures a fixed normalized option snapshot. +- `` still tracks reactive options and unsubscribes before resubscribing. +- Hook callbacks still receive `undefined` as `this`; `` callbacks retain + their existing options-object receiver. +- Defaults still apply only to `undefined`; explicit `null` values pass through. +- History access, 404-to-valid-route bypass, resolver publication/reset, + unsubscribe timing, error propagation, and pending-promise behavior are + unchanged. + +The production change is confined to `packages/vue-router/src/useBlocker.tsx`. + +## Bundle-size result + +Fresh paired full-matrix artifacts: + +- exact base `697ebb6ddbd433d052b6b4707938a5c595865d58`: + `/private/tmp/vue-blocker-final-control-full.json` +- clean final candidate `3c7c0184098880317fcc457a8d26fab51dcd6639`: + `/private/tmp/vue-blocker-final-full.json` + +Both runs used offline frozen installs with scripts disabled. All seven direct +benchmark `@tanstack` links and Vue Router's package-internal workspace links +were verified to resolve inside their intended worktrees before measurement. + +| Scenario | Raw | Initial gzip | Gzip | Brotli | +| ----------------- | -----: | -----------: | -----: | -----: | +| `vue-router.full` | -765 B | -148 B | -148 B | -29 B | +| `vue-start.full` | -765 B | -164 B | -163 B | -47 B | + +The other fifteen scenarios are byte-identical across raw, initial gzip, gzip, +and Brotli. In particular, `vue-router.minimal` remains identical because it +does not retain both blocker entry points. This confirms that the shared helper +does not leak across the tree-shaking boundary. + +An independent clean `vue-router.full` attribution run at production commit +`08f4d624b60901ff0f3250cec42cf7290641f87c` reproduced the same `-765 B` raw, +`-148 B` gzip, and `-29 B` Brotli result in +`/private/tmp/vue-blocker-reviewed.json`. + +## Runtime validation + +An emitted-code benchmark compared the exact base with production commit +`08f4d624b60901ff0f3250cec42cf7290641f87c` using real Vue refs, computed +values, effects, scopes, and ticks. Correctness assertions covered location and +param conversion, the 404 bypass, hook versus component callback receivers, +registration, cleanup, and reactive resubscription. + +Two independent full runs used 8 paired warmups and 96 measured AB/BA samples +per case. The synchronous-false invocation path was approximately 2.8-3.3% +faster for the hook and 2.4% faster for ``. Promise-false paths were +neutral to faster, and resubscription was neutral. + +The only noisy case, `` setup, received two 160-pair confirmation runs. +Their geometric deltas were `+0.36%` (95% CI `-1.14..+1.89`) and `+0.20%` +(95% CI `-1.54..+1.96`), with paired medians converging toward zero. No tested +path showed a reproducible candidate-slower direction. + +Full method, distributions, emitted-code hashes, script, and logs are under +`/private/tmp/vue-blocker-runtime-697ebb6d-08f4d624b6/`. + +## Validation + +- focused blocker suite: 14 passed, no Vitest type errors +- full Vue Router unit suite: 54 files, 818 passed, 1 skipped, no type errors +- Vue Router type suite: 17 files, 138 passed, no type errors +- Vue Router ESLint: 0 errors; 79 pre-existing warnings +- full 17-scenario bundle matrix: passed +- formatting and `git diff --check`: passed + +Focused tests cover both entry points, `blocked -> reset/proceed -> idle` +resolver lifecycles, callback receivers, `null` versus `undefined` defaults, +reactive option changes, unsubscribe-before-resubscribe ordering, and unmount +cleanup. diff --git a/packages/vue-router/src/useBlocker.tsx b/packages/vue-router/src/useBlocker.tsx index 17eef20f7a..f945023521 100644 --- a/packages/vue-router/src/useBlocker.tsx +++ b/packages/vue-router/src/useBlocker.tsx @@ -128,42 +128,35 @@ function _resolveBlockerOpts( } } -export function useBlocker< - TRouter extends AnyRouter = RegisteredRouter, - TWithResolver extends boolean = false, ->( - opts: UseBlockerOpts, -): TWithResolver extends true ? Vue.Ref> : void - -/** - * @deprecated Use the shouldBlockFn property instead - */ -export function useBlocker( - blockerFnOrOpts?: LegacyBlockerOpts, -): Vue.Ref - -/** - * @deprecated Use the UseBlockerOpts object syntax instead - */ -export function useBlocker( - blockerFn?: LegacyBlockerFn, - condition?: boolean | any, -): Vue.Ref - -export function useBlocker( - opts?: UseBlockerOpts | LegacyBlockerOpts | LegacyBlockerFn, - condition?: boolean | any, -): Vue.Ref | void { - const { - shouldBlockFn, - enableBeforeUnload = true, - disabled = false, - withResolver = false, - } = _resolveBlockerOpts(opts, condition) +function getBlockerLocation( + router: AnyRouter, + location: HistoryLocation, +): AnyShouldBlockFnLocation { + const parsedLocation = router.parseLocation(location) + const [, rawParams, foundRoute] = router.getMatchedRoutes( + parsedLocation.pathname, + ) + if (foundRoute === undefined) { + return { + routeId: '__notFound__', + fullPath: parsedLocation.pathname, + pathname: parsedLocation.pathname, + params: rawParams, + search: parsedLocation.search, + } + } + return { + routeId: foundRoute.id, + fullPath: foundRoute.fullPath, + pathname: parsedLocation.pathname, + params: rawParams, + search: parsedLocation.search, + } +} +function useBlockerImpl(getArgs: () => UseBlockerOpts) { const router = useRouter() - const { history } = router - + const history = router.history const resolver = Vue.ref({ status: 'idle', current: undefined, @@ -174,34 +167,14 @@ export function useBlocker( }) Vue.watchEffect((onCleanup) => { - const blockerFnComposed = async (blockerFnArgs: BlockerFnArgs) => { - function getLocation( - location: HistoryLocation, - ): AnyShouldBlockFnLocation { - const parsedLocation = router.parseLocation(location) - const [, rawParams, foundRoute] = router.getMatchedRoutes( - parsedLocation.pathname, - ) - if (foundRoute === undefined) { - return { - routeId: '__notFound__', - fullPath: parsedLocation.pathname, - pathname: parsedLocation.pathname, - params: rawParams, - search: parsedLocation.search, - } - } - return { - routeId: foundRoute.id, - fullPath: foundRoute.fullPath, - pathname: parsedLocation.pathname, - params: rawParams, - search: parsedLocation.search, - } - } + const args = getArgs() + if (args.disabled) { + return + } - const current = getLocation(blockerFnArgs.currentLocation) - const next = getLocation(blockerFnArgs.nextLocation) + const blockerFnComposed = async (blockerFnArgs: BlockerFnArgs) => { + const current = getBlockerLocation(router, blockerFnArgs.currentLocation) + const next = getBlockerLocation(router, blockerFnArgs.nextLocation) // Allow navigation away from 404 pages to valid routes if ( @@ -211,12 +184,12 @@ export function useBlocker( return false } - const shouldBlock = await shouldBlockFn({ + const shouldBlock = await args.shouldBlockFn({ action: blockerFnArgs.action, current, next, }) - if (!withResolver) { + if (!args.withResolver) { return shouldBlock } @@ -248,21 +221,63 @@ export function useBlocker( return canNavigateAsync } - if (disabled) { - return - } - const unsubscribe = history.block({ blockerFn: blockerFnComposed, - enableBeforeUnload, + enableBeforeUnload: args.enableBeforeUnload, }) onCleanup(() => { - if (unsubscribe) unsubscribe() + if (unsubscribe) { + unsubscribe() + } }) }) - return withResolver ? resolver : undefined + return resolver +} + +export function useBlocker< + TRouter extends AnyRouter = RegisteredRouter, + TWithResolver extends boolean = false, +>( + opts: UseBlockerOpts, +): TWithResolver extends true ? Vue.Ref> : void + +/** + * @deprecated Use the shouldBlockFn property instead + */ +export function useBlocker( + blockerFnOrOpts?: LegacyBlockerOpts, +): Vue.Ref + +/** + * @deprecated Use the UseBlockerOpts object syntax instead + */ +export function useBlocker( + blockerFn?: LegacyBlockerFn, + condition?: boolean | any, +): Vue.Ref + +export function useBlocker( + opts?: UseBlockerOpts | LegacyBlockerOpts | LegacyBlockerFn, + condition?: boolean | any, +): Vue.Ref | void { + const { + shouldBlockFn, + enableBeforeUnload = true, + disabled = false, + withResolver = false, + } = _resolveBlockerOpts(opts, condition) + // useBlocker callbacks historically receive no `this`; callbacks do. + const args: UseBlockerOpts = { + shouldBlockFn: (blockerArgs) => shouldBlockFn(blockerArgs), + enableBeforeUnload, + disabled, + withResolver, + } + const resolver = useBlockerImpl(() => args) + + return args.withResolver ? resolver : undefined } const _resolvePromptBlockerArgs = ( @@ -350,109 +365,7 @@ const BlockImpl = Vue.defineComponent({ } }) - // Use a reactive useBlocker that re-subscribes when args change - const router = useRouter() - const { history } = router - - const resolver = Vue.ref({ - status: 'idle', - current: undefined, - next: undefined, - action: undefined, - proceed: undefined, - reset: undefined, - }) - - Vue.watchEffect((onCleanup) => { - const args = blockerArgs.value - - if (args.disabled) { - return - } - - const blockerFnComposed = async (blockerFnArgs: BlockerFnArgs) => { - function getLocation( - location: HistoryLocation, - ): AnyShouldBlockFnLocation { - const parsedLocation = router.parseLocation(location) - const [, rawParams, foundRoute] = router.getMatchedRoutes( - parsedLocation.pathname, - ) - if (foundRoute === undefined) { - return { - routeId: '__notFound__', - fullPath: parsedLocation.pathname, - pathname: parsedLocation.pathname, - params: rawParams, - search: parsedLocation.search, - } - } - return { - routeId: foundRoute.id, - fullPath: foundRoute.fullPath, - pathname: parsedLocation.pathname, - params: rawParams, - search: parsedLocation.search, - } - } - - const current = getLocation(blockerFnArgs.currentLocation) - const next = getLocation(blockerFnArgs.nextLocation) - - // Allow navigation away from 404 pages to valid routes - if ( - current.routeId === '__notFound__' && - next.routeId !== '__notFound__' - ) { - return false - } - - const shouldBlock = await args.shouldBlockFn({ - action: blockerFnArgs.action, - current, - next, - }) - if (!args.withResolver) { - return shouldBlock - } - - if (!shouldBlock) { - return false - } - - const promise = new Promise((resolve) => { - resolver.value = { - status: 'blocked', - current, - next, - action: blockerFnArgs.action, - proceed: () => resolve(false), - reset: () => resolve(true), - } - }) - - const canNavigateAsync = await promise - resolver.value = { - status: 'idle', - current: undefined, - next: undefined, - action: undefined, - proceed: undefined, - reset: undefined, - } - - return canNavigateAsync - } - - const unsubscribe = history.block({ - blockerFn: blockerFnComposed, - enableBeforeUnload: args.enableBeforeUnload, - }) - - onCleanup(() => { - if (unsubscribe) unsubscribe() - }) - }) + const resolver = useBlockerImpl(() => blockerArgs.value) return () => { const defaultSlot = slots.default diff --git a/packages/vue-router/tests/useBlocker.test.tsx b/packages/vue-router/tests/useBlocker.test.tsx index 374ce2906e..72f113d1cc 100644 --- a/packages/vue-router/tests/useBlocker.test.tsx +++ b/packages/vue-router/tests/useBlocker.test.tsx @@ -1,11 +1,18 @@ import '@testing-library/jest-dom/vitest' import { afterEach, describe, expect, test, vi } from 'vitest' -import { cleanup, fireEvent, render, screen } from '@testing-library/vue' +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/vue' import * as Vue from 'vue' import { z } from 'zod' import { Block, + Outlet, RouterProvider, createMemoryHistory, createRootRoute, @@ -180,7 +187,11 @@ describe('useBlocker', () => { test('gives correct arguments to shouldBlockFn', async () => { const rootRoute = createRootRoute() - const shouldBlockFn = vi.fn().mockReturnValue(true) + let receiver: unknown = 'not called' + const shouldBlockFn = vi.fn(function (this: unknown) { + receiver = this + return true + }) const IndexComponent = () => { const navigate = useNavigate() @@ -231,6 +242,7 @@ describe('useBlocker', () => { ).toBeInTheDocument() expect(window.location.pathname).toBe('/') + expect(receiver).toBeUndefined() expect(shouldBlockFn).toHaveBeenCalledWith({ action: 'REPLACE', @@ -425,11 +437,336 @@ describe('useBlocker', () => { expect(window.location.pathname).toBe('/invoices') }) + test('defaults only undefined hook options', async () => { + const rootRoute = createRootRoute() + const history = createMemoryHistory() + const block = vi.spyOn(history, 'block') + const results: Array = [] + + const IndexComponent = Vue.defineComponent({ + setup() { + results.push( + useBlocker({ + shouldBlockFn: () => false, + enableBeforeUnload: undefined, + disabled: undefined, + withResolver: undefined, + }), + useBlocker({ + shouldBlockFn: () => false, + enableBeforeUnload: null, + disabled: null, + withResolver: null, + } as any), + ) + + return () =>

Index

+ }, + }) + + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: IndexComponent, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute]), + history, + }) + + render() + + expect(await screen.findByRole('heading', { name: 'Index' })).toBeVisible() + expect(results).toEqual([undefined, undefined]) + expect( + block.mock.calls.map(([options]) => options.enableBeforeUnload), + ).toEqual([true, null]) + }) + + test.each(['useBlocker', ''] as const)( + '%s resolver returns to idle after reset and proceed', + async (surface) => { + const history = createMemoryHistory({ initialEntries: ['/'] }) + const renderResolver = (resolver: { + status: 'idle' | 'blocked' + reset?: () => void + proceed?: () => void + }) => ( + <> + {resolver.status} + {resolver.status === 'blocked' && ( + <> + + + + )} + + ) + + const RootComponent = Vue.defineComponent({ + setup() { + const navigate = useNavigate() + const renderNavigation = () => ( + <> + + + + ) + + if (surface === 'useBlocker') { + const blocker = useBlocker({ + shouldBlockFn: () => true, + withResolver: true, + }) + + return () => ( + <> + {renderResolver(blocker.value)} + {renderNavigation()} + + ) + } + + return () => ( + <> + true} + withResolver={true} + children={renderResolver} + /> + {renderNavigation()} + + ) + }, + }) + + const rootRoute = createRootRoute({ component: RootComponent }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>

Index

, + }) + const postsRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + component: () =>

Posts

, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, postsRoute]), + history, + }) + + render() + + const postsButton = await screen.findByRole('button', { name: 'Posts' }) + expect(screen.getByTestId('blocker-status')).toHaveTextContent('idle') + + await fireEvent.click(postsButton) + await waitFor(() => { + expect(screen.getByTestId('blocker-status')).toHaveTextContent( + 'blocked', + ) + }) + expect(history.location.pathname).toBe('/') + + await fireEvent.click( + screen.getByRole('button', { name: 'Reset blocker' }), + ) + await waitFor(() => { + expect(screen.getByTestId('blocker-status')).toHaveTextContent('idle') + }) + expect(history.location.pathname).toBe('/') + expect(screen.getByRole('heading', { name: 'Index' })).toBeVisible() + + await fireEvent.click(postsButton) + await waitFor(() => { + expect(screen.getByTestId('blocker-status')).toHaveTextContent( + 'blocked', + ) + }) + + await fireEvent.click( + screen.getByRole('button', { name: 'Proceed blocker' }), + ) + expect( + await screen.findByRole('heading', { name: 'Posts' }), + ).toBeVisible() + expect(screen.getByTestId('blocker-status')).toHaveTextContent('idle') + expect(history.location.pathname).toBe('/posts') + }, + ) + + test(' resubscribes to reactive options and cleans up in order', async () => { + const history = createMemoryHistory({ initialEntries: ['/'] }) + const nextHistory = createMemoryHistory({ initialEntries: ['/posts'] }) + const firstShouldBlock = vi.fn(() => false) + const secondShouldBlock = vi.fn(() => true) + const shouldBlockFn = Vue.ref(firstShouldBlock) + const enableBeforeUnload = Vue.ref boolean)>(true) + const withResolver = Vue.ref(false) + const events: Array = [] + let nextSubscriptionId = 0 + const actualBlock = history.block.bind(history) + const block = vi.spyOn(history, 'block').mockImplementation((options) => { + const subscriptionId = ++nextSubscriptionId + events.push(`subscribe:${subscriptionId}`) + const unsubscribe = actualBlock(options) + + return () => { + events.push(`unsubscribe:${subscriptionId}`) + unsubscribe() + } + }) + + const IndexComponent = Vue.defineComponent({ + setup() { + return () => ( + <> + ( + <> + + {resolver.status} + + {resolver.status === 'blocked' && ( + <> + + + + )} + + )} + /> +

Index

+ + ) + }, + }) + + const rootRoute = createRootRoute() + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: IndexComponent, + }) + const postsRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + component: () =>

Posts

, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, postsRoute]), + history, + }) + + const view = render() + const blockerArgs = { + action: 'PUSH' as const, + currentLocation: history.location, + nextLocation: nextHistory.location, + } + + expect(await screen.findByRole('heading', { name: 'Index' })).toBeVisible() + await waitFor(() => expect(block).toHaveBeenCalledTimes(1)) + + await expect(block.mock.calls[0]![0].blockerFn(blockerArgs)).resolves.toBe( + false, + ) + expect(firstShouldBlock).toHaveBeenCalledTimes(1) + + shouldBlockFn.value = secondShouldBlock + await waitFor(() => expect(block).toHaveBeenCalledTimes(2)) + await expect(block.mock.calls[1]![0].blockerFn(blockerArgs)).resolves.toBe( + true, + ) + expect(secondShouldBlock).toHaveBeenCalledTimes(1) + + enableBeforeUnload.value = false + await waitFor(() => expect(block).toHaveBeenCalledTimes(3)) + + withResolver.value = true + await waitFor(() => expect(block).toHaveBeenCalledTimes(4)) + + const resetResult = block.mock.calls[3]![0].blockerFn(blockerArgs) + await waitFor(() => { + expect(screen.getByTestId('reactive-blocker-status')).toHaveTextContent( + 'blocked', + ) + }) + await fireEvent.click( + screen.getByRole('button', { name: 'Reset reactive blocker' }), + ) + await expect(resetResult).resolves.toBe(true) + await waitFor(() => { + expect(screen.getByTestId('reactive-blocker-status')).toHaveTextContent( + 'idle', + ) + }) + + const proceedResult = block.mock.calls[3]![0].blockerFn(blockerArgs) + await waitFor(() => { + expect(screen.getByTestId('reactive-blocker-status')).toHaveTextContent( + 'blocked', + ) + }) + await fireEvent.click( + screen.getByRole('button', { name: 'Proceed reactive blocker' }), + ) + await expect(proceedResult).resolves.toBe(false) + await waitFor(() => { + expect(screen.getByTestId('reactive-blocker-status')).toHaveTextContent( + 'idle', + ) + }) + expect(secondShouldBlock).toHaveBeenCalledTimes(3) + + expect( + block.mock.calls.map(([options]) => options.enableBeforeUnload), + ).toEqual([true, true, false, false]) + expect(events).toEqual([ + 'subscribe:1', + 'unsubscribe:1', + 'subscribe:2', + 'unsubscribe:2', + 'subscribe:3', + 'unsubscribe:3', + 'subscribe:4', + ]) + + view.unmount() + + expect(events).toEqual([ + 'subscribe:1', + 'unsubscribe:1', + 'subscribe:2', + 'unsubscribe:2', + 'subscribe:3', + 'unsubscribe:3', + 'subscribe:4', + 'unsubscribe:4', + ]) + }) + test(' disabled property is reactive', async () => { const rootRoute = createRootRoute() // Use a shared reactive ref for the disabled state const disabled = Vue.ref(false) + let receiver: unknown = 'not called' + const shouldBlockFn = vi.fn(function (this: unknown) { + receiver = this + return true + }) const IndexComponent = Vue.defineComponent({ setup() { @@ -437,7 +774,7 @@ describe('useBlocker', () => { return () => ( <> - true} disabled={disabled.value} /> +

Index

@@ -472,20 +809,22 @@ describe('useBlocker', () => { let postsButton = await screen.findByRole('button', { name: 'Posts' }) - fireEvent.click(postsButton) + await fireEvent.click(postsButton) expect( await screen.findByRole('heading', { name: 'Index' }), ).toBeInTheDocument() expect(window.location.pathname).toBe('/') + expect(receiver).toMatchObject({ shouldBlockFn }) // Update the shared ref - Vue's reactivity will propagate the change disabled.value = true + await Vue.nextTick() postsButton = await screen.findByRole('button', { name: 'Posts' }) - fireEvent.click(postsButton) + await fireEvent.click(postsButton) expect( await screen.findByRole('heading', { name: 'Posts' }),