Skip to content

Conversation

@aa-ndrej
Copy link
Contributor

@aa-ndrej aa-ndrej commented Dec 4, 2024

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

Description

Both vue-router and useRouteQuery differentiate between undefined and null for query parameters when reading/getting the value:

// Path: `/projects`
useRoute().query.archived       // undefined
useRouteQuery('archived').value // undefined ✅

// Path: `/projects?archived`
useRoute().query.archived       // null
useRouteQuery('archived').value // null ✅

useRouteQuery however does not differentiate between undefined and null when writing/setting the value:

useRouter().replace({ query: { archived: undefined } }) // → `/projects`
useRouteQuery('archived').value = undefined // → `/projects` ✅ (but throws a type error ❌ [1])

useRouter().replace({ query: { archived: null } }) // → `/projects?archived`
useRouteQuery('archived').value = null // → `/projects` ❌

In fact TypeScript even prevents us from assigning undefined to the value [1] in the default scenario.
This PR adds undefined to the type definition and aligns the use of undefined and null with vue-router when setting a value.

I am not sure if this is considered a breaking change or not. Previouslly developers likely used useRouteQuery('archived').value = null (because [1] prevented them from using undefined) to "remove" the query parameter from the URL and this would now lead to an URL like /projects?archived instead of /projects. This is however aligned with vue-router.

To remove the query param from the URL we would have to assign undefined or use null as the default value (useRouteQuery('archived', null)) and then asign null.

I am not sure if this change in the URL could actually break something or "just" leads to uglier URLs which could be easily addressed with the two options mentioned above.

#3583 mentions these URL changes. The linked issue however (#3290) only complains about the default value not removing the query param, which still works with this PR.

If this change is considered breaking, we could only include it in the next major version. However I think this change is important to align useRouteQuery with vue-router.

Additional context

These changes also allow us (with transform get/set) to have boolean flags in the URL while having a "pretty" URL like /projects?archived instead of /projects?archived= or /projects?archived=1 or similar.

Also related

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 4, 2024
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Dec 23, 2024
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Dec 23, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 24, 2024
@antfu antfu merged commit 0cc45a8 into vueuse:main Dec 24, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Types of useRouteQuery are missing undefined

2 participants