clean up

Author: nicolethoen

packages/module/package.json

@@ -35,7 +35,7 @@
     "tag": "prerelease"
   },
   "dependencies": {
-    "@novnc/novnc": "^1.5.0",
+    "@novnc/novnc": "1.5.0",
     "@patternfly/react-core": "^6.0.0",
     "@patternfly/react-styles": "^6.0.0",
     "@spice-project/spice-html5": "^0.2.1",

packages/module/patternfly-docs/content/extensions/react-console/examples/SerialConsoleCustom.jsx

@@ -1,4 +1,4 @@
-import { useState, useRef, createRef } from 'react';
+import React, { useState, useRef, createRef, useCallback } from 'react';
 import { debounce } from '@patternfly/react-core';
 import { SerialConsole } from '@patternfly/react-console';
 
@@ -7,16 +7,24 @@ export const SerialConsoleCustom = () => {
   const setConnected = useRef(debounce(() => setStatus('connected'), 3000)).current;
   const ref2 = createRef();
 
+  const onConnect = useCallback(() => {
+    setStatus('loading');
+    setConnected();
+  }, [setConnected]);
+
+  const onDisconnect = useCallback(() => {
+    setStatus('disconnected');
+  }, []);
+
+  const onData = useCallback((data) => {
+    ref2.current?.onDataReceived?.(data);
+  }, []);
+
   return (
     <SerialConsole
-      onConnect={() => {
-        setStatus('loading');
-        setConnected();
-      }}
-      onDisconnect={() => setStatus('disconnected')}
-      onData={data => {
-        ref2.current.onDataReceived(data);
-      }}
+      onConnect={onConnect}
+      onDisconnect={onDisconnect}
+      onData={onData}
       status={status}
       ref={ref2}
     />

packages/module/src/components/SerialConsole/XTerm.tsx

@@ -80,6 +80,11 @@ export const XTerm: React.FunctionComponent<XTermProps> = ({
   }, [onBeforeUnload]);
 
   useEffect(() => {
+    const container = ref.current;
+    if (!container) {
+      return;
+    }
+
     const fitAddon = new FitAddon();
     terminalRef.current = new Terminal({
       cols,
@@ -92,8 +97,8 @@ export const XTerm: React.FunctionComponent<XTermProps> = ({
 
     const onWindowResize = () => {
       const geometry = fitAddon.proposeDimensions();
-      if (geometry) {
-        terminalRef.current.resize(geometry.rows, geometry.cols);
+      if (geometry && terminalRef.current) {
+        terminalRef.current.resize(geometry.cols, geometry.rows);
       }
     };
 
@@ -107,29 +112,45 @@ export const XTerm: React.FunctionComponent<XTermProps> = ({
 
     terminalRef.current.loadAddon(fitAddon);
 
-    terminalRef.current.open(ref.current);
+    let resizeListener: (() => void) | null = null;
 
-    const resizeListener = debounce(onWindowResize, 100);
-
-    if (!rows) {
-      if (canUseDOM) {
+    // Defer open so the container is laid out and xterm's renderer has valid dimensions
+    // before Viewport._innerRefresh runs (avoids "Cannot read properties of undefined (reading 'dimensions')")
+    const rafId = requestAnimationFrame(() => {
+      if (!container.isConnected || !terminalRef.current) {
+        return;
+      }
+      terminalRef.current.open(container);
+      resizeListener = !rows ? debounce(onWindowResize, 100) : null;
+      if (resizeListener && canUseDOM) {
         window.addEventListener('resize', resizeListener);
       }
-      onWindowResize();
-    }
-    terminalRef.current.focus();
+      if (!rows) {
+        onWindowResize();
+      }
+      terminalRef.current?.focus();
+    });
 
     return () => {
-      terminalRef.current.dispose();
-      if (canUseDOM) {
+      cancelAnimationFrame(rafId);
+      if (resizeListener && canUseDOM) {
         window.removeEventListener('resize', resizeListener);
       }
+      terminalRef.current?.dispose();
+      terminalRef.current = null;
       onFocusOut();
     };
   }, [cols, fontFamily, fontSize, onData, onFocusOut, onTitleChanged, rows]);
 
   // ensure react never reuses this div by keying it with the terminal widget
   // Workaround for xtermjs/xterm.js#3172
-  return <div ref={ref} role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;
+  return (
+    <div
+      ref={ref}
+      role="list"
+      onFocus={onFocusIn}
+      onBlur={onFocusOut}
+    />
+  );
 };
 XTerm.displayName = 'XTerm';

packages/module/src/components/VncConsole/VncConsole.tsx

@@ -94,6 +94,7 @@ export const VncConsole: React.FunctionComponent<VncConsoleProps> = ({
   textCtrlAltDel
 }) => {
   const rfb = useRef<any>(null);
+  const rfbDisconnectedRef = useRef(false);
 
   const novncElem = useRef<HTMLDivElement>(null);
   const [status, setStatus] = useState(CONNECTING);
@@ -104,6 +105,7 @@ export const VncConsole: React.FunctionComponent<VncConsoleProps> = ({
 
   const _onDisconnected = useCallback(
     (e: any) => {
+      rfbDisconnectedRef.current = true;
       setStatus(DISCONNECTED);
       onDisconnected(e);
     },
@@ -141,6 +143,10 @@ export const VncConsole: React.FunctionComponent<VncConsoleProps> = ({
   }, [rfb, _onDisconnected, _onSecurityFailure]);
 
   const connect = useCallback(() => {
+    if (!novncElem.current) {
+      return;
+    }
+    rfbDisconnectedRef.current = false;
     const protocol = encrypt ? 'wss' : 'ws';
     const url = `${protocol}://${host}:${port}/${path}`;
 
@@ -175,22 +181,26 @@ export const VncConsole: React.FunctionComponent<VncConsoleProps> = ({
 
   useEffect(() => {
     initLogging(vncLogging);
-    try {
-      connect();
-    } catch (e) {
-      onInitFailed && onInitFailed(e);
-      rfb.current = undefined;
-    }
+    // Defer RFB creation so the container is laid out and has dimensions (avoids noVNC "dimensions" undefined)
+    const rafId = requestAnimationFrame(() => {
+      try {
+        connect();
+      } catch (e) {
+        onInitFailed && onInitFailed(e);
+        rfb.current = undefined;
+      }
+    });
 
     return () => {
+      cancelAnimationFrame(rafId);
       disconnect();
       removeEventListeners();
       rfb.current = undefined;
     };
   }, [connect, onInitFailed, removeEventListeners, vncLogging]);
 
   const disconnect = () => {
-    if (!rfb.current) {
+    if (!rfb.current || rfbDisconnectedRef.current) {
       return;
     }
     rfb.current.disconnect();

yarn.lock

@@ -3432,10 +3432,10 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@novnc/novnc@npm:^1.5.0":
-  version: 1.6.0
-  resolution: "@novnc/novnc@npm:1.6.0"
-  checksum: 10c0/b014445548b769e66e33f0f60ec6a1676aeff9cad509c371b4fa6b00b496a8b8a9df15707411230df72ed205d819eca08c6555b67717a805fb8043045c831d08
+"@novnc/novnc@npm:1.5.0":
+  version: 1.5.0
+  resolution: "@novnc/novnc@npm:1.5.0"
+  checksum: 10c0/811a1b3b6a0bfbef3092babb8b1b1febc0273c79abd1902231558194d64125d5fa7f36219e45d94b78fbae6f5444bc39637909f4aa7d96691479dca27b9052c9
   languageName: node
   linkType: hard
 
@@ -3636,7 +3636,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@patternfly/react-console@workspace:packages/module"
   dependencies:
-    "@novnc/novnc": "npm:^1.5.0"
+    "@novnc/novnc": "npm:1.5.0"
     "@patternfly/documentation-framework": "npm:6.0.0-alpha.108"
     "@patternfly/patternfly": "npm:^6.0.0"
     "@patternfly/patternfly-a11y": "npm:^4.3.1"