Skip to content

Commit

Permalink
fix: null should be preserved in relative navigations
Browse files Browse the repository at this point in the history
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
  • Loading branch information
posva committed Dec 15, 2023
1 parent b298d56 commit c93e259
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
13 changes: 13 additions & 0 deletions packages/router/__tests__/router.spec.ts
Expand Up @@ -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: '' } })
Expand Down
29 changes: 25 additions & 4 deletions packages/router/src/encoding.ts
@@ -1,3 +1,4 @@
import { assign } from './utils'
import { warn } from './warning'

/**
Expand Down Expand Up @@ -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<string, unknown>
): Record<string, unknown> {
const targetParams = assign({}, obj)
for (const key in targetParams) {
if (targetParams[key] == null) {
delete targetParams[key]
}
}

return targetParams
}

/**
Expand Down
10 changes: 9 additions & 1 deletion packages/router/src/utils/index.ts
Expand Up @@ -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) {
Expand Down

0 comments on commit c93e259

Please sign in to comment.