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): skip value update in mounted hook if value was previously updated #12482

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

edison1105
Copy link
Member

@edison1105 edison1105 commented Nov 28, 2024

close #12460

The root cause is that the component's mounted hook is executed asynchronously, while the beforeUpdate hook is executed synchronously. If an update occurs during the component's mounting, the beforeUpdate hook will execute before mounted. This also causes vModelText's beforeUpdate to execute before mounted, resulting in the value set in the beforeUpdate hook being updated again in the mounted hook.

  • playground 1 onBeforeUpdate of the component is executed before onMounted
  • playground 2 beforeUpdate of the directive is executed before mounted

I'm not sure if this is the correct solution, but the impact should be minimal. Perhaps the correct approach is to ensure that the mounted hook always executes before the beforeUpdate hook.

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 100 kB (+123 B) 38 kB (+34 B) 34.3 kB (+62 B)
vue.global.prod.js 158 kB (+123 B) 57.8 kB (+28 B) 51.4 kB (+4 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 47.2 kB 18.3 kB 16.8 kB
createApp 55.2 kB 21.3 kB 19.5 kB
createSSRApp 59.3 kB 23.1 kB 21 kB
defineCustomElement 60.1 kB 22.9 kB 20.8 kB
overall 69.1 kB 26.5 kB 24.1 kB

Copy link

pkg-pr-new bot commented Nov 28, 2024

Open in Stackblitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@12482

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@12482

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@12482

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@12482

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@12482

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@12482

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@12482

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@12482

@vue/shared

npm i https://pkg.pr.new/@vue/shared@12482

vue

npm i https://pkg.pr.new/vue@12482

@vue/compat

npm i https://pkg.pr.new/@vue/compat@12482

commit: 90daa5c

@edison1105
Copy link
Member Author

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Nov 28, 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 failure success
vuetify success success
vueuse success failure
vue-simple-compiler success success

@edison1105 edison1105 marked this pull request as draft November 28, 2024 08:10
@edison1105 edison1105 marked this pull request as ready for review November 28, 2024 08:49
@edison1105 edison1105 added scope: v-model 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. ready for review This PR requires more reviews labels Nov 28, 2024
@edison1105 edison1105 changed the title fix(vModel): skip value update in mounted hook if value was previously set fix(vModel): skip value update in mounted hook if value was previously updated Nov 28, 2024
@jods4
Copy link
Contributor

jods4 commented Nov 28, 2024

To be honest I don't know what the best resolution is.

This feels a bit like a hack/work-around: vModelText is modified so that out-of-order hooks do not impact the <input> value, but the root cause of problem is still there and may affect other directives or components.

I think the intuition / expectation of Vue developers are:

  1. mounted is called after component is inserted in DOM.
  2. beforeUpdate is never called before mounted.

Based on this, I'd say that if an update happens while the component is being mounted, then maybe one solution may be this: no beforeUpdate or updated hooks are called; and when mounted is invoked it contains the current values, not a snapshot of values when it was enqueued?

@edison1105
Copy link
Member Author

then maybe one solution may be this: no beforeUpdate or updated hooks are called; and when mounted is invoked it contains the current values, not a snapshot of values when it was enqueued?

If beforeUpdate or updated is not called, it might cause some edge cases, I think it should put the beforeUpdate and updated hooks into pendingPostFlushCbs to ensure they are called.

@jods4
Copy link
Contributor

jods4 commented Nov 29, 2024

@edison1105 Maybe that would be a solution, too.
It's an important change because what was a sync hook would become async/delayed but I can't tell if this is an issue or not.
To mitigate this, one approach could be to queue beforeUpdate and updated only when mounted hasn't been called yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. ready for review This PR requires more reviews scope: v-model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Directives mounted called *after* beforeUpdate
3 participants