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

fix(vModel): fix v-model being reset in updated #8639

Closed
wants to merge 16 commits into from

Conversation

Alfred-Skyblue
Copy link
Member

@Alfred-Skyblue Alfred-Skyblue commented Jun 25, 2023

fixed #8638
fixed #8579

If the updated hook of the component is triggered during the triggering of the instruction, the updated hook of the instruction will be triggered and the value of v-model will be reset, resulting in the value of v-model not being updated as expected.

@Alfred-Skyblue Alfred-Skyblue marked this pull request as ready for review June 25, 2023 10:42
@Alfred-Skyblue
Copy link
Member Author

TBR

@sxzz sxzz added the ready for review This PR requires more reviews label Aug 15, 2023
@stoner-w
Copy link

I encountered the same problem. How should I bypass this problem now? But in my business, I must bind the click event on the checkbox to modify other values. Is there any way to achieve this?

@Alfred-Skyblue
Copy link
Member Author

I encountered the same problem. How should I bypass this problem now? But in my business, I must bind the click event on the checkbox to modify other values. Is there any way to achieve this?

Hello, I'm not familiar with your business context. Could you provide the minimal reproducible link, please?

@stoner-w
Copy link

stoner-w commented Sep 1, 2023

Hello, my business is more complicated, it is actually similar to the scene in #8638, what can you do?

@Alfred-Skyblue
Copy link
Member Author

Refer here, you can temporarily use asynchronous execution of click events to solve this problem, such as:

function onClick() {
  setTimeout(() => {
    // do something
  })
}

Playground

@stoner-w
Copy link

stoner-w commented Sep 1, 2023

Thank you. According to your tips, I can now use it normally. Thank you for your help.

@sxzz sxzz removed the ready for review This PR requires more reviews label Sep 1, 2023
@Alfred-Skyblue Alfred-Skyblue changed the title fix(vModel): fix the issue where checked gets reset during an update fix(vModel): fix the issue where checked and select gets reset during an update Sep 14, 2023
@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 90.8 kB (-2 B) 34.5 kB (+9 B) 31.1 kB
vue.global.prod.js 148 kB (-2 B) 53.7 kB (+7 B) 48 kB (-3 B)

Usages

Name Size Gzip Brotli
createApp 50.8 kB 19.9 kB 18.1 kB
createSSRApp 54.1 kB 21.2 kB 19.3 kB
defineCustomElement 53.1 kB 20.6 kB 18.8 kB
overall 64.6 kB 24.9 kB 22.6 kB

@Alfred-Skyblue Alfred-Skyblue changed the title fix(vModel): fix the issue where checked and select gets reset during an update fix(vModel): fix v-model being reset in updated Sep 14, 2023
Copy link

codspeed-hq bot commented Dec 12, 2023

CodSpeed Performance Report

Merging #8639 will not alter performance

Comparing Alfred-Skyblue:fix-vModel-checkbox (3b9227b) with main (04d2c05)

Summary

✅ 53 untouched benchmarks

@edison1105
Copy link
Member

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Dec 13, 2023

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools failure failure
nuxt success success
pinia failure success
quasar success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros success success
vuetify success failure
vueuse success success
vue-simple-compiler success success

@stoner-w
Copy link

@Alfred-Skyblue 这个 PR 有合并计划吗?因为我启动了新项目,这导致我在很多地方需要在定时器中处理事件。

@Alfred-Skyblue
Copy link
Member Author

@Alfred-Skyblue 这个 PR 有合并计划吗?因为我启动了新项目,这导致我在很多地方需要在定时器中处理事件。

Waiting for review

@Alfred-Skyblue
Copy link
Member Author

I am unable to simulate browser usage scenarios in unit testing, possibly due to issues with event triggering. Therefore, I have not added test cases temporarily.

@kevmul
Copy link

kevmul commented Jan 5, 2024

I think the one thing this PR is missing is some a test showing it fails before this change and passes after. Just to prevent this issue from happening again in the future.

@edison1105
Copy link
Member

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented May 31, 2024

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools success success
nuxt success success
pinia success success
primevue success success
quasar success success
radix-vue success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros success success
vuetify success success
vueuse success success
vue-simple-compiler success success

Comment on lines +160 to +161
return // store the v-model value on the element so it can be accessed by the
// change listener.
Copy link
Contributor

Choose a reason for hiding this comment

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

Prettier has moved this comment, so it's now in the wrong place. I think you'll need to add braces around the if body to get the comment to stay where it's meant to be.

// store the v-model value on the element so it can be accessed by the
// change listener.
// #8638
if (value === oldValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning early like this won't work if the value is an Array or Set that's been modified. e.g.:

Running this same example against main works as expected:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Rejected
7 participants