Approach of conversion for array in query to avoid breaking change #817
Conversation
src/v2/guide/migration-vue-router.md
Outdated
| ```javascript | ||
| '$route.query.users': { | ||
| handler(val) { | ||
| this.$route.query.users = Array.isArray(val) ? val : [val] |
There was a problem hiding this comment.
Why should we talk about watchers? I think the relevant part is that the user should check if val is an array, right?
There was a problem hiding this comment.
@posva This watcher can help to convert the query to an array no matter how many elements are provided in the url.
src/v2/guide/migration-vue-router.md
Outdated
| ```javascript | ||
| '$route.query.users': { | ||
| handler(val) { | ||
| this.$route.query.users = Array.isArray(val) ? val : [val] |
There was a problem hiding this comment.
We shouldn't talk about watchers, simply show the example like:
const users = Array.isArray(this.$route.query.users)
? users
: [users]Modifying the $route object is a bad idea because it's like a snapshot of the current state, it's supposed to be read only
|
Sorry if I wasn't clear enough in my requests. I updated the part, could you check to see if I'm missing something? |
|
Could you please revert that last commit? It's easier to keep the example simple to make easy to match every one case. If you want to use a computed property in the example, please explain why |
src/v2/guide/migration-vue-router.md
Outdated
| ``` | ||
|
|
||
| And then replace the query reference `this.$router.query.users` to the computed `users` in the context accordingly. | ||
| If you were relying on `$route.query.users` in your template, you could use this inside of a computed property. If you were using the query result in a watcher, you should apply the check there. | ||
|
|
There was a problem hiding this comment.
The query users could be used anywhere. When a computed property is added, just replacing every this.$router.query.users with the computed users is sufficient and straightforward.
Adding the computed property is to separate and simplify the checking if the query array is used MULTIPLE TIMES in a component no matter in the script block or the template block, and no obvious defect if it's a one-time-usage.
Besides, if the user is FOR SURE the query parameter SHOULD BE an array during the migration from vue-router 0.7 to vue-router 2, EVEN adding a watcher is acceptable, to my opinion. It will not break the query's consistency, but consolidate the query's consistency.
src/v2/guide/migration-vue-router.md
Outdated
| Since vue-router will treat `?users=Tom` as `query.users == 'Tom'`, unless you are for sure that the array will have more than one element, you should check and convert the query to array if necessary. | ||
|
|
||
| One practical way is to add a computed property like this: | ||
| When passing arrays to query parameters the syntax is no longer `/foo?users[]=Tom&users[]=Jerry`, instead, the new syntax is `/foo?users=Tom&users=Jerry`. Internally, `$route.query.users` will still be an Array, but if there's only one parameter in the query: `/foo?users=Tom`, when directly accessing this route, there's no way for the router to know if we were expecting `users` to be an Array. Because of this, consider using the following check: |
There was a problem hiding this comment.
the syntax is about URL, or QueryString (if URL is not mentioned).
|
@posva Explained in the review of your commit. |
|
@posva And I intended using watcher, which is really effortless in resolving the problem caused by such a breaking change during migration. And I still think it's the best practice. |
|
@KingMario 👍 Updated some lines to polish it |
|
@posva Thx |
|
Still think adding watcher 一劳永逸 |
|
@KingMario |
|
The watcher idea relied on modifying the $route object which hours be
avoided because it's a snapshot. While it made it easier to adapt, it will
teach people a pattern we want to avoid
…On Tue, 14 Mar 2017, 01:35 kazuya kawaguchi, ***@***.***> wrote:
@KingMario <https://github.com/KingMario>
You can send the pull request :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#817 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAoicYm370mxc9jXEanVCrVLBOvVZmosks5rleDBgaJpZM4MZE8v>
.
|
related to: vuejs/vue-router#1232