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

Getting recursive updates error when adding boolean prop without ="true" #3371

Closed
plehnen opened this issue Mar 5, 2021 · 4 comments
Closed
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working

Comments

@plehnen
Copy link

plehnen commented Mar 5, 2021

Version

3.0.7

Reproduction link

https://codesandbox.io/s/bold-drake-1qwvq?file=/src/App.vue

Steps to reproduce

I found two things which are weird: I get the "Maximum recursive updates exceeded" error only if I write <VEntry mandatory />.

But if I write <VEntry :mandatory="true" /> (or false) it works. (See comment in App.vue in the demo)

(For me a huge indicator that there is something wrong internally, since both should mean the same)

Additionally: If I wrap one of the passed computed refs with a readonly, it outputs other warnings. (See comment in DescriptionListEntry.vue in the demo)

What is expected?

  • consistent behaviour with or without ="true" on boolean props (ideally without the recursive error)

  • no internal write access on readonly computed refs

What is actually happening?

  • Maximum recursive updates exceeded.

  • Set operation on key "_ value" failed: target is readonly. Set operation on key "_ dirty" failed: target is readonly.


I am so confused right now. I tried to remove everything around the issue to track it down to a minimal reproduction. Hope this helps.

@edison1105
Copy link
Member

you should

  const register = (isVisible) => {
    console.log("useRelocatableParent - register");
    const index = nextIndex++;

	// like this
    registeredEntries.value[index] = { isVisible: isVisible.value };
    return index;
  };

@SaavyGirl

This comment has been minimized.

@plehnen
Copy link
Author

plehnen commented Mar 6, 2021

@edison1105 this would loose reactivity. I need to pass computed refs, which will update as soon as their state change. Do you mean I should watch for changes and then call a injected function again each time? I thought the benifit of exposing computed is that I don't need to do that anymore. (This would still not explain why everything works when I write :mandatory="true" instead of just mandatory)

@LinusBorg
Copy link
Member

LinusBorg commented Mar 6, 2021

To get the smaller issue out of cleared up: wrapping a computed in readonly doesn't make much sense as its readonly already. But nonetheless we should probably not have it break.

Should likely be handled in a separate issue. (-> #3376, PR: #3377)

The thing we should concentrate on here is that, yes, it's weird and possibly a bug that this breaks only when relying on the default value.

@LinusBorg LinusBorg added 🐞 bug Something isn't working 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Mar 6, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants