Changed behaviour for optional params

[bodograumann]

Reproduction

https://github.com/vuejs/router/blob/main/packages/router/__tests__/router.spec.ts#L300-L328

Steps to reproduce the bug

• Route with an optional parameter
• Set the parameter
• Unset the parameter with undefined

Expected behavior

I would have expected the parameter to be unset as in versions before 4.2, but I know this might not have been the intended behaviour.

Actual behavior

This issue is about the changes in #1814

It is a bit of a mess, because several things come together.

1. The documentation says undefined and null are stringified: 0c91292
2. Before the change they where not stringified, but could actually be used to unset optional params in the current route.
3. After the change they are actually removed instead of stringified.

Additional information

Whatever the original intented use was, the use-case of unsetting the value of an optional parameter was never properly documented.
In practice using undefined worked and was used by us and others, so the change is causing breakages in real-world usages.

I checked and unsetting an optional parameter with "" did work at least in 4.1.6, but I am not sure if it also did in earlier versions, when we first implemented this code.

Imho using the empty string as "not set" is bad practice and they should be normalized to undefined instead.

[posva]

I added a test but that seems to be working. If that's not what you meant a boiled down reproduction or a failing test would help. I updated the docs too

[bodograumann]

Ok I saw it and am a bit confused how that works. Let me look into it.

Also wanted to note about the docs again, but seems you were faster 😆

[bodograumann]

The issue occurs when you only want to update the params of the current route, without changing the name:

  it('removes null/undefined params when current location has it', async () => {
    const { router } = await newRouter()

    await router.push({ name: 'optional', params: { p: 'a' } })
    await router.push({ params: { p: null } })
    expect(router.currentRoute.value.params).toEqual({})

    await router.push({ name: 'optional', params: { p: 'a' } })
    await router.push({ params: { p: undefined } })
    expect(router.currentRoute.value.params).toEqual({})
  })

[posva]

Ah I see, the behavior should be consistent:

• router.push({ name: 'optional', params: { p: null }})
• router.push({ params: { p: null }})

No matter the current location.

The actual fix is more complicated than it seems because it requires some refactoring so I recommend you to use the documented approach for the moment of using a param with an empty string to remove it

[bodograumann]

Ack, we are implementing the workaround right now.
Glad that this is getting fxed.