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

Feature - deep $watch #2462

Merged
merged 3 commits into from Dec 20, 2021
Merged

Feature - deep $watch #2462

merged 3 commits into from Dec 20, 2021

Conversation

SimoTod
Copy link
Collaborator

@SimoTod SimoTod commented Dec 7, 2021

This PR enables deep watching by default. JSON.stringify(value) will touch any single property (or element for arrays) so they'll get picked up by the reactivity framework.

Pending questions before merging:
VueJs doesn't offer it by default but it has a deep flag. To me, it makes more sense to just make it deep by default which is what most of the users asked for, I think. However, I'm happy to introduce the flag if we think it's better, either defaulted to false like VueJs or to true.
If the watcher changes a different property on the same object, it will trigger an infinite loop, should we introduce a control variable to pause the watcher during the change?

@ryangjchandler
Copy link
Contributor

What's the performance impact of using JSON.stringify on a larger data object, perhaps an array with plenty of elements?

@SimoTod
Copy link
Collaborator Author

SimoTod commented Dec 7, 2021

I would expect we'll need a very huge dataset to make an appreciable difference and at that point, Alpine itself would have issue because it needs to crawl the same number of nodes to convert them to proxies. But it's a good argument for making it optional.

For real arrays, we are already crawling all elements to convert them to strings to be fair.

@ryangjchandler
Copy link
Contributor

Yeah, I'd personally opt for the Vue-like API with the { deep: true } third argument myself. Would also open up opportunities in the future for an immediate prop that would really be like an Alpine.effect() call.

@SimoTod
Copy link
Collaborator Author

SimoTod commented Dec 7, 2021

https://codepen.io/SimoTod/pen/jOGqzBB this uses the current implementation
https://codepen.io/SimoTod/pen/XWedEpE?editors=1111 uses the deep watcher
The performance impact is slightly noticeable with 100000 elements and with 1000000 is obvious.
Those datasets are probably unusual in Alpine components and not sure what a watch function would do there but we might need that flag, I'll wait for Caleb though. It's just a bit inconsistent that real arrays are already deep watched while nested objects will need a flag. Maybe the flag should be defaulted to true and people can turn it to false if they want a shallow watch because they have performance issues.

@ryangjchandler
Copy link
Contributor

Nice, thanks for the benchmark Simone. Yeah, in this case then, I'd just always have deep watching enabled.

@calebporzio
Copy link
Collaborator

I like this as the default, thanks for checking on those things @ryangjchandler and @SimoTod - and thanks for actually benchmarking the difference @SimoTod - great addition!

@calebporzio calebporzio merged commit eafa3db into alpinejs:main Dec 20, 2021
@earthboundkid
Copy link
Contributor

You could test for structuredClone and use it where available (just FF at the moment).

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.

None yet

4 participants