Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

How to deal with ref attributes? #116

Closed
SantosGuillamot opened this issue Dec 2, 2022 · 1 comment
Closed

How to deal with ref attributes? #116

SantosGuillamot opened this issue Dec 2, 2022 · 1 comment

Comments

@SantosGuillamot
Copy link
Contributor

I was running the stress tests on some sites, and I found the same hydrationError in all of them: TypeError: Cannot create property 'current' on string 'MobileMenu' (the string varies between them).

I've been triaging it, and I believe this error occurs because they have a ref attribute in some HTML elements. As ref is also used by Preact, I assume at some point it tries to do something like ref.current === 'string' and produces this error.

Some sites are adding ref because they use other libraries like Vuejs or plugins like Curator.io, which also need the ref attribute.

This only happens when I run the stress testing script. If I paste the manual script into the browser console, I cannot reproduce it. I've checked the vDom with @DAreRodz, and the ref property is there, but it doesn't throw any error.

I tested 4000 sites and 8 of them had this issue:

  • kxbox.com: TypeError: Cannot create property 'current' on string 'pc.kx.idx.banner1.ljxz'
  • jhamiba.com: TypeError: Cannot create property 'current' on string 'MobileMenu'
  • meidouya.com: TypeError: Cannot create property 'current' on string 'MobileMenu'
  • wpdaxue.com: TypeError: Cannot create property 'current' on string 'MobileMenu'
  • 22vd.com: TypeError: Cannot create property 'current' on string 'nofollow'
  • knewsmart.com: TypeError: Cannot create property 'current' on string 'MobileMenu'
  • mfisp.com: TypeError: Cannot create property 'current' on string 'MobileMenu'
  • pro-q.it: TypeError: Cannot create property 'current' on string 'image'

Any idea about how should we approach this?

@DAreRodz
Copy link
Collaborator

@SantosGuillamot, I've been taking a look at this issue, and I think we can ignore the ref attribute while generating virtual nodes, i.e., something like:

diff --git a/src/runtime/vdom.js b/src/runtime/vdom.js
index 5657128..b11cec9 100644
--- a/src/runtime/vdom.js
+++ b/src/runtime/vdom.js
@@ -24,6 +24,8 @@ export default function toVdom(node) {
 			const [, prefix, suffix] = /wp-([^:]+):?(.*)$/.exec(n);
 			wpDirectives[prefix] = wpDirectives[prefix] || {};
 			wpDirectives[prefix][suffix || 'default'] = val;
+		} else if (n === 'ref') {
+			continue;
 		} else {
 			props[n] = attributes[i].value;
 		}

What does this imply? Practically nothing, because even if the ref attribute is not saved in the props generated by running toVdom(), the attribute remains in the DOM node after hydrating; it is not removed.

There could be problems if that node is in any part of the DOM that is unmounted and mounted again, in which case there would be two possibilities:

  1. The third-party library that handles the ref attribute has already been run before hydrate, so the changes it makes are most likely already in the DOM, and everything will work as usual.
  2. This third-party library runs after hydration; if that happens, when unmounting and re-mounting the node, it is most likely that this third-party library will not be executed again, so it doesn't matter if ref is there or not. 🤷

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

No branches or pull requests

2 participants