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

feat: improve reactive checks #502

Merged
merged 9 commits into from
Sep 12, 2020

Conversation

pikax
Copy link
Member

@pikax pikax commented Sep 4, 2020

Instead of changing reactive object we just use a weak set to keep track on what objects are reactive

This allows to have arrays as reactive, bringing it closer to vue 3

Still need to:

  • Extensive testing

Close #501, close #219

@pikax pikax requested a review from antfu September 4, 2020 15:55
@pikax pikax marked this pull request as ready for review September 5, 2020 07:49
},
})
}
// TODO way to track new array items
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we hijack the array methods or just leave a caveat in the readme?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is quite interesting idea, just thinking we might need to listen to other "events" to keep it consistent, for example, if you assign an array to an object, are we hijacking that array?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean "events"? If I recall correctly, Vue 2 hijacked the array's push/unshift, etc to achieve the reactivity updates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you add a new property to an object or when you change the property of the obj to an array

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, I got what you mean. And yeah, if we handle the hijacking here and wrap the value with markReactive for newly added properties, they should be well covered I think.

@antfu antfu merged commit 255dc72 into vuejs:master Sep 12, 2020
@pikax pikax deleted the feat/improve_reactive_implementation branch September 12, 2020 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"reactive" doesn't work well with Array
2 participants