Skip to content
This repository has been archived by the owner on Nov 9, 2021. It is now read-only.

[Vue warn]: Error in nextTick: "RangeError: Maximum call stack size exceeded" #27

Closed
cfour-hi opened this issue Aug 29, 2019 · 12 comments · Fixed by #42
Closed

[Vue warn]: Error in nextTick: "RangeError: Maximum call stack size exceeded" #27

cfour-hi opened this issue Aug 29, 2019 · 12 comments · Fixed by #42
Labels
bug 🐛 Something isn't working
Milestone

Comments

@cfour-hi
Copy link

e.g.
https://codesandbox.io/embed/vue-template-3wd0w

The key point is on the container property
Is my usage wrong?

@probil
Copy link
Owner

probil commented Aug 29, 2019

Hi @Monine 👋
Thanks for your issue.

container param is optional. Are you sure you need it?
I'll check your example more deeply soon.

In a meantime you can check the code of the demo
https://github.com/probil/vue-moveable/blob/master/src/App.vue

@cfour-hi
Copy link
Author

Yes, I want to be in a specific container.

@cfour-hi
Copy link
Author

cfour-hi commented Sep 4, 2019

Is there any progress? We can discuss it together.

@probil probil added the bug 🐛 Something isn't working label Sep 4, 2019
@probil
Copy link
Owner

probil commented Sep 4, 2019

Hi @Monine
I can confirm that there is a bug.
I don't have time right now to investigate the reason why it's happening.

It looks strange because there is nothing special about it. 🤔
The component takes container element as a property and passes it to moveable. No magic here

Let's try to inline component from the repo to your codesandbox and use development version of vue so we have access to sourcemaps

@cfour-hi
Copy link
Author

cfour-hi commented Sep 5, 2019

Think differently.
Is it possible to use custom directives to avoid this bug?
I will find time to try.

@romansp
Copy link
Contributor

romansp commented Oct 16, 2019

After trying out different things I found out that RangeError is thrown when passed container prop is a Vue component itself. To avoid it you can create a wrapping element and pass it instead.

See updated codesandbox here (check console): https://codesandbox.io/embed/vue-template-67q68

@stefanaz
Copy link

Having similar issue with usage of container option,
in case it is bounded directly to the app: container: document.getElementById('app'),:

RangeError: Maximum call stack size exceeded at Function.[Symbol.hasInstance] (<anonymous>)

While my Vue instance is mounted to the same app div.
But in case the moveable container points to the child div of the app div, then all seems to be fine and no errors.
It is not related to the nextTick(), but I post it just in case it may help you to find the issue with 'RangeError: Maximum call stack size exceeded'.

@romansp
Copy link
Contributor

romansp commented Oct 17, 2019

Did further further investigation. This is happening due to watcher on $props being marked as deep.

watch: {
$props: {
handler(newOptions) {
Object.keys(newOptions).forEach((key) => {
const existingValue = this.moveable[key];
const newValue = newOptions[key];
if (existingValue === newValue) return;
this.moveable[key] = newOptions[key];
});
},
deep: true,
},

When passed container prop is a Vue component then watcher has to traverse the whole object and eventually "traps" itself trying to build a dependency tree.

Simple repro available here: https://codesandbox.io/s/vue-template-20djs. Try clicking update buttons and error will appear in the console. Removing deep fixes that. There is no dependency on moveable package whatsoever, so it's purely misuse of Vue's APIs.

To fix this should avoid deep-watching for changes on container prop. We probably can put moveable options on separate prop, e.g. options and deep-watch only it. This will also make it much easier to support for future versions of moveable without making changes to props list in Moveable.vue. This is obviously a breaking change.

@probil
Copy link
Owner

probil commented Oct 17, 2019

This is cool.
Thanks for your time on investigation @romansp . Would you like to make a PR with a fix?

Without deep on $props we can't handle reactive updates (to sync props and moveable). But I think we can watch props one-by-one so deep is not needed. There will be a lot of repetitions but I have an idea how to handle it.

@romansp
Copy link
Contributor

romansp commented Oct 17, 2019

@probil, Yes, sure I will try and open a PR.

What do you think about changing component's API from enumerating all moveable options as separate props and use a single options prop instead? This options prop can be deep-watched separately.

This might be a better option as component will support all existing (and future possible) moveable options by default, automatically fixing #31. Maybe it will be also possible to no longer pin moveable version in package.json and move to peerDependencies?

@probil
Copy link
Owner

probil commented Oct 17, 2019

This might be a better option as component will support all existing (and future possible) moveable options by default, automatically fixing #31.

🤔 it may look like a good idea but it's not a Vue-way of doing things. And after that change there will be no point of having this component.

I think we can do it in another way:

const REACTIVE_PROPS = [
  'bounds',
  'dragArea',
  'draggable',
  // and all other, can be taken from `moveable` package.
];

const watchReactiveProp = key => function (newValue) {
  const existingValue = this.moveable[key]; 
  if (existingValue === newValue) return; 
  this.moveable[key] = newValue; 
}

const passMulpleReactivePropsToMoveable = props => props.reduce((acc, prop) => {
  acc[prop] = watchReactiveProp(prop);
  return acc;
}, {})

export default {
   watch: {
     ...passMulpleReactivePropsToMoveable(REACTIVE_PROPS),
   }
}

Maybe it will be also possible to no longer pin moveable version in package.json and move to peerDependencies?

I thought about that. It would be really nice to get rid of moveable from directdependenciess. Would you like to create a separate issue for that?

romansp added a commit to romansp/vue-moveable that referenced this issue Oct 17, 2019
When container prop is a Vue component  watcher will have to traverse the whole object and eventually "traps" itself trying to build a dependency tree.
@probil probil reopened this Oct 22, 2019
@probil probil added this to the v1.0.0 milestone Oct 22, 2019
@probil
Copy link
Owner

probil commented Oct 22, 2019

Released as part of v1.0.0

@probil probil closed this as completed Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants