Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query/Location can be an array of strings. (fix #1932) #2050

Merged
merged 1 commit into from
Feb 7, 2018
Merged

Query/Location can be an array of strings. (fix #1932) #2050

merged 1 commit into from
Feb 7, 2018

Conversation

zigomir
Copy link
Contributor

@zigomir zigomir commented Feb 7, 2018

Query and Location can be arrays as can be deduced from a spec here and here.

I guess for custom parsing user needs to provide own types and override ones coming from router somehow.

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@posva
Copy link
Member

posva commented Feb 7, 2018

Oh thanks!

@Kingwl
Copy link
Member

Kingwl commented May 10, 2018

it's very helpful to me ! (after release

@tarunmangukiya
Copy link

Any plans when this is going to be released? It's been more than 6 months since last release!

@fuzzzerd
Copy link

I see that this was released in the last day or two and I've started getting typescript errors.

With v3.0.1 I was able to do the following:

const myId: string = this.$route.query.queryStringVariable;

after updating to v3.0.2 the above code throws an error that type string | string[] is un-assignable to type string.

Was my example above working by accident? It seems that the only way to get this to work is to rewrite it as

const myId: string = this.$route.query.queryStringVariable as string;

which is sub-optimal, but perhaps I'm doing something wrong?

@dinvlad
Copy link

dinvlad commented Nov 27, 2018

Same issue here, our code has been assuming this.$route.query.queryStringVariable is a string for many months now, but looks like this is a breaking change?

Seems like for safety, we should change the code to Array.isArray(queryStringVariable) ? queryStringVariable[0] : queryStringVariable now.

Alternatively, had the breaking change been to switch to string[] everywhere now, then we could simply use queryStringVariable[0] from now on.

@zigomir
Copy link
Contributor Author

zigomir commented Nov 27, 2018

It was working by coincidence – see console.logs below in this runkit example – where I copied default parseQuery function (without flow types) from vue-router's query.js file.

Otherwise I agree it could be nicer to always return same type string[].

@dinvlad
Copy link

dinvlad commented Nov 27, 2018

Yeah, makes sense. I can also see that the "new" definition (string | string[]) is only breaking for TypeScript users, and provides a compatibility bridge for vanilla JS. Perhaps we could change to string[] only in 4.0?

@dinvlad
Copy link

dinvlad commented Nov 27, 2018

I think the string vs array was not clearly documented in https://github.com/vuejs/vue-router/tree/dev/docs/api#route-object-properties (there, query is only vaguely defined as an Object of key-value pairs).

@fuzzzerd
Copy link

Is splashing Array.isArray(queryStringVariable) ? queryStringVariable[0] : queryStringVariable; around all of the views where query string parameters are processed considered the typescript solution until v4 is released with only string[]?

@khuguet-tfg
Copy link

khuguet-tfg commented Nov 28, 2018

I'm experiencing this issue as well. It feels like semantic versioning rules were not followed here. A patch release that contains a type change on a public API is normally not allowed. I know I'm pointing out the obvious, but just venting my frustration a bit.

@zigomir
Copy link
Contributor Author

zigomir commented Nov 28, 2018

I'd argue that public API wasn't changed – typings were wrong in the first place. But I can see your frustration – I felt it before, because typings were wrong.

@fuzzzerd
Copy link

I suppose that's what we get when we're adding Typescript in after the fact; but it never the less is a breaking change since definitions are provided.

In any event, lets all look forward to v4 where this will get a correction.

@Akimyou
Copy link

Akimyou commented Aug 2, 2019

Disappointment. I have to change my code about the query, such as:

const { addShowItem } = this.$route.query
if (addShowItem && typeof addShowItem === 'string') {
}

Hope can fix it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants