[DevTools Bug]: React Dev Tools breaks craigslist?

ericandrewlewis commented 1 year ago

Website or app

https://newyork.craigslist.org/search/hhh

Repro steps

Enable React Dev Tools
Visit https://newyork.craigslist.org/search/hhh
50% of the time you will see broken page https://imgur.com/a/yJPeAvA
Disable React Dev Tools
Visit https://newyork.craigslist.org/search/hhh
100% of the time you will see a working page

How often does this bug happen?

Often

DevTools package (automated)

No response

DevTools version (automated)

No response

Error message (automated)

No response

Error call stack (automated)

TypeError: svs-boot-setManifest-exception:Cannot read properties of null (reading 'insertBefore')
    at cl.injectCss (bbe8a523a089547e594fa2f101021699a377645c.js:6:12097)
    at cl.injectResource (bbe8a523a089547e594fa2f101021699a377645c.js:6:14293)
    at bbe8a523a089547e594fa2f101021699a377645c.js:6:20658
    at Array.forEach (<anonymous>)
    at injectResourceSet (bbe8a523a089547e594fa2f101021699a377645c.js:6:20634)
    at bigBang (bbe8a523a089547e594fa2f101021699a377645c.js:6:17286)
    at cl.setManifest (bbe8a523a089547e594fa2f101021699a377645c.js:6:18123)
    at manifest.js:1:4
cl.unexpected @ bbe8a523a089547e594fa2f101021699a377645c.js:6
cl.setManifest @ bbe8a523a089547e594fa2f101021699a377645c.js:6
(anonymous) @ manifest.js:1
VM1218:1 Uncaught SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at localStorage-092e9f9e2f09450529e744902aa7cdb3a5cc868d.html:38:37

Error component stack (automated)

No response

GitHub query string (automated)

No response

mondaychen commented 1 year ago

This is because React DevTools extension needs to insert a script tag at the beginning of the page to make sure it can connect to React. In the craigslist page you provided, it tries to insert before the parent node of the first script tag.

function getInsertPoint() {
      try {
        return doc.getElementsByTagName("script")[0];
      } catch (e) {}
      return 0;
    }

then

var insertPoint = getInsertPoint();
      (cl.injectCss = function (e, t) {
        if (!cl.onCssComplete.lockExists(e)) {
          var o = doc.createElement("link");
          (o.type = "text/css"),
            (o.rel = "stylesheet"),
            (o.href = e),
            (o.onload = function () {
              t && t(), cl.onCssComplete.unlock(e);
            }),
            cl.onCssComplete.lock(e),
            insertPoint.parentNode.insertBefore(o, insertPoint);
        }
      })

So when you have the extension installed, there's a chance the first script tag is the devtools one instead of their own.

I'm not sure why it's done this way. If they try to find the <head> element, document.head is much safer and easier.

Unfortunately, there's no way we can change our behavior in near future. If you can get in touch with craigslist developers, please pass on my advice. Thanks.

ericandrewlewis commented 1 year ago

Hi @mondaychen, thanks for taking a look at this.

Do I understand that React DevTools extension inserts a script tag at the beginning of the page, and then removes it from the document at some later point in its lifecycle?

So craigslist' code is looking for a place in the DOM to inject a <link> tag asynchronously. Likely, some of the time (50%) the React DevTools script tag still exists in the DOM, so craigslist injection works okay. The other half, the DevTools script tag is gone, which throws the error.

Good point for craigslist code to modify the getInsertPoint() implementation to target a more stable DOM selector for injection like document.head.

ericandrewlewis commented 1 year ago

I filled out craigslist's contact form describing the issue and linking here, hopefully it gets routed correctly ¯_(ツ)_/¯

An alternative solution would be if React DevTools can leave the script tag it inserts into the DOM?

mondaychen commented 1 year ago

An alternative solution would be if React DevTools can leave the script tag it inserts into the DOM?

I don't think that's any better. That would break some other code that looks for the first element later (I've seen that). We are actually discussing a new approach that doesn't require inserting the script early. We'll get rid of this problem if that's done.

facebook / react