vuejs / core

🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
https://vuejs.org/
MIT License
47.36k stars 8.28k forks source link

"too much recursion" when objects recursively reference each other #5318

Open MBuchalik opened 2 years ago

MBuchalik commented 2 years ago

Version

3.2.29

Reproduction link

sfc.vuejs.org/

Steps to reproduce

When launching the repro in the SFC Playground, you will immediately notice the error "too much recursion". There are no additional steps required.

What is expected?

I expected to see no errors.

What is actually happening?

In the SFC Playground, you will notice the error "too much recursion". When running it locally, you might see something like this:

Uncaught (in promise) RangeError: Maximum call stack size exceeded
    at Function.[Symbol.hasInstance] (<anonymous>)
    at isDate (shared.esm-bundler.js?99c4:485:1)
    at looseEqual (shared.esm-bundler.js?99c4:386:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)
    at looseEqual (shared.esm-bundler.js?99c4:413:1)

To me, it looks like Vue is trying to deep-compare these objects and does not track already visited ones.


This is a data structure you might see e.g. when dealing with tree-like structures that are aware of their parents. One solution would be to filter out the "parent" information. But that could be non-trivial when using more complex data structures e.g. generated by a library.

posva commented 2 years ago

Avoid the recursivity by marking the object as raw:


const parent = markRaw({
  children: [a, b]
})
MBuchalik commented 2 years ago

Thank you for the reply! 👍

If the data structure is something I can control, then this certainly works. But if the data structure is generated by some kind of (external) library, then you have to make the entire data structure non-reactive. Which of course does not really work if you are using this data structure in other parts of your application where you depend on reactivity.

I must say that I was a bit surprised in general that no simply identity check (===) is performed here. If you look at my example: If a and b have the same shape, then it is not possible to pre-select b. This probably happens because Vue does not perform identity checks, but rather compares object shapes. Is it possible to change this behavior?

LinusBorg commented 2 years ago

We do identity checks, but they fail as a raw original is being compared to its reactive proxy. So the check falls back to a shape comparison, which then runs into an endless loop because of the circular self-references.

Definitely solvable but needs a bit of evaluation to not break other edge cases.

paul-thebaud commented 1 year ago

Hello, any news on this please? Any schedule fix or work on it? I'm experiencing the issue with an ORM like library where instances reference each other.

yonathan9669 commented 1 year ago

Hello, I have basically the same problem as @paul-thebaud, I've been trying several workarounds by now but haven't gotten a real solution yet.

Does any of you have at least a suggestion?

My particular problem is that I'm referencing back the "parent" entity I'm dealing with in order to compute things based on its state changes.

I actually make it work in basic cases, but when the parent-child relation goes beyond 3 steps deep, a "too much recursion" error appears and blocks everything.

Example:

Lead -> Deal -> SaleOrder -> Item

parent -> child 
       -> parent -> child
                 -> parent -> child

The first 2 recursive links work well, but the third one breaks everything.

During my experiments, I've noticed a few things.

if I inject the parent like this

Object.defineProperty(child, path, { value: parent })

It actually does not break since it's not enumerable, but now the child doesn't notice the parent changes so nothing is recomputed. I'm looking for a simple way to maybe inject only a property that triggers this re-render process when the parent changes.

Any ideas here?

Maybe inject a forceUpdate (render) function in child obj to be triggered from parent?

By the way @posva the markRaw function doesn't change anything for me I'm still getting the "Recursion Hell"

yonathan9669 commented 1 year ago

If it helps for anyone out there, I figure a solution out of this using the following approach.

phil294 commented 1 year ago

Does any of you have at least a suggestion?

instead of Object.defineProperty, I solved this problem with markRaw as hinted above - not sure why it doesn't work for you. In my case, I changed my pinia getter from

{
  ...,
  children: [...],
  parent: parent
}

to

{
  ...,
  static: markRaw({
    children: [...],
  },
  parent: parent
}

and then access children as x.static.children whenever necessary. It's not reactive anymore then, true, but that's probably not a dealbreaker?

LinusBorg commented 1 year ago

the overall point is: this only happens when these two conditions are met:

  1. a raw object is being compared to its reactive proxy counterpart with the internal looseEqual() helper. on the user-facing side of things, this can only happen when using checkboxes with explicitly provided true-value/false-value or v-model on select elements.
  2. the object contains circular references, i.e. a self-reference

Thus, as a workaround, you can either make all your data raw or reactive.

playground (note lines 4+5)

As to why this hasn't been solved yet: it's a bit tricky. The most straightforward solution would to make looseEqual also check the raw values if any of the comparators are reactive proxies. But since we use the looseEqual()function in quite a few other places internally, we have to verify that "rawObj === itsReactiveProxy // => true`" is not creating a problem in those other places (or the one shown in OP).

And we simply haven't gotten around to do that, since this is also kind of an edge case and can be worked around as shown above