-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix(useRouteHash,useRouteQuery,useRouteParams): re-evaluates the value immediately #3002
Conversation
I think it was on purpose to collect all changes from multiple sources and avoid conflicts |
History: #901 -> #2646 -> commit You are right, and I followed those adjustments and worked to not break the tests. But apparently, the tests are not consistent. After your comment, I reviewed the entire code again and did some more tests, and realized that there is a break. I am working on the correction and will send it soon with more tests. Thank you. |
@antfu, the latest updates ensure that the route is updated with multiple sources and add more tests. You can see the code working here. Check how the current version is working here. As you can see, the current variable value doesn't change correctly even using |
1ca1d84
to
cffa38f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explaination! I left some concern here.
Working on multiple scope example |
@anteriovieira i have some questions. Firstly, this PR is causing some errors: #3173 In my understanding, if we need an immediate value and a reactive value, we can do it like this: let isChanging = false
const ref = computed<any>({
get() {
const val = route.query[name]
const newVal = _query.get(name)
const data = (isChanging ? newVal : val) ?? defaultValue
return transform(data as T)
},
set(v) {
isChanging = true
_query.set(name, (v === defaultValue || v === null) ? undefined : v)
nextTick(async () => {
await router[toValue(mode)]({
...route,
query: { ...route.query, ...Object.fromEntries(_query.entries()) },
})
isChanging = false
})
},
}) but, they do not work with scope.stop() because computed is cleaned up and cannot trigger the getter. Maybe we can change the unit test? |
Hi @lishaobos , thanks! I'm going to investigate it. |
😊,i have another idea, they can pass unit test watch(() => route.query[name], (v) => {
_query.set(name, (v === defaultValue || v === null) ? undefined : v)
_trigger?.()
}, {
flush: 'sync',
immediate: true,
})
return customRef<any>((track, trigger) => {
_trigger = trigger
return {
get() {
track()
const data = _query.get(name) ?? defaultValue
return transform(data as T)
},
set(v) {
_query.set(name, (v === defaultValue || v === null) ? undefined : v)
trigger()
nextTick(() => {
router[toValue(mode)]({
...route,
query: { ...route.query, ...Object.fromEntries(_query.entries()) },
})
})
},
}
}) |
Hi @lishaobos , thanks! You had a good solution for that. Sorry for the delay. |
Before submitting the PR, please make sure you do the following
fixes #123
).Description
It will re-evaluate the value immediately.
Additional context
By default, Vue.js caches the result of a computed property and only re-evaluates it if one of its dependencies has changed. But currently, it seems that it is not updating or getting the correct value on the same tick. These updates will force a re-evaluation after the value has been updated.
Example:
live code
🤖 Generated by Copilot at a4035ad
This pull request refactors and improves the router composables
useRouteHash
,useRouteParams
, anduseRouteQuery
, by enhancing their reactivity, consistency, and type support. It also adds or updates the corresponding test files to verify their functionality and behavior. Additionally, it renames theRouterQueryValue
type toRouteDefaultValue
to avoid confusion.🤖 Generated by Copilot at a4035ad
RouterQueryValue
type toRouteDefaultValue
to avoid confusion (link, link, link, link, link)_types.ts
for consistency (link, link, link)useRouteHash
,useRouteParams
, anduseRouteQuery
to use local refs instead of directly modifying the route object, to improve reactivity and avoid side effects (link, link, link)useRouteHash
composable, with two test cases to check the initial and updated values of the hash (link)useRouteParams
composable, to importRef
type, addparams
argument togetRoute
helper, and add two test cases to check the initial and updated values of the param (link, link, link, link)useRouteQuery
composable, to importRef
type, simplifygetRoute
helper, and add a test case to check the updated value of the query (link, link, link, link)