feat: use `Session` to track IPCs and improve UI by hitarth-gg · Pull Request #268 · electron/devtron

hitarth-gg · 2025-07-02T15:47:59Z

This PR changes/adds a couple of things:

IPC messages from the ServiceWorker and Renderer processes to the Main process are now tracked via Session listeners for the -ipc-message, -ipc-invoke, and -ipc-message-sync events.
Improved ipcRenderer tracking: IPC events invoking on, off, once, addListener, removeListener, removeAllListeners methods on ipcRenderer are now properly tracked along with the method used.
Prevent multiple installations of Devtron.
Fixed the buggy Detail panel.
Detail panel can now be docked to bottom or right.
Added support for dark theme.
Added tooltips for Direction badges and Channel names.
Added LockToBottom feature to automatically scroll to the newly added event.
Fixed some other minor bugs as well.
Background service worker now stays alive without relying on any hacks or frequent pings. This was made possible by using serviceWorker.startTask(). As a result, the previous workarounds have been removed in favor of this approach.

IPC events going from Main to ServiceWorker are not tracked yet.

Dock to Right	Dock to Bottom

Dark Theme:

erickzhao · 2025-07-04T16:37:49Z

package.json

    "postcss": "^8.5.4",
    "postcss-loader": "^8.1.1",
    "postcss-preset-env": "^10.2.1",
+    "prettier": "^3.6.2",


With Prettier 3.6, we can enable the --experimental-cli flag for a performance boost.

ref electron/website#841

Done, added the linting scripts here - 1102a77

Edit: Ran the linters in this commit - 56da3ae

erickzhao · 2025-07-04T16:42:26Z

src/extension/background/index.ts

-/* ------------------------------------------------------ */
-/**
- * This is used to keep the background script alive. More testing is needed to determine whether it is needed or not
- * since other KEEP_ALIVE methods are already implemented in the content script and panel script.
- * Code copied from: https://stackoverflow.com/questions/66618136/persistent-service-worker-in-chrome-extension
- */
-const keepAlive = () => setInterval(chrome.runtime.getPlatformInfo, 20e3);
-chrome.runtime.onStartup.addListener(keepAlive);
-keepAlive();
-/* ------------------------------------------------------ */
-


It would be good to put an explainer in the PR body to explain why we don't need to keep the background script alive using this hack anymore.

Added a bullet point explaining the change.

erickzhao · 2025-07-04T16:43:24Z

src/lib/service-worker-preload.ts

+      // @ts-expect-error: addIpcEvent is available in the background service-worker
+      addIpcEvent(data);


nit: we might be able to add it to the global scope in TypeScript using https://www.typescriptlang.org/docs/handbook/declaration-files/templates/global-modifying-module-d-ts.html

Done, I declared it globally in global.d.ts - 4387be5

erickzhao · 2025-07-04T16:50:33Z

src/index.ts

+    const sw = await ses.serviceWorkers.startWorkerForScope(extension.url);
+    sw.startTask();
+    registerIpcListeners(ses, sw);
+  } catch (error_1) {


nit: any reason we're calling this error_1?

No particular reason tbh. I renamed error_1 and error_2 to error - 2425951

erickzhao · 2025-07-04T16:51:34Z

src/index.ts

+  if (isInstalled) {
+    throw new Error(
+      'Devtron has already been installed. Please avoid calling devtron.install() more than once.',
+    );
+  }


Wondering what everyone's thoughts are on silently returning vs. throwing an error. I think throwing might be a bit disruptive for the end user?

I agree, throwing might be a bit too much here. I replaced it with a simple return;. Let me know if you’d prefer I also add a console.warn() to display a warning. - 9e62744

…error

yangannyx · 2025-07-09T05:06:46Z

src/extension/react/context/context.tsx

+export default function DevtronProvider({ children }: DevtronProviderProps) {
+  // Theme
+  const [theme, setThemeState] = useState<Theme>('light');
+  const setTheme = (newTheme: Theme) => {


One of the gotchas with using react context is that when the provider value changes, React will re-render all components that call useContext(DevtronContext), or useDevtronContext() in this case. Right now, the setters and value object are getting recalculated on every render. It's important to avoid this performance footgun by making sure the provider value object is stable. To do this:

Memoize all of the setX callbacks using useCallback so that they aren't recalculated every render

Memoize the object you're passing to DevtronContext.Provider's value parameter using useMemo

Done! - 544bc34

yangannyx · 2025-07-09T05:09:00Z

src/extension/react/components/Panel.tsx

+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [],


You should be able to safely add setLockToBottom to this deps array after you make the DevtronProvider value stable! See comment below

Fixed - 544bc34

yangannyx · 2025-07-09T05:27:10Z

src/extension/react/components/Panel.tsx

+  const lockToBottomRef = useRef(lockToBottom);
+  const gridRef = useRef<AgGridReact<IpcEventDataIndexed> | null>(null);
+
  const portRef = useRef<chrome.runtime.Port | null>(null);


Is portRef being read anywhere?

It wasn't being used anywhere before but I ended up using it in clearEvents callback in this commit - f7b8436

yangannyx · 2025-07-09T05:44:46Z

src/extension/react/components/Panel.tsx

   * Comment out the useEffect below if you want to test the UI in dev mode on localhost
-   * and use JSON data from test_data/test_data.js for testing.
+   * and use JSON data from test_data/test_data.ts for testing.


It might be easier to set a local debug value at the top of the file so that developers don't need to comment out this entire block

const DEBUG = false function Panel() { ... useEffect (() => { if (DEBUG === true) { return } ... } ... }

Done, I added const isDev = process.env.NODE_ENV === 'development'; instead. The app now loads test_data dynamically during development mode. - de1c472

yangannyx · 2025-07-09T05:53:53Z

src/extension/react/components/Panel.tsx

+
  const portRef = useRef<chrome.runtime.Port | null>(null);
-  const pingIntervalRef = useRef<NodeJS.Timeout | null>(null);
  const clearEventsRef = useRef(() => {});


Using a useCallback is preferred for functions, it's a bit non-standard to have a callback conditionally set with a ref. If you want this callback to do nothing in dev mode, you can use the local debug value in the callback definition (see comment below). If you want this callback to do nothing until the first render useEffect finishes, you can track isPortReady in a useState, which you can set as the last step in the useEffect

Fixed it here - f7b8436

yangannyx

LGTM!

yangannyx · 2025-07-10T06:23:42Z

src/extension/react/components/Panel.tsx

  /**
   * Comment out the useEffect below if you want to test the UI in dev mode on localhost
   * and use JSON data from test_data/test_data.ts for testing.
   */


Nit: Is this comment still relevant?

I removed it here - 333d2d1

yangannyx · 2025-07-10T06:25:17Z

src/extension/react/components/Panel.tsx

  const lockToBottomRef = useRef(lockToBottom);
  const gridRef = useRef<AgGridReact<IpcEventDataIndexed> | null>(null);
-
+  const [isPortReady, setIsPortReady] = useState(false);


nit (non-blocking): I would co-locate this useState with the useStates higher up in the component for better organization

Done - 333d2d1

yangannyx · 2025-07-10T06:26:56Z

src/extension/react/components/Panel.tsx

+    if (isDev) {
+      setEvents([]);
+      return;
+    }
+    if (!isPortReady || !portRef.current) return;
+    try {
+      portRef.current.postMessage({ type: MSG_TYPE.CLEAR_EVENTS } satisfies MessagePanel);
+      setEvents([]);
+    } catch (error) {
+      console.error('Devtron - Error clearing events:', error);
+    }


nit (non-blocking): it'd be nice to add newlines to break up these statements for better readability

Done - 333d2d1 & 315efd9

samuelmaddock

Sorry for the late review, great progress!

samuelmaddock · 2025-07-10T15:26:39Z

src/index.ts

+          moduleType,
+          preloadFileName,
+        ),
+        type: 'service-worker',


Not needed, but an ID might be useful for folks who see this in ses.getPreloadScripts()

Suggested change

type: 'service-worker',

type: 'service-worker',

id: 'devtron-preload',

Added! - b572b8e

samuelmaddock · 2025-07-10T15:27:25Z

src/index.ts

+        path.resolve('node_modules', '@electron', 'devtron', 'dist', 'extension'),
+        { allowFileAccess: true },
+      );
+      startServiceWorker(ses, devtron);


Suggested change

startServiceWorker(ses, devtron);

await startServiceWorker(ses, devtron);

Fixed - 48da593

samuelmaddock · 2025-07-10T15:38:22Z

src/index.ts

+  }
+  isInstalled = true;
+
+  app.on('session-created', async (ses) => {


Maybe this should explicitly install to session.defaultSession initially in case it's already been created at the point install() is called.

I tried doing that here - 44a8f47

If this approach doesn't look right to you, I'm happy to explore other options if needed.

Update: The previous code flow here 44a8f47 caused Devtron to not install properly on newly created sessions.

To fix this, I updated the code to first add a listener for session-created, and only then attempt to install Devtron on the defaultSession - 87adb6e

samuelmaddock · 2025-07-10T15:43:53Z

src/index.ts

+      ses.registerPreloadScript({
+        filePath: path.resolve(
+          'node_modules',
+          '@electron',
+          'devtron',
+          'dist',
+          moduleType,
+          preloadFileName,
+        ),


If you define an export for the preload in the package.json, you can resolve it like this:

Suggested change

ses.registerPreloadScript({

filePath: path.resolve(

'node_modules',

'@electron',

'devtron',

'dist',

moduleType,

preloadFileName,

),

import { createRequire } from 'node:module';

const dirname = moduleType === 'esm' ? import.meta.dirname : __dirname;

const filePath = createRequire(dirname).resolve('@electron/devtron/preload');

ses.registerPreloadScript({

filePath,

Done! - 1de3100

hitarth-gg added 8 commits July 2, 2025 14:15

feat: use Session to track IPCs and improve UI

aacbd32

feat: improve ipcRenderer tracking logic

e9ebfa8

chore: fix renderer tooltip

a325be3

feat: change Direction badge style and add tooltip for Channel

49b9278

chore: update comment

802f6a9

feat: update tooltip settings

9532eb1

feat: prevent multiple installations

1061de2

feat: track ipcRenderer.off

6d79297

hitarth-gg marked this pull request as ready for review July 2, 2025 16:54

chore: update README.md

e810813

erickzhao reviewed Jul 4, 2025

View reviewed changes

hitarth-gg added 5 commits July 6, 2025 23:52

chore: silently return on multiple installations instead of throwing …

9e62744

…error

refactor: rename error_1 and error_2 to error

2425951

chore: globally declare addIpcEvent()

4387be5

chore: add lint scripts

1102a77

chore: run linters

56da3ae

yangannyx reviewed Jul 9, 2025

View reviewed changes

chore: memoize context value and setters

544bc34

hitarth-gg force-pushed the feat/use-session branch from 07d40cb to 544bc34 Compare July 9, 2025 20:24

hitarth-gg added 3 commits July 10, 2025 01:59

chore: run prettier

6b6e2ea

chore: replace ref-based clearEvents with useCallback

f7b8436

chore: simplify dev testing

de1c472

yangannyx approved these changes Jul 10, 2025

View reviewed changes

hitarth-gg added 2 commits July 10, 2025 13:54

chore: cleanup

333d2d1

chore: cleanup

315efd9

samuelmaddock reviewed Jul 10, 2025

View reviewed changes

hitarth-gg added 4 commits July 11, 2025 16:42

chore: add id for devtron preload script

b572b8e

chore: await startServiceWorker

48da593

chore: resolve preload path using dirname

1de3100

chore: explicitly install devtron to session.defaultSession

44a8f47

fix: ensure devtron installs on newly created sessions

87adb6e

erickzhao approved these changes Jul 15, 2025

View reviewed changes

samuelmaddock approved these changes Jul 15, 2025

View reviewed changes

samuelmaddock merged commit e313ceb into next Jul 15, 2025

dsanders11 deleted the feat/use-session branch July 19, 2025 01:21

hitarth-gg mentioned this pull request Jul 19, 2025

[GSoC] Devtron: Progress, Known Issues, and Next Steps #272

Open

23 tasks

dsanders11 added the GSoC 2025 label Jul 22, 2025

		// @ts-expect-error: addIpcEvent is available in the background service-worker
		addIpcEvent(data);

	type: 'service-worker',
	type: 'service-worker',
	id: 'devtron-preload',

	startServiceWorker(ses, devtron);
	await startServiceWorker(ses, devtron);

Conversation

hitarth-gg commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hitarth-gg Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hitarth-gg Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hitarth-gg Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangannyx Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangannyx Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangannyx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hitarth-gg Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelmaddock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hitarth-gg commented Jul 2, 2025 •

edited

Loading

hitarth-gg Jul 7, 2025 •

edited

Loading

hitarth-gg Jul 6, 2025 •

edited

Loading

hitarth-gg Jul 6, 2025 •

edited

Loading

yangannyx Jul 9, 2025 •

edited

Loading

yangannyx Jul 9, 2025 •

edited

Loading

hitarth-gg Jul 10, 2025 •

edited

Loading