brimdata / zui

Zui is a powerful desktop application for exploring and working with data. The official front-end to the Zed lake.
https://www.brimdata.io/download/
Other
1.78k stars 129 forks source link

React Devtools not installing #2604

Open philrz opened 1 year ago

philrz commented 1 year ago

Repro is with Brim commit 0a07e2a.

As shown in the video below, when starting the app in Dev mode via yarn start, several error messages now appear that were not seen in the past.

https://user-images.githubusercontent.com/5934157/205149706-c6a93a09-0341-4913-aafc-ac9deedf45cb.mp4

Further digging by @jameskerr and myself tracked down that this seems to be caused by React Devtools failing to install. I've confirmed that when I apply this patch:

diff --git a/src/js/electron/extensions.ts b/src/js/electron/extensions.ts
index 87bd194f..a5583da8 100644
--- a/src/js/electron/extensions.ts
+++ b/src/js/electron/extensions.ts
@@ -1,6 +1,5 @@
 import env from "src/app/core/env"
 import installExtension, {
-  REACT_DEVELOPER_TOOLS,
   REDUX_DEVTOOLS,
 } from "electron-devtools-installer"

@@ -9,7 +8,7 @@ import log from "electron-log"
 export function installExtensions() {
   if (env.isIntegrationTest) return
   // @ts-ignore The types package is not up to date
-  return installExtension([REDUX_DEVTOOLS, REACT_DEVELOPER_TOOLS], {
+  return installExtension([REDUX_DEVTOOLS], {
     loadExtensionOptions: {allowFileAccess: true},
   })
     .then(() => log.info("Devtools loaded"))

...the errors shown in the video no longer appear. Those errors:

[17129:1201/120633.155887:ERROR:extensions_browser_client.cc(58)] Extension Error:
  OTR:     false
  Level:   2
  Source:  chrome-extension://ckmekjigllimnjlldimhgomdoepmgfok/build/background.js
  Message: Uncaught TypeError: Cannot read properties of undefined (reading 'registerContentScripts')
  ID:      ckmekjigllimnjlldimhgomdoepmgfok
  Type:    RuntimeError
  Context: chrome-extension://ckmekjigllimnjlldimhgomdoepmgfok/build/background.js
  Stack Trace: 
    {
      Line:     107
      Column:   1
      URL:      chrome-extension://ckmekjigllimnjlldimhgomdoepmgfok/build/background.js
      Function: (anonymous function)
    }
[17129:1201/120633.156312:ERROR:extensions_browser_client.cc(58)] Extension Error:
  OTR:     false
  Level:   1
  Source:  manifest.json
  Message: Service worker registration failed
  ID:      ckmekjigllimnjlldimhgomdoepmgfok
  Type:    ManifestError

@jameskerr explained that the electron-devtools-installer always installs the current version of the requested DevTools, so this unfortunately leaves us exposed to situations like this where the latest version may cause us problems. I do see at https://www.npmjs.com/package/react-devtools that they tagged their latest version just a few days ago, so this does indeed seem to line up with when I started noticing these errors.

I did some searches and can't seem to find a way to get electron-devtools-installer to "pin" a particular version instead. This is not my area of expertise, though.

I also did some searches of the open issues for electron-devtools-installer and react-devtools looking for anything that might explain my specific error messages. When looking for references to "manifest", I did find https://github.com/MarshallOfSound/electron-devtools-installer/issues/229 which pointed to https://github.com/facebook/react/pull/25753 which has the changes for the latest React Devtools, and that includes https://github.com/facebook/react/pull/25145 which are changes that reference "manifest" stuff. I'm not certain this is the root cause and if so whether it places the blame on React Devtools or on how we try to use it, but figured I'd point it out.

Bartel-C8 commented 1 year ago

It's an Electron bug: https://github.com/electron/electron/issues/36545

philrz commented 1 year ago

Thanks very much @Bartel-C8!