WARNING: DO NOT USE QUERYSELECTOR

I don't really mean that. We encountered a case where querySelector crashed our webapp but in general it's a great tool that in most cases acceptable.

I was looking through some obscure error messages in sentry (error monitoring) and came over a rare but consistent error message: VM691:1 Uncaught DOMException: Failed to execute 'querySelector'.

This error crashes the whole app and sends the user to a 500 error page. Not good.

As always I thought "probably some weird safari bug, nothing to worry about" and went into debugging mode. It was not safari, but just how querySelector works.

Background

At NHI we have content behind a paywall and lazy loads the content client side. This means that anchor links are not present at the page when the page is "finished loading", so we have implemented a solution that tries to jump to the anchor until a user do some actions on the page (scroll, click ...). It works, but it's a hack.

Here is the original code:

1/**
2 * This component needs to exist since we lazy-load content.
3 * If there is a #hash we continiously try to scroll the element into view until 5 seconds
4 * or the user interacts with the page
5 */
6export default function useEventualScroll() {
7     React.useEffect(() => {
8          if (!window.location.hash) {
9               return
10          }
11          // removes ? from the hash
12          const hash = window.location.hash.replace(/\?.*/, "")
13          const el = document.querySelector(hash)
14          if (!el) {
15               return
16          }
17
18          const intervalId = setInterval(() => {
19               el.scrollIntoView(true)
20          }, 50)
21
22          const timeoutId = setTimeout(() => {
23               clearInterval(intervalId)
24          }, 5000)
25
26          const disconnect = () => {
27               clearTimeout(timeoutId)
28               clearInterval(intervalId)
29          }
30
31          window.addEventListener("mousedown", disconnect, { once: true })
32          window.addEventListener("keydown", disconnect, { once: true })
33          window.addEventListener("touchmove", disconnect, { once: true })
34          window.addEventListener("wheel", disconnect, { once: true })
35          return () => {
36               disconnect()
37               window.removeEventListener("mousedown", disconnect)
38               window.removeEventListener("keydown", disconnect)
39               window.removeEventListener("touchmove", disconnect)
40               window.removeEventListener("wheel", disconnect)
41          }
42     }, [])
43}

The problem

So what is the problem? The first problem is that the anchor tags are generated by our CMS (editors..) and they do sometimes things not even the greatest QA could anticipate.

The second problem is that we used querySelector wrong where we should have used getElementById instead.

How querySelector works

the spec states that querySelector takes a valid selector string and if it fails it should throw an exception.

The spec states

To scope-match a selectors string selectors against a node, run these steps:

      1. Let s be the result of parse a selector selectors.

      2. If s is failure, then throw a "SyntaxError" DOMException.

      3. Return the result of match a selector against a tree with s and node’s root using scoping root node.

And the selector string need to be a valid css selector as specced. In short, this means that the query need to be valid.

When I think about it, it's in the same already but I have always thought about it as a cool catch all replacement of getElementByX.

This is valid: document.querySelector("#oppfolging-show")

This will throw error: document.querySelector("#oppfolging-show.")

This is valid: document.getElementById("#oppfolging-show.")

This is valid: document.getElementById("#oppfolging-show")

The fix

The fix is extremely easy.

1. We know that the anchor always is an id
2. getElementById is not victim of the constraints that makes querySelector crash our app

Ideally we should make the anchor tags querySelector friendly, but no one wants to delve into the legacy CMS to fix this.

Instead we just use getElementById instead. No more crashes!

Final code

1/**
2 * This component needs to exist since we lazy-load content.
3 * If there is a #hash we continiously try to scroll the element into view until 5 seconds
4 * or the user interacts with the page
5 */
6export default function useEventualScroll() {
7     React.useEffect(() => {
8          if (!window.location.hash) {
9               return
10          }
11          // This regex removes everything after ? and replaces # with "" since we now look after ids without the # prefix
12          const hash = window.location.hash.replace(/(\?.*)|\#/, "")
13          const el = document.getElementById(hash)
14          if (!el) {
15               return
16          }
17
18          const intervalId = setInterval(() => {
19               el.scrollIntoView(true)
20          }, 50)
21
22          const timeoutId = setTimeout(() => {
23               clearInterval(intervalId)
24          }, 5000)
25
26          const disconnect = () => {
27               clearTimeout(timeoutId)
28               clearInterval(intervalId)
29          }
30
31          window.addEventListener("mousedown", disconnect, { once: true })
32          window.addEventListener("keydown", disconnect, { once: true })
33          window.addEventListener("touchmove", disconnect, { once: true })
34          window.addEventListener("wheel", disconnect, { once: true })
35          return () => {
36               disconnect()
37               window.removeEventListener("mousedown", disconnect)
38               window.removeEventListener("keydown", disconnect)
39               window.removeEventListener("touchmove", disconnect)
40               window.removeEventListener("wheel", disconnect)
41          }
42     }, [])
43}