Increase debounce time, add flush on blur, ui tweaks (#167)

* UI perf tests

* Flush local value

* Fixes for format

* More tests for filagg panel

* UI improvements, table cleanup

* Fullscreen in url

* Fix for format

* Update icons

* Correct guesses and timing of graph/table offers

* Small fixes

* Drop timeout

* Sync to localstorage on desktop

* issues with localstorage

* Add race detector everywhere

* Fixes for localstorage lookups

* Update dev_faq.md

* Update architecture doc

* Fix for tests

* Pipe instead

* Fix conn test

* Add notes about macos race bug

* Add note about minor version

.github/workflows/pull_requests.yml

@@ -41,7 +41,7 @@ jobs:
         SNOWFLAKE_USER: ${{ secrets.SNOWFLAKE_USER }}
         SNOWFLAKE_PASSWORD: ${{ secrets.SNOWFLAKE_PASSWORD }}
         SNOWFLAKE_ACCOUNT: ${{ secrets.SNOWFLAKE_ACCOUNT }}
-    - run: cd runner && go test -coverprofile ../coverage/gounit.cov
+    - run: cd runner && go test -race -coverprofile ../coverage/gounit.cov
     - run: ./runner/scripts/test_coverage.sh
     # Test server builds
     - run: yarn build-server
@@ -134,8 +134,9 @@ jobs:
     - run: ./scripts/ci/prepare_macos.sh
     # Needed so we can have ./build/desktop_runner.js ready for tests
     - run: yarn build-test-runner
-    - run: yarn test --runInBand --detectOpenHandles --forceExit --verbose shared ui desktop
-    - run: cd runner && go test -cover
+    # https://github.com/golang/go/issues/49138 bug in macos -race detector
+    - run: MallocNanoZone=0 yarn test --runInBand --detectOpenHandles --forceExit --verbose shared ui desktop
+    - run: cd runner && MallocNanoZone=0 go test -race -cover
     - run: yarn release-desktop 0.0.0-e2etest
     - run: git reset --hard # release blows everything up
     - run: yarn e2e-test
@@ -166,7 +167,7 @@ jobs:
     # Needed so we can have ./build/desktop_runner.js ready for tests
     - run: yarn build-test-runner
     - run: yarn test --runInBand --detectOpenHandles --forceExit --verbose shared ui desktop
-    - run: cd runner && go test -cover
+    - run: cd runner && go test -race -cover
     - run: yarn release-desktop 0.0.0-e2etest
     - run: git reset --hard # release blows everything up
     - run: yarn e2e-test

ARCHITECTURE.md

@@ -33,25 +33,14 @@ something to install all missing dependencies.
 
 ### ./desktop/panel
 
-NOTE: This code is being migrated to Go. All panel types except for a
-few database vendors have been ported to Go. A number of Node panel
-handlers have been deleted since they are no longer used.
-
-This is where eval handlers for each panel type (program, database,
-etc.) are defined.
-
-All database-specific handlers are defined in
-./desktop/panel/databases/*.ts since there are so many of them.
-
-Evaluation happens by spawning a Node.js subprocess where all the eval
-handlers are actually run. ./desktop/runner.ts is the entrypoint for
-this on desktop. ./server/runner.ts is the equivalent on the server.
-
-This allows easy resource cleanup and easy "kill" panel eval support.
+Evaluation happens by spawning a runner (see ./runner below)
+subprocess where all the eval handlers are actually run. This allows
+easy resource cleanup and easy "kill" panel eval support.
 
 ## ./runner
 
-This is where the Go port of the original Node.js panel eval code is.
+./runner/eval.go is the main eval entrypoint but the command line
+entrypoint is at ./runner/cmd/main.go.
 
 ## ./server
 
@@ -99,7 +88,20 @@ This is where types for all entities in state are described. Any time
 you want to add a new panel or connector, etc. you'll need to add it
 here.
 
+If the change needs to be read by the runner you'll also need to add
+the change to ./runner/state.go. There is nothing at the moment
+keeping these two files in sync other than tests that may exist.
+
 ### ./shared/languages
 
 This is where all supported languages are defined and where their
 libraries for interacting with DataStation are defined.
+
+The reason it's shared is because Python and JavaScript run in the
+browser in browser mode but on a machine in desktop/server mode.
+
+Language wrappers are defined in Jsonnet so that they can include
+comments and newlines. That is translated to JSON and embedded in Go
+with `yarn build-language-definitions`. The CI will complain if these
+definitions ever go out of date but it may be confusing locally not to
+see your changes until you run this and recompile the Go runner.

DEV_FAQ.md

@@ -1,6 +1,11 @@
+## yarn test vs yarn test-local
+
+The latter just doesn't involve running test coverage which takes
+longer and isn't that useful locally.
+
 ## Jest crashes with too many workers exception
 
-Run tests again with `--runInBand`: `yarn test --runInBand`.
+Run tests again with `--runInBand`: `yarn test-local --runInBand`.
 
 ## The packaged app is not starting, giving error messages
 
@@ -23,12 +28,12 @@ general.
 
 Should move to something that is not ts-jest eventually.
 
-## Run a single JavaScript test file
+## Run a single JavaScript test-local file
 
-Pass the test file name to `yarn test`.
+Pass the test-local file name to `yarn test`.
 
 ```bash
-yarn test server/exporter.test.js
+yarn test-local server/exporter.test.js
 ```
 
 ## Run tests on only a single runner type
@@ -41,7 +46,7 @@ tests against that runner will be run.
 For example:
 
 ```bash
-yarn test desktop/panel/file.test.js --dsrunner=memory
+yarn test-local desktop/panel/file.test.js --dsrunner=memory
 ```
 
 ## Adding/editing language libraries
@@ -55,4 +60,11 @@ To re-run this process, use on Mac or Linux:
 
 ```
 $ yarn build-language-definitions
-```
+```
+
+## Tests blow up with SIGABRT on macos when -race is on
+
+See https://github.com/golang/go/issues/49138. Set `MallocNanoZone=0`
+before running tests to get around this.
+
+Or upgrade to a more recent minor version.

desktop/panel/http.test.js

@@ -3,6 +3,7 @@ require('../../shared/polyfill');
 const { ensureSigningKey } = require('../secret');
 const { spawn } = require('child_process');
 const { CODE_ROOT } = require('../constants');
+const fetch = require('node-fetch');
 const path = require('path');
 const cp = require('child_process');
 const os = require('os');
@@ -47,10 +48,6 @@ beforeAll(async () => {
 
   // Start a new server for all tests
   server = spawn('python3', ['-m', 'http.server', PORT]);
-  let ready = false;
-  server.on('spawn', () => {
-    ready = true;
-  });
 
   server.stdout.on('data', (data) => {
     console.log(data.toString());
@@ -60,16 +57,18 @@ beforeAll(async () => {
     console.warn(data.toString());
   });
 
+  // Keep trying to connect to the server until it's ready
   return new Promise(async (resolve, reject) => {
-    try {
-      while (!ready) {
-        console.log('still waiting');
-        await new Promise((resolve) => setTimeout(resolve, 300));
+    while (true) {
+      try {
+        await fetch('http://localhost:' + PORT);
+        resolve();
+        return;
+      } catch (e) {
+        console.log(e);
       }
 
-      resolve();
-    } catch (e) {
-      reject(e);
+      await new Promise((resolve) => setTimeout(resolve, 1000));
     }
   });
 });

package.json

@@ -20,10 +20,12 @@
     "format": "yarn prettier --write \"**/*.js\" \"**/*.test.js\" \"**/*.test.jsx\" \"**/*.ts\" \"**/*.tsx\" \"**/*.css\" \"**/*.json\"",
     "test-licenses": "yarn license-checker --production --onlyAllow 'Python-2.0;AFLv2.1;BSD;MIT;Apache-2.0;Apache 2.0;Apache*;ISC;BSD-3-Clause;MIT*;BSD-2-Clause;Unlicense'",
     "generate-test-data": "node --unhandled-rejections=strict ./scripts/generate_test_data.js",
-    "test": "jest --passWithNoTests --coverage",
+    "test": "yarn test-local --coverage",
+    "test-local": "jest --passWithNoTests",
     "e2e-test": "node --unhandled-rejections=throw ./e2e/index.js"
   },
   "dependencies": {
+    "@tabler/icons": "^1.53.0",
     "ace-builds": "^1.4.13",
     "chart.js": "^3.5.1",
     "cookie-parser": "^1.4.5",

runner/scripts/runner_test.build

@@ -1,3 +1,3 @@
 yarn esbuild desktop/runner.ts --bundle --platform=node --sourcemap --external:sqlite3 --external:react-native-fs --external:react-native-fetch-blob --external:oracledb "--external:@elastic/elasticsearch" "--external:wasm-brotli" --external:prometheus-query --external:snowflake-sdk --external:ssh2 --external:ssh2-promise --external:ssh2-sftp-client --external:cpu-features --external:electron --external:pg-native --target=node10.4 --outfile=build/desktop_runner.js
 
-cd runner/cmd && go test -o ../../build/go_desktop_runner_test{required_ext} -coverpkg="." -c -tags testrunmain
+cd runner/cmd && go test -race -o ../../build/go_desktop_runner_test{required_ext} -coverpkg="." -c -tags testrunmain

runner/ssh.go

@@ -203,6 +203,54 @@ fi`, remoteFileName, remoteFileName)
 	return nil
 }
 
+// SOURCE: https://www.stavros.io/posts/proxying-two-connections-go/
+func chanFromConn(conn net.Conn) chan []byte {
+	c := make(chan []byte)
+
+	go func() {
+		b := make([]byte, 1024)
+
+		for {
+			n, err := conn.Read(b)
+			if n > 0 {
+				res := make([]byte, n)
+				// Copy the buffer so it doesn't get changed while read by the recipient.
+				copy(res, b[:n])
+				c <- res
+			}
+			if err != nil {
+				c <- nil
+				break
+			}
+		}
+	}()
+
+	return c
+}
+
+// SOURCE: https://www.stavros.io/posts/proxying-two-connections-go/
+func pipe(conn1 net.Conn, conn2 net.Conn) {
+	chan1 := chanFromConn(conn1)
+	chan2 := chanFromConn(conn2)
+
+	for {
+		select {
+		case b1 := <-chan1:
+			if b1 == nil {
+				return
+			} else {
+				conn2.Write(b1)
+			}
+		case b2 := <-chan2:
+			if b2 == nil {
+				return
+			} else {
+				conn1.Write(b2)
+			}
+		}
+	}
+}
+
 func withRemoteConnection(si *ServerInfo, host, port string, cb func(host, port string) error) error {
 	if si == nil {
 		return cb(host, port)
@@ -236,19 +284,7 @@ func withRemoteConnection(si *ServerInfo, host, port string, cb func(host, port
 			return
 		}
 
-		go func() {
-			_, err = io.Copy(remoteConn, localConn)
-			if errC != nil {
-				errC <- err
-			}
-
-		}()
-
-		// Remote server
-		go func() {
-			_, err = io.Copy(localConn, remoteConn)
-			errC <- err
-		}()
+		pipe(localConn, remoteConn)
 	}()
 
 	localPort := localConn.Addr().(*net.TCPAddr).Port

shared/constants.ts

@@ -7,14 +7,8 @@ export const RPC_ASYNC_RESPONSE = 'rpcAsyncResponse';
 
 function getConfig<T>(v: string, _default: T) {
   const key = 'DS_CONFIG_' + v;
-  let wg;
-  try {
-    wg = window as any;
-  } catch (e) {
-    wg = global as any;
-  }
-  if (key in wg) {
-    return wg[key] as T;
+  if (key in globalThis) {
+    return (globalThis as any)[key] as T;
   }
 
   return _default;
@@ -42,3 +36,7 @@ export const MODE_FEATURES = {
 // There is no /docs/development/ so replace it with /docs/latest/
 const DOCS_VERSION = VERSION === 'development' ? 'latest' : VERSION;
 export const DOCS_ROOT = SITE_ROOT + '/docs/' + DOCS_VERSION;
+
+export const IN_TESTS = globalThis.process
+  ? process.env.JEST_WORKER_ID !== undefined
+  : false;

shared/state.ts

@@ -321,15 +321,15 @@ export interface GraphField {
 
 export type GraphPanelInfoType = 'bar' | 'pie' | 'line';
 
-export type GraphPanelInfoWidth = 'small' | 'medium' | 'large';
+export type PanelInfoWidth = 'small' | 'medium' | 'large';
 
 export class GraphPanelInfo extends PanelInfo {
   graph: {
     panelSource: string;
     ys: Array<GraphField>;
     x: string;
     type: GraphPanelInfoType;
-    width: GraphPanelInfoWidth;
+    width: PanelInfoWidth;
     colors: {
       unique: boolean;
     };
@@ -455,6 +455,8 @@ export class TablePanelInfo extends PanelInfo {
   table: {
     columns: Array<TableColumn>;
     panelSource: string;
+    width: PanelInfoWidth;
+    rowNumbers: boolean;
   };
 
   constructor(
@@ -466,6 +468,8 @@ export class TablePanelInfo extends PanelInfo {
     this.table = {
       columns: defaults.columns || [],
       panelSource: defaults.panelSource || '',
+      width: defaults.width || 'small',
+      rowNumbers: defaults.rowNumbers || true,
     };
   }
 }

shared/table.ts

@@ -1,4 +1,5 @@
 import { NotAnArrayOfObjectsError } from './errors';
+import { getPath } from './object';
 
 export function columnsFromObject(
   value: any,
@@ -21,7 +22,7 @@ export function columnsFromObject(
 
     const cells: Record<string, any> = {};
     (columns || []).forEach((name) => {
-      cells[name] = row[name];
+      cells[name] = getPath(row, name);
     });
     return cells;
   });

testsetup.js

@@ -14,6 +14,14 @@ const jsdom = new JSDOM('<!doctype html><html><body></body></html>', {
 });
 const { window } = jsdom;
 
+class ResizeObserver {
+  observe() {}
+  unobserve() {}
+  disconnect() {}
+}
+
+window.ResizeObserver = ResizeObserver;
+
 function copyProps(src, target) {
   Object.defineProperties(target, {
     ...Object.getOwnPropertyDescriptors(src),

ui/Connector.tsx

@@ -1,3 +1,4 @@
+import { IconPencil, IconTrash } from '@tabler/icons';
 import * as React from 'react';
 import { ConnectorInfo, DatabaseConnectorInfo } from '../shared/state';
 import { Button } from './components/Button';
@@ -40,7 +41,7 @@ export function Connector({
               onClick={() => setExpanded(true)}
               title="Edit"
             >
-              edit
+              <IconPencil />
             </Button>
           )}
           <span title="Delete data source">
@@ -50,7 +51,7 @@ export function Connector({
               action="Delete"
               render={(confirm: () => void) => (
                 <Button icon onClick={confirm} type="outline">
-                  delete
+                  <IconTrash />
                 </Button>
               )}
             />