Skip to content

The type definition of Route seems to be partially wrong #3566

@dora-gt

Description

@dora-gt

Version

3.5.1

Reproduction link

https://github.com/vuejs/vue-router/blob/dev/types/router.d.ts#L203

Steps to reproduce

None, just the source code seems to be wrong.

What is expected?

The type definition of Route#query should be

query: Dictionary<string | null | (string | null)[]>

The current implementation cannot express this type of query

/path?foo#bar

as it doesn't have any value and it is not converted to an array.

What is actually happening?

https://github.com/vuejs/vue-router/blob/dev/types/router.d.ts#L203

The type definition of Route#query is

query: Dictionary<string | (string | null)[]>

Activity

added a commit that references this issue on Oct 15, 2021
izabela-grzeskowiak-allegro

izabela-grzeskowiak-allegro commented on Oct 15, 2021

@izabela-grzeskowiak-allegro
mussinbenarbia

mussinbenarbia commented on Nov 16, 2022

@mussinbenarbia

@posva Hello 👋 I'd like to add some information/context to this. Looking at the implementation of the stringifyQuery function I can see that it uses a function encode for primitive values other than undefined and null. This encode function internally uses encodeURIComponent on the value passed in.

Since encodeURIComponent accepts numbers and booleans as well, what do you think about allowing these as well in the typing for Location["query"]? The implementation already handles these as well, and the end result is the same, a string URL.

Example

vm.$router.push({
  name: 'someRoute',
  query: { foo: 'foo', bar: 1, baz: true, qux: ['hello', 1, false] },
});
// Type 'number' is not assignable to type 'string'.
// Type 'boolean' is not assignable to type 'string'.
// Type '(string | number | false)[]' is not assignable to type 'string'.

The result is: foo=foo&bar=1&baz=true&qux=hello&qux=1&qux=false.

If you agree with this I would gladly work on a PR (add tests as well). This should cover most use cases without any changes to the current functionality.

posva

posva commented on Nov 18, 2022

@posva
Member

@mussinbenarbia You can give it a try if you want to accept null in query in Route.

The reason number and false are not assignable when doing a push is because there isn't any casting (like in v4) so the final type of the query ends up staying a number (non consistent if you reload the page). Therefore, without adding the forced casting (which could break a lot of users code and that's why it was added in vue router 4 only), it's better to keep string and null as the only acceptable types.

added
fixed on 4.xThis issue has been already fixed on the v4 but exists in v3
on Nov 18, 2022
mussinbenarbia

mussinbenarbia commented on Nov 19, 2022

@mussinbenarbia

Thank you for the feedback 👍 I'll give it a go.

Still, I'd like if possible to clarify something.
I understand that Route["query"], can only accept string or null, that's how vm.$route.query is typed and since we do not do any casting, no matter what we push to, the parsed values will always be strings.

Isn't it still possible to allow number and boolean in Location["query"] at push time? The way I'm intepreting it is, when we push to a Location, we can pass in an object in query the values of which can be string, number, boolean, null, undefined or an array of these types.
When stringified, everything will be turned into a string (with the exception of particular handling done for undefined and null), and we will not get back the same types when we parse the query for example on page load.

Isn't that still an improvement? With the full understanding that no matter how Location["query"] is typed, Route["query"] at parse time will always be a Dictionary<string>. This still allows us to push without getting a TS error. Am I missing something?

Thank you!

linked a pull request that will close this issue on Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @posva@dora-gt@mussinbenarbia@izabela-grzeskowiak-allegro

      Issue actions

        The type definition of Route seems to be partially wrong · Issue #3566 · vuejs/vue-router