Skip to content

fix(runtime-core): when props is typeof ref, they lose data and don't stay responsive #7157

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

Closed
wants to merge 2 commits into from

Conversation

yingpengsha
Copy link

Add the test cases for which props will be specially handled

@pikax pikax added the 🧹 p1-chore Priority 1: this doesn't change code behavior. label Oct 20, 2023
@skirtles-code
Copy link
Contributor

I'm curious what the goal is here. Why is it important that the props aren't wrapped in proxies?

I believe this will come into conflict with #8222, which wraps them in a proxy. If there's a compelling reason why those objects must not be readonly proxies then it may be necessary to tweak that PR.

@yingpengsha
Copy link
Author

After such a long time, my memory has deviated.

I can only be certain of one thing: there was an unexpected event where reactive props caused the component to lose its reactive state.

However, upon re-examination, I've identified another issue. When declaring a view in this manner: h(Component, Ref), something gets lost.

This issue can be traced back to the guardReactiveProps function. I've revised the changes in the PR to include compatibility logic.

However, it seems that this does not quite match the results I initially observed. Whatever the case, this experience teaches us that whenever we submit a PR or ISSUE, we should thoroughly document the background and process to prevent forgetting details over time.

:)

/cc @skirtles-code

@yingpengsha yingpengsha changed the title test(props): the props are not reactivity in funcational component fix(runtime-core): when props is typeof ref, they lose data and don't stay responsive Apr 22, 2024
@skirtles-code
Copy link
Contributor

It seems that the new changes you've pushed are unrelated to the original contents of the PR. It would probably have been better to open a new PR for these rather than reusing this one. The previous conversations and labels on this PR no longer make sense in the context of the new changes.

I'm not sure whether using h(Component, Ref) would be considered a valid use case. Could you provide more details of the actual use case? A playground example at https://play.vuejs.org/ might help us to understand why this is needed.

I also think you'd need to add a test case if you want to introduce this change.

@yingpengsha
Copy link
Author

I will recall and reconsider the necessity of this change. If new information arises, I will submit a new PR to elaborate on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 p1-chore Priority 1: this doesn't change code behavior.
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

3 participants