From 67fee5f90bfb7e82884c599fde3d7d75c35d3ff9 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Tue, 20 Jun 2023 17:52:04 +0200 Subject: [PATCH] fix: null should be preserved in relative navigations The fix is a bit more complicated that I anticipated, I will come back to this later on as the currently documented version works perfectly. - the nullish params are removed before being passed to the matcher - The encodeParam function transform null into '' - The applyToParams also works with arrays but it makes no sense to allow null in array params Ideally, I would make the matcher a bit more permissive so the encoding is kept at the router level. I think the matcher sholud be responsible for removing the nullish parameters but that also means the encode function should leave nullish values untouched. We might need an intermediate Type for this shape of Params, it gets a little bit tedious in terms of types, so I would like to avoid adding more types. Close #1893 --- packages/router/__tests__/router.spec.ts | 13 +++++++++++ packages/router/src/encoding.ts | 29 ++++++++++++++++++++---- packages/router/src/utils/index.ts | 10 +++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/packages/router/__tests__/router.spec.ts b/packages/router/__tests__/router.spec.ts index a60db5393..50d60f039 100644 --- a/packages/router/__tests__/router.spec.ts +++ b/packages/router/__tests__/router.spec.ts @@ -331,6 +331,19 @@ describe('Router', () => { expect(router.currentRoute.value.params).toEqual({}) }) + it('removes null/undefined optional params when current location has it on relative navigations', async () => { + const { router } = await newRouter() + const withParam = router.resolve({ name: 'optional', params: { p: 'a' } }) + const implicitNull = router.resolve({ params: { p: null } }, withParam) + const implicitUndefined = router.resolve( + { params: { p: undefined } }, + withParam + ) + + expect(implicitNull.params).toEqual({}) + expect(implicitUndefined.params).toEqual({}) + }) + it('keeps empty strings in optional params', async () => { const { router } = await newRouter() const route1 = router.resolve({ name: 'optional', params: { p: '' } }) diff --git a/packages/router/src/encoding.ts b/packages/router/src/encoding.ts index e46707080..75d76518f 100644 --- a/packages/router/src/encoding.ts +++ b/packages/router/src/encoding.ts @@ -1,3 +1,4 @@ +import { assign } from './utils' import { warn } from './warning' /** @@ -120,14 +121,34 @@ export function encodePath(text: string | number): string { /** * Encode characters that need to be encoded on the path section of the URL as a * param. This function encodes everything {@link encodePath} does plus the - * slash (`/`) character. If `text` is `null` or `undefined`, returns an empty - * string instead. + * slash (`/`) character. If `text` is `null` or `undefined`, it keeps the value as is. * * @param text - string to encode * @returns encoded string */ -export function encodeParam(text: string | number | null | undefined): string { - return text == null ? '' : encodePath(text).replace(SLASH_RE, '%2F') +export function encodeParam( + text: string | number | null | undefined +): string | null | undefined { + return text == null ? text : encodePath(text).replace(SLASH_RE, '%2F') +} + +/** + * Remove nullish values from an object. This function creates a copy of the object. Used for params and query. + * + * @param obj - plain object to remove nullish values from + * @returns a new object with only defined values + */ +export function withoutNullishValues( + obj: Record +): Record { + const targetParams = assign({}, obj) + for (const key in targetParams) { + if (targetParams[key] == null) { + delete targetParams[key] + } + } + + return targetParams } /** diff --git a/packages/router/src/utils/index.ts b/packages/router/src/utils/index.ts index af90cb5ce..4b04067ce 100644 --- a/packages/router/src/utils/index.ts +++ b/packages/router/src/utils/index.ts @@ -13,10 +13,18 @@ export function isESModule(obj: any): obj is { default: RouteComponent } { export const assign = Object.assign +export function applyToParams( + fn: (v: string | number | null | undefined) => string | null | undefined, + params: RouteParamsRaw | undefined +): RouteParamsRaw export function applyToParams( fn: (v: string | number | null | undefined) => string, params: RouteParamsRaw | undefined -): RouteParams { +): RouteParams +export function applyToParams( + fn: (v: string | number | null | undefined) => string | null | undefined, + params: RouteParamsRaw | undefined +): RouteParams | RouteParamsRaw { const newParams: RouteParams = {} for (const key in params) {