-
-
Notifications
You must be signed in to change notification settings - Fork 5k
Open
Labels
TypescriptTypescript related issuesTypescript related issuescontribution welcomefixed on 4.xThis issue has been already fixed on the v4 but exists in v3This issue has been already fixed on the v4 but exists in v3good first issue
Description
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)[]>
Metadata
Metadata
Assignees
Labels
TypescriptTypescript related issuesTypescript related issuescontribution welcomefixed on 4.xThis issue has been already fixed on the v4 but exists in v3This issue has been already fixed on the v4 but exists in v3good first issue
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
vuejs#3566 | fix type issue
izabela-grzeskowiak-allegro commentedon Oct 15, 2021
#3647
mussinbenarbia commentedon Nov 16, 2022
@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 thanundefined
andnull
. Thisencode
function internally usesencodeURIComponent
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
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 commentedon Nov 18, 2022
@mussinbenarbia You can give it a try if you want to accept
null
in query inRoute
.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.mussinbenarbia commentedon Nov 19, 2022
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 acceptstring
ornull
, that's howvm.$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
andboolean
inLocation["query"]
at push time? The way I'm intepreting it is, when we push to aLocation
, we can pass in an object inquery
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 aDictionary<string>
. This still allows us to push without getting a TS error. Am I missing something?Thank you!