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

defineModel setter function not working properly when using functions like replace #10279

Closed
sina-byn opened this issue Feb 6, 2024 · 13 comments

Comments

@sina-byn
Copy link

sina-byn commented Feb 6, 2024

Vue version

3.4.15

Link to minimal reproduction

https://play.vuejs.org/#eNp9Uk1v2zAM/SuELnGxzD70FrgD9tHDBnQbttymHTybdtXJkiBRWYIg/32UHHtB0eViWXyP1CP5juKtc+UuotiIOrReOYKAFN0badTorCc4gsceTtB7O8KKqasFeji8t/xn0NAZLquLWKrLZABpWmsCwXjY4p7gLlUsVqsbaepqepSf4wvh6HRDyDeA7aMKoIyLBNboAzRti44CdGpQFDaJUl8K2L0ebYe6NHH8hf5Oiuk1KaDienW1FBdrQYEF9Woon4I13PoxqxQtF1Ma/RdHigVLsYGMJKzR2v75lGPkI67nePuI7e8X4k9hn2JSfPUY0O9QigWjxg/IyhJ8//1zVrmA3EXUzL4CfsNgdUwaJ9q7aDqWfcHLaj/mLSkzbMP9ntCEuakkNDFPmS8F7ynN8X+t/5N7W97mPGlOPMVnq75moTTZSNg989FsjB95dWvgQ/UKffjJJumwVwYfElJkLVy12DU64s2sTfVQLDnnzS8gsM0oegM5p/TI62+xqKT88Koa1pANmGjcTm40fy9z5l6vG7WeTEoHh+w6mjx3dmOyYTpfcOHpL8fjLVA=

Steps to reproduce

type in the provided input - make sure that you use a combination of digits and letters

What is expected?

the expected behavior is that the input should only accept digits and not anything else by replacing every non-numeric character with an empty string

What is actually happening?

the problem is that once a single letter is typed the input keeps showing the letters that are being typed until another digit is typed that's when the non-numeric characters are replaced with empty strings

System Info

No response

Any additional comments?

No response

@yangxiuxiu1115
Copy link
Contributor

It looks like the problem is more similar to #10254.

@sina-byn
Copy link
Author

sina-byn commented Feb 7, 2024

It looks like the problem is more similar to #10254.

is sure does but the problem is instead of using a ref I was using a reactive containing an object when I first encountered this issue. Does that make a difference?

@yangxiuxiu1115
Copy link
Contributor

Essentially there is no difference, Vue's responsive update is only triggered when the corresponding value changes. Whether you use ref or reactive here their values don't change.

@sina-byn
Copy link
Author

sina-byn commented Feb 7, 2024

Essentially there is no difference, Vue's responsive update is only triggered when the corresponding value changes. Whether you use ref or reactive here their values don't change.

so you advise that we refer to the other issue?

@yangxiuxiu1115
Copy link
Contributor

I just thought it might not be a problem and maybe you could use the program to solve the problem you are having. Similar to this comment.

@sina-byn
Copy link
Author

sina-byn commented Feb 7, 2024

I just thought it might not be a problem and maybe you could use the program to solve the problem you are having. Similar to this comment.

I fixed my problem with a workaround solution I managed to implement.
However the problem in my case is not implementing the functionality its that the setter function of the given model that is synced with my input is not updating the value properly and I can't seem to understand why.

@yangxiuxiu1115
Copy link
Contributor

Sorry, I now think this might be a bug, and even if it's not a bug it's a very misleading feature, and maybe it should be fixed. As can be found in #10301 and this issue, it's easy to produce results that don't match your intuition when using defineModel's set.

@LinusBorg
Copy link
Member

LinusBorg commented Feb 9, 2024

It looks like the problem is more similar to #10254.

Yes, this is about the same thing, basically. When the model emits the same value to the parent as it did before, then the parent won't re-render, and the child won't receive a fresh model prop, so it won't update.

However I think we might be able to improve this, we can trigger the customRef even if the new value that is to be emitted is the same as the current prop value. Will need testing though.

I will also close #10301 as it is presenting the same issue as well, just with a simplified example.

@sina-byn
Copy link
Author

sina-byn commented Feb 9, 2024

It looks like the problem is more similar to #10254.

Yes, this is about the same thing, basically. When the model emits the same value to the parent as it did before, then the parent won't re-render, and the child won't receive a fresh model prop, so it won't update.

However I think we might be able to improve this, we can trigger the customRef even if the new value that is to be emitted is the same as the current prop value. Will need testing though.

I will also close #10301 as it is presenting the same issue as well, just with a simplified example.

I am not really sure about what's happening behind the scenes but wouldn't adding a wrapper object behind the scenes make the value to be not the same as the reference is updated???

And I was wondering what do you mean by the same value, doesn't the replace function create a new value in this case?

And I was wondering if there is an issue that you can assign me so that I can work on this improvement with the necessary guides?

@LinusBorg
Copy link
Member

LinusBorg commented Feb 9, 2024

Here's a demo of the underlying effect, without using a child component and using a writeable computed instead of defineModel:

https://play.vuejs.org/#eNqNUk1vnEAM/SvWXCDqFrTKbbtbqR85tFLSqs2t0wMFQyaFAc0H3RXiv9djAkujJOoFZvyePX72G8S7rkt6j2In9jY3qnNg0fnurdSq6VrjYACDJYxQmraBiKjRAl2fPrR00qjdA5ykq1ioeyYPkBPgHRb/FHsjNYDUeautg+Z045tfaOAQHo2j7XYbXUg9w31WeyRsLhQPIblCF19wm84bvdRIJva4CRzSFPdECmd4TDlAnxjs6izHOJXy46u02kAUHgYYpR7psE+n4dBY6OKwIbZDugHc3ikLSlND0Or6BFmeY+csFKpSzu4CZT/B/eumLbA+SMHvSsH5w3CWPdJz+3QpLzbCWZJeqiq5t62mJbECKcIEVI3mS+cUjUaK3axNiqyu2z+fOeaMR9bPOXeY/34ifm+PISbFV4MWTU+NLZjLDI13gq++3+CRzgtIanxN7BfAb2jb2oceJ9p7rwtqe8Xjbj+xRZSubu3V0aG2s6jQKK+B+TQ4j8Fez0k/t3uZXM7royk+MuVLZn/SpGcL/uAVboB+qlRo7E+yT4Gl0ngdkMmSbLew48VyqoR4yUk0r3sBYfYu5zzrRZbDQvm7zvk/qz740J06JBM6XtjKlfynSErktQvHv+BAZrc=

When you write digits, it works fine. when you start adding letters, it doesn't update anymore.

What's going on? Well,

  • the setter function always assigns the same trimmed value to myNumber.
  • So the value of myNumber never changes,
  • so there's no reason to re-render,
  • so the component doesn't re-render.
  • And without a re-render, the value of the input can't be updated to the digit-only value.
  • ...and so, the letters stay in the input.

The same happens basically in your original example, or in the shortened example of #10301, just that the always-same value gets emitted to the parent component - which doesn't need to re-render, so the model prop never gets updated, so the child never re-renders ....

I think we can make it work by adding some additional logic in defineModel's internal implementation, but I'm not sure right now if we want that implicit "magic" to take place when defineModel is essentially meant to be a shorthand for my computed example above, just with a prop and event.

@sina-byn
Copy link
Author

sina-byn commented Feb 11, 2024

@LinusBorg I don't get what you mean by the same trimmed value? how is it considered the same if the letters are removed?
And I was wondering if this is considered a bug and how can I contribute to fixing it if that's necessary?

@LinusBorg
Copy link
Member

LinusBorg commented Feb 11, 2024

@LinusBorg I don't get what you mean by the same trimmed value? how is it considered the same if the letters are removed?

After the letters are removed, you always end up with the same value as one input earlier. Look at my example:

  1. Type in 111 -> myNumber will be assigned '111'
  2. Type in 111a -> myNumber will be assigned '111' - which is the same value as before. So myNumber didn't actually change
  3. Type in 111aa -> again, myNumber will be assigned '111' - which is the same value as before.

so after 1., Vue has no reason to re-render and update the input value again - it's not aware that it has to.

And I was wondering if this is considered a bug and how can I contribute to fixing it if that's necessary?

I'd agree with Evan in #10254 that this is expected behavior in a way, as it is how Vue's reactivity works. As mentioned in my previous reply, I see a possibility for some optimization in defineModel's implementation to account for this scenario, yet I wonder if that's desirable. defineModel would then behave more differently than the code it was meant to be an abstraction for.

Contributing would start with properly understanding the cause of the issue and then talk about possible solutions and their implications with us.

@sina-byn
Copy link
Author

sina-byn commented Feb 11, 2024

@LinusBorg I don't get what you mean by the same trimmed value? how is it considered the same if the letters are removed?

After the letters are removed, you always end up with the same value as one input earlier. Look at my example:

  1. Type in 111 -> myNumber will be assigned '111'
  2. Type in 111a -> myNumber will be assigned '111' - which is the same value as before. So myNumber didn't actually change
  3. Type in 111aa -> again, myNumber will be assigned '111' - which is the same value as before.

so after 1., Vue has no reason to re-render and update the input value again - it's not aware that it has to.

And I was wondering if this is considered a bug and how can I contribute to fixing it if that's necessary?

I'd agree with Evan in #10254 that this is expected behavior in a way, as it is how Vue's reactivity works. As mentioned in my previous reply, I see a possibility for some optimization in defineModel's implementation to account for this scenario, yet I wonder if that's desirable. defineModel would then behave more differently than the code it was meant to be an abstraction for.

Contributing would start with properly understanding the cause of the issue and then talk about possible solutions and their implications with us.

ok then I will read more about the reactivity and what happens behind the scenes so I can contribute. Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants