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

Add flag to disable reactivity #1060

Closed
wants to merge 2 commits into from
Closed

Add flag to disable reactivity #1060

wants to merge 2 commits into from

Conversation

basvanmeurs
Copy link
Contributor

In Vugel we noticed that when using a template ref <container ref="test" /> and then use the ref function rather than shallowRef function:

const test = ref("test");

We get a proxy to the node.
When invoking any method on test, the proxyfication of it causes the JIT optimizer to deopt functions, leading to performance problems. This problem doesn't really occur in vue dom itself, but it does affect all javascript-based custom renderers.

We'd like developers to always use shallowRef for template refs to avoid these problems, so we'd like to disable reactivity. Judging by the source code it can be fixed by marking every node as raw using markAsRaw, but that would affect performance. Internally _isVNode and _isVue are used for a similar reason. _isVue doesn't seem to be set anywhere, so I guess it's old and should be removed?! For our own use case this PR adds the _notReactive flag. We can simply set it on our prototype.

By the way, the Composition API describes how to use template refs. Shouldn't it use shallowRef rather than ref? It doesn't seem to make sense to make all (HTML) element attributes reactive? The typical use case is that a developer wants to get a reference to the template.

@yyx990803
Copy link
Member

markRaw simply adds the item to a WeakMap, there really isn't too much of a perf cost - if any, it's not that different from a property existence check.

Native DOM nodes or any browser native objects are not observed because they wouldn't pass the isObservableType check - they'd return something like [object HTMLXXXElement].

@yyx990803 yyx990803 closed this Apr 30, 2020
@basvanmeurs
Copy link
Contributor Author

markRaw simply adds the item to a WeakMap, there really isn't too much of a perf cost - if any, it's not that different from a property existence check.

Thanks for your response Evan, but can you please reconsider it?

Looking up in a WeakSet can be more costly than a simple property lookup. Especially when there are many children. To illustrate, I tried calling the markRaw in the createElement for all children individually. I profiled that 50% of all createElements time now ends up in the markRaw method, and 10% of all total time (!!). The property method (just like is used with _isVNode) costs no performance.

In our case feels it like a waste, especially as Vugel is all about performance. This is not a solution for our use case, and the alternative for us is to not check at all and put a notice in the docs. Which is ok, but not as elegant..

markRaw

@yyx990803
Copy link
Member

Hmm, I didn't realize WeakSet/Map addition can be this expensive at large sizes. We may have to reconsider internal reactive-ness tracking cost of this too.

I am a bit torn on "magic hidden flag exposed as public API"... feels dirty. I'll think about this in the next beta release.

@yyx990803
Copy link
Member

Did a refactor in d901b6b - this technically allows you to use __v_skip to mark an object as raw.

@basvanmeurs
Copy link
Contributor Author

That's awesome. Thanks!

basvanmeurs added a commit to Planning-nl/vugel that referenced this pull request May 4, 2020
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.

2 participants