Merge pull request #294 from gbmhunter/develop

Release of v4.10.1.
gbmhunter · Nov 13, 2023 · c05ade5 · c05ade5
2 parents 9cd943d + c28c0af
commit c05ade5
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 55 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -28,6 +28,7 @@
     "*.{css,sass,scss}.d.ts": true
   },
   "cSpell.words": [
+    "abcdefghijklmnopqurstuvwxyz",
     "Applyable",
     "autorun",
     "autoscroll",

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## Unreleased
 
+## [4.10.1] - 2023-11-13
+
+### Fixed
+
+- Fixed bug which caused fatal error in JS if app was opened after local storage was cleared.
+- Fixed MobX console warning.
+
+### Changed
+
+- If no configs exist in the browser's local store, default-created config will be saved back to store even if no changes are made.
+- Removed unneeded console.log messages.
+
 ## [4.10.0] - 2023-11-12
 
 ### Added
@@ -576,7 +588,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Added auto-scroll to TX pane, closes #89.
 - Added special delete behaviour for backspace button when in "send on enter" mode, closes #90.
 
-[unreleased]: https://github.com/gbmhunter/NinjaTerm/compare/v4.10.0...HEAD
+[unreleased]: https://github.com/gbmhunter/NinjaTerm/compare/v4.10.1...HEAD
+[4.10.1]: https://github.com/gbmhunter/NinjaTerm/compare/v4.9.0...v4.10.1
 [4.10.0]: https://github.com/gbmhunter/NinjaTerm/compare/v4.9.0...v4.10.0
 [4.9.0]: https://github.com/gbmhunter/NinjaTerm/compare/v4.8.0...v4.9.0
 [4.8.0]: https://github.com/gbmhunter/NinjaTerm/compare/v4.7.0...v4.8.0

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "ninjaterm",
-  "version": "4.10.0",
+  "version": "4.10.1",
   "private": true,
   "dependencies": {
     "@emotion/react": "^11.11.1",

diff --git a/src/App.tsx b/src/App.tsx
@@ -209,17 +209,15 @@ export class App {
     // getPorts() returns ports that the user has previously approved
     // this app to be able to access
     let approvedPorts = await navigator.serial.getPorts();
-    console.log('ports: ', approvedPorts);
 
     // const lastUsedSerialPort = this.appStorage.data.lastUsedSerialPort;
     const lastUsedSerialPort: LastUsedSerialPort = this.appStorage.getData('lastUsedSerialPort') as LastUsedSerialPort;
     if (lastUsedSerialPort === null) {
-      console.log('Did not find last used serial port in local storage.');
+      // Did not find last used serial port data in local storage, so do nothing
       return;
     }
 
     const lastUsedPortInfoStr = JSON.stringify(lastUsedSerialPort.serialPortInfo);
-    console.log('lastUsedPortInfoStr=', lastUsedPortInfoStr);
     // If the JSON representation of the last used port is just "{}",
     // it means that the last used port didn't contain any valuable
     // information to uniquely identify it, so don't bother trying to
@@ -234,7 +232,6 @@ export class App {
       const approvedPortInfo = approvedPort.getInfo();
       const approvedPortInfoStr = JSON.stringify(approvedPort.getInfo());
       if (approvedPortInfoStr === lastUsedPortInfoStr) {
-        console.log('Found a match, opening port. portInfo=', approvedPortInfoStr);
         // Found a match, open it
         runInAction(async () => {
           this.port = approvedPort;
@@ -289,14 +286,19 @@ export class App {
         this.serialPortInfo = this.port.getInfo();
         // Save the info for this port, so we can automatically re-open
         // it on app re-open in the future
-        const lastUsedSerialPort: LastUsedSerialPort = this.appStorage.getData('lastUsedSerialPort');
+        let lastUsedSerialPort: LastUsedSerialPort = this.appStorage.getData('lastUsedSerialPort');
+        if (lastUsedSerialPort === null) {
+          lastUsedSerialPort = new LastUsedSerialPort();
+        }
         lastUsedSerialPort.serialPortInfo = this.serialPortInfo;
         this.appStorage.saveData('lastUsedSerialPort', lastUsedSerialPort);
 
       });
       if (this.settings.portConfiguration.connectToSerialPortAsSoonAsItIsSelected) {
         await this.openPort();
-        this.portState = PortState.OPENED;
+        runInAction(() => {
+          this.portState = PortState.OPENED;
+        });
         // Goto the terminal pane
         this.setShownMainPane(MainPanes.TERMINAL);
       }
@@ -345,18 +347,17 @@ export class App {
       if (printSuccessMsg) {
         this.snackbar.sendToSnackbar('Serial port opened.', 'success');
       }
-      this.portState = PortState.OPENED;
       // This will automatically close the settings window if the user is currently in it,
       // clicks "Open" and the port opens successfully.
       if (this.closeSettingsDialogOnPortOpenOrClose) {
         this.setSettingsDialogOpen(false);
       }
 
-      this.keepReading = true;
-      this.closedPromise = this.readUntilClosed();
-
-      // this.appStorage.data.lastUsedSerialPort.portState = PortState.OPENED;
-      // this.appStorage.saveData();
+      runInAction(() => {
+        this.portState = PortState.OPENED;
+        this.keepReading = true;
+        this.closedPromise = this.readUntilClosed();
+      });
 
       const lastUsedSerialPort: LastUsedSerialPort = this.appStorage.getData('lastUsedSerialPort');
       lastUsedSerialPort.portState = PortState.OPENED;
@@ -435,22 +436,12 @@ export class App {
           } else {
             this.snackbar.sendToSnackbar(`Serial port was removed unexpectedly.\nReturned error from reader.read():\n${error}`, 'error');
           }
-
-          // this.setPortState(PortState.CLOSED);
-          // runInAction(() => {
-          //   // Setting this.port to null means the port needs to be
-          //   // reselected in the UI (which makes sense because we just
-          //   // lost it)
-          //   // this.port = null;
-          //   // this.closedPromise = null;
-          // });
       } finally {
         // Allow the serial port to be closed later.
         this.reader.releaseLock();
       }
     }
 
-    console.log('GOT HERE! Closing port.... keepReading: ', this.keepReading);
     await this.port!.close();
 
     // If keepReading is true, this means close() was not called, and it's an unexpected
@@ -468,8 +459,6 @@ export class App {
       // have been removed/disappeared
       this.port = null;
     }
-
-    console.log('readUntilClosed() finished.');
   }
 
   setPortState(newPortState: PortState) {
@@ -496,7 +485,6 @@ export class App {
   }
 
   async closePort(goToReopenState = false) {
-    console.log('closePort() called.')
     if (this.lastSelectedPortType === PortType.REAL) {
       this.keepReading = false;
       // Force reader.read() to resolve immediately and subsequently
@@ -535,18 +523,6 @@ export class App {
     this.portState = PortState.CLOSED;
   }
 
-  // async setPortState(newPortState: PortState) {
-  //   if (newPortState === PortState.CLOSED) {
-  //     await this.closePort();
-  //   } else if (newPortState === PortState.CLOSED_BUT_WILL_REOPEN) {
-  //     // this.closeButWillReopenPort();
-  //   } else if (newPortState === PortState.OPENED) {
-  //     await this.openPort();
-  //   } else {
-  //     throw Error('Unsupported port state!');
-  //   }
-  // }
-
   /**
    * This is called from either the TX/RX terminal or TX terminal
    * (i.e. any terminal pane that is allowed to send data). This function

diff --git a/src/AppView.tsx b/src/AppView.tsx
@@ -95,12 +95,9 @@ declare global {
 
 window.app = app;
 
-console.log('TEST')
-
 const AppView = observer((props: Props) => {
 
   useEffect(() => {
-    console.log('useEffect1() called.');
     // We need to register the service worker AFTER the app
     // has rendered, because it we do it before we won't
     // be able to enqueue a snackbar to tell the user there
@@ -119,7 +116,6 @@ const AppView = observer((props: Props) => {
     })
 
     const initFn = async () => {
-      console.log('Calling init()...');
       await app.onAppUiLoaded();
     }
 

diff --git a/src/Storage/AppStorage.spec.ts b/src/Storage/AppStorage.spec.ts
@@ -1,16 +1,45 @@
-import { expect, test } from 'vitest'
+import { expect, test, describe, beforeEach } from 'vitest'
 
 import AppStorage from './AppStorage';
 
-test('get config returns null when nothing stored there', () => {
-  const appStorage = new AppStorage();
-  const value = appStorage.getConfig(['prop1', 'prop2']);
-  expect(value).toEqual(null);
+beforeEach(() => {
+  // Clear local storage, because otherwise jsdom persists storage
+  // between tests
+  window.localStorage.clear();
 })
 
-test('basic get and set works', () => {
-  const appStorage = new AppStorage();
-  appStorage.saveConfig(['prop1', 'prop2'], 'hello');
-  const value = appStorage.getConfig(['prop1', 'prop2']);
-  expect(value).toEqual('hello');
-})
+describe('config tests', () => {
+  test('get config returns null when nothing stored there', () => {
+    const appStorage = new AppStorage();
+    const value = appStorage.getConfig(['prop1', 'prop2']);
+    expect(value).toEqual(null);
+  })
+
+  test('basic get and set works', () => {
+    const appStorage = new AppStorage();
+    appStorage.saveConfig(['prop1', 'prop2'], 'hello');
+    const value = appStorage.getConfig(['prop1', 'prop2']);
+    expect(value).toEqual('hello');
+  })
+
+  test('can get parent key and see child', () => {
+    const appStorage = new AppStorage();
+    appStorage.saveConfig(['prop1', 'prop2'], 'hello');
+    const value = appStorage.getConfig(['prop1']);
+    expect(value).toEqual({ prop2: 'hello' });
+  })
+
+  test('can read back root config', () => {
+    const appStorage = new AppStorage();
+    appStorage.saveConfig(['prop1', 'prop2'], 'hello');
+    const value = appStorage.getConfig([]);
+    expect(value).toEqual({ prop1: { prop2: 'hello' } });
+  })
+
+  test('can set root config', () => {
+    const appStorage = new AppStorage();
+    appStorage.saveConfig([], 'hello');
+    const value = appStorage.getConfig([]);
+    expect(value).toEqual('hello');
+  })
+});
diff --git a/src/Storage/AppStorage.ts b/src/Storage/AppStorage.ts
@@ -4,6 +4,11 @@ class Config {
   configData: any = {};
 }
 
+/**
+ * This class manages the local storage (persistance browser based key/object data store)
+ * for the application. It is used to store both general data (saveData/getData) and configurations
+ * (saveConfig/getConfig).
+ */
 export default class AppStorage {
 
   configs: Config[] = [];
@@ -14,8 +19,13 @@ export default class AppStorage {
     // Read in configurations
     const configsStr = window.localStorage.getItem('configs');
     if (configsStr === null) {
+      // No config key found in users store, create one!
       const defaultConfig = new Config();
       this.configs.push(defaultConfig);
+      // Save just-created config back to store. Not strictly needed as it
+      // will be saved as soon as any changes are made, but this feels
+      // cleaner.
+      window.localStorage.setItem('configs', JSON.stringify(this.configs));
     } else {
       this.configs = JSON.parse(configsStr);
     }
@@ -50,7 +60,13 @@ export default class AppStorage {
       }
       obj = obj[key];
     }
-    obj[keys[keys.length - 1]] = data;
+    // If no keys were provided, we are writing to the entire
+    // config object
+    if (keys.length === 0) {
+      this.activeConfig.configData = data;
+    } else {
+      obj[keys[keys.length - 1]] = data;
+    }
     window.localStorage.setItem('configs', JSON.stringify(this.configs));
   }