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

Vue infinite loop detection should throw a real error in dev, not just log a warning to console #7437

Closed
Inori-Lover opened this issue Dec 30, 2022 · 19 comments · Fixed by #7447
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. has PR A pull request has already been submitted to solve the issue

Comments

@Inori-Lover
Copy link

Inori-Lover commented Dec 30, 2022

Vue version

3.2.45

Link to minimal reproduction

github repo

Steps to reproduce

  1. pnpm dev, open chrome dev tool
  2. click the button
  3. pnpm dev && pnpm preview, click button again
  4. compare both console log

What is expected?

both dev and prod mode should work in same way; or the code did not cause recursively update

What is actually happening?

in dev mode, recursive update will stop auto;
in prod mode, it just continue run

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (12) x64 Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz
    Memory: 21.69 GB / 31.92 GB
  Binaries:
    Node: 16.17.1 - ~\AppData\Local\fnm_multishells\17284_1672413295400\node.EXE
    npm: 8.15.0 - ~\AppData\Local\fnm_multishells\17284_1672413295400\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.963.0), Chromium (108.0.1462.54)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    vue: ^3.2.45 => 3.2.45

Any additional comments?

sfc.vuejs.org even in PROD mode, it still use dev vue file so i upload a github repo.

the key code

watch(() => [state.value.deep], () => {
  state.value = { deep: false }
  // console.log('watch', ++count)
})

====

english is not my first lang, so i write a chinese version desc here too.

复现:

  1. clone repo后,跑dev,尝试点击button
  2. 观察控制台输出
  3. pnpm dev && pnpm preview, 再次点击button
  4. 再次观察控制台输出

期望:
dev与prod mode两边的行为一致,或者这段代码根本不会产生死循环更新

实际:
dev会在安全范围内停掉这个watch
prod则会跑到死

@sxzz
Copy link
Member

sxzz commented Dec 30, 2022

+++ watch(() => state.value.deep, () => {
--- watch(() => [state.value.deep], () => {

or check the new value and old value yourself

It will avoid recursive updates.

@sxzz
Copy link
Member

sxzz commented Dec 30, 2022

in dev mode, recursive update will stop auto; in prod mode, it just continue run

The behaviors of dev and prod are expected since Vue emitted a warning when dev mode. The developers should fix this.

if (count > RECURSION_LIMIT) {
const instance = fn.ownerInstance
const componentName = instance && getComponentName(instance.type)
warn(
`Maximum recursive updates exceeded${
componentName ? ` in component <${componentName}>` : ``
}. ` +
`This means you have a reactive effect that is mutating its own ` +
`dependencies and thus recursively triggering itself. Possible sources ` +
`include component template, render function, updated hook or ` +
`watcher source function.`
)
return true
} else {
seen.set(fn, count + 1)
}

[Vue warn]: Maximum recursive updates exceeded. This means you have a reactive effect that is mutating its own dependencies and thus recursively triggering itself. Possible sources include component template, render function, updated hook or watcher source function.

@sxzz sxzz closed this as completed Dec 30, 2022
@Inori-Lover
Copy link
Author

+++ watch(() => state.value.deep, () => {
--- watch(() => [state.value.deep], () => {

or check the new value and old value yourself

It will avoid recursive updates.

i am not interested in how to fix this code; if i write the logic, data update will not appera in watch cb but in event cb.
i think it is important than a framework have same behavior in both mode, both stop the browser or both stop in max count

@sxzz
Copy link
Member

sxzz commented Dec 30, 2022

I think it is an intentionally optimized behavior of Vue. There are some small differences in behavior between production and development modes, but they are aimed to improve the developing experience.

In this case, the browser will be stuck by infinite looping. So that's why Vue will stop it and emit a warning in dev mode. Relate PR: #456

If you insist that the behavior can be improved or removed, I'm also happy to reopen the issue for a more in-depth discussion.

/cc @LinusBorg @yyx990803

@sxzz sxzz reopened this Dec 30, 2022
@Inori-Lover
Copy link
Author

@sxzz tks

@LinusBorg
Copy link
Member

LinusBorg commented Dec 30, 2022

i think it is important than a framework have same behavior in both mode, both stop the browser or both stop in max count

  • the max count check has a negative performance impact, so it's active only in dev.
  • and it's in dev because the warning we can give this way is much more helpful than a context-free max stack size exceeded.

So it's a compromise to get both better dev experience and better prod performance.

What specific problem does the current behavior pose for you that would be worth giving that up? Both will effectively stop the app and throw an error?

@Inori-Lover
Copy link
Author

Inori-Lover commented Dec 31, 2022

@LinusBorg problem is same wrong code, vue "work" in dev mode but not in prod mode.

may be vue team can consider thrown error in dev mode too?

@sxzz
Copy link
Member

sxzz commented Dec 31, 2022

Actually, warnings are not to be ignored in Vue.

@Inori-Lover
Copy link
Author

@sxzz i know vue team has warning me/us in console to friendly reminder me modify my code, but catch an error then change it to a warn is not a good practice, right? it make wrong code "work".
and we can not expect all dever open dev tool all the time

@sxzz
Copy link
Member

sxzz commented Dec 31, 2022

@Inori-Lover 🤔 Hmmm it makes sense.

I think we should change the warning to an error too and it's a serious error to be fixed by developers.

@LinusBorg LinusBorg changed the title it should run same logic in both dev and prod mode Vue infinite loop detection should throw a real error, not only log a warning to console in dev. Dec 31, 2022
@LinusBorg
Copy link
Member

Agreed.

@LinusBorg LinusBorg changed the title Vue infinite loop detection should throw a real error, not only log a warning to console in dev. Vue infinite loop detection should throw a real error in dev, not just log a warning to console Dec 31, 2022
@smallnine9
Copy link
Contributor

smallnine9 commented Jan 1, 2023

do we need a pr?

@sxzz
Copy link
Member

sxzz commented Jan 1, 2023

Sure, if you would like

@smallnine9
Copy link
Contributor

so, maybe we need to replace the 'warn' with throwing an error? But I want to know how to test changes in prod mode? Since the SFC also running in dev mode

smallnine9 added a commit to smallnine9/core that referenced this issue Jan 2, 2023
@sxzz sxzz added has PR A pull request has already been submitted to solve the issue 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. labels Jan 2, 2023
@7Z0nE
Copy link

7Z0nE commented Apr 26, 2023

@smallnine9 @sxzz Why was this PR closed without comment? I think this is a much needed feature and should be merged soon.

@Syntax-J
Copy link

same here @yyx990803

@acn-masatadakurihara
Copy link

@yyx990803 @smallnine9 @sxzz
In this large application, correlated infinite loops cannot be noticed and cannot be detected by UT or static analysis.
I feel that this also evolves into the question of whether or not Vue.js is the architecture of choice for building enterprise apps. Please check it out.

yyx990803 pushed a commit that referenced this issue Dec 19, 2023
@sxzz
Copy link
Member

sxzz commented Dec 19, 2023

Resolved in #7447

@sxzz sxzz closed this as completed Dec 19, 2023
@acn-masatadakurihara
Copy link

Thanks for the tremendously fast response.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. has PR A pull request has already been submitted to solve the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants