From 3d614d6d414d0d34026f516dbfad7d296000efcf Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Mon, 19 Jun 2023 17:16:16 -0700
Subject: [PATCH] fix(base-driver): allow subclass to define shape of settings
 object

The mixin approach prevented subclasses of BaseDriver from narrowing the type of `this.settings`.  Now, that's possible.

Also:
- removed an unused type argument from BD constructor
- and did some typedef aliasing
- remove unneeded type assertion from log mixin
---
 .../lib/basedriver/commands/index.ts          |  1 -
 .../lib/basedriver/commands/log.ts            |  6 ++---
 .../lib/basedriver/commands/settings.ts       | 26 -------------------
 .../lib/basedriver/device-settings.js         |  8 ++++--
 packages/base-driver/lib/basedriver/driver.ts | 19 +++++++++++---
 5 files changed, 25 insertions(+), 35 deletions(-)
 delete mode 100644 packages/base-driver/lib/basedriver/commands/settings.ts

diff --git a/packages/base-driver/lib/basedriver/commands/index.ts b/packages/base-driver/lib/basedriver/commands/index.ts
index 096f3a7ff..860b9b979 100644
--- a/packages/base-driver/lib/basedriver/commands/index.ts
+++ b/packages/base-driver/lib/basedriver/commands/index.ts
@@ -1,6 +1,5 @@
 import './event';
 import './find';
 import './log';
-import './settings';
 import './timeout';
 import './execute';
diff --git a/packages/base-driver/lib/basedriver/commands/log.ts b/packages/base-driver/lib/basedriver/commands/log.ts
index 166a1a3ba..b4026d8c7 100644
--- a/packages/base-driver/lib/basedriver/commands/log.ts
+++ b/packages/base-driver/lib/basedriver/commands/log.ts
@@ -1,6 +1,6 @@
-import {Constraints, Driver, ILogCommands, LogDefRecord} from '@appium/types';
+import type {Constraints, Driver, ILogCommands} from '@appium/types';
 import _ from 'lodash';
-import {BaseDriver} from '../driver';
+import type {BaseDriver} from '../driver';
 import {mixin} from './mixin';
 
 declare module '../driver' {
@@ -9,7 +9,7 @@ declare module '../driver' {
 }
 
 const LogCommands: ILogCommands = {
-  supportedLogTypes: <LogDefRecord>{},
+  supportedLogTypes: {},
 
   async getLogTypes<C extends Constraints>(this: BaseDriver<C>) {
     this.log.debug('Retrieving supported log types');
diff --git a/packages/base-driver/lib/basedriver/commands/settings.ts b/packages/base-driver/lib/basedriver/commands/settings.ts
deleted file mode 100644
index 1953516f7..000000000
--- a/packages/base-driver/lib/basedriver/commands/settings.ts
+++ /dev/null
@@ -1,26 +0,0 @@
-import {BaseDriver} from '../driver';
-import {Constraints, ISettingsCommands, StringRecord} from '@appium/types';
-import {mixin} from './mixin';
-
-declare module '../driver' {
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  interface BaseDriver<C extends Constraints> extends ISettingsCommands {}
-}
-
-const SettingsCommands: ISettingsCommands = {
-  async updateSettings<C extends Constraints>(this: BaseDriver<C>, newSettings: StringRecord) {
-    if (!this.settings) {
-      this.log.errorAndThrow('Cannot update settings; settings object not found');
-    }
-    return await this.settings.update(newSettings);
-  },
-
-  async getSettings<C extends Constraints>(this: BaseDriver<C>) {
-    if (!this.settings) {
-      this.log.errorAndThrow('Cannot get settings; settings object not found');
-    }
-    return this.settings.getSettings();
-  },
-};
-
-mixin(SettingsCommands);
diff --git a/packages/base-driver/lib/basedriver/device-settings.js b/packages/base-driver/lib/basedriver/device-settings.js
index 1da3456b0..202df2dfe 100644
--- a/packages/base-driver/lib/basedriver/device-settings.js
+++ b/packages/base-driver/lib/basedriver/device-settings.js
@@ -9,7 +9,7 @@ import {errors} from '../protocol/errors';
 export const MAX_SETTINGS_SIZE = 20 * 1024 * 1024; // 20 MB
 
 /**
- * @template {import('@appium/types').StringRecord} T
+ * @template {StringRecord} T
  * @implements {IDeviceSettings<T>}
  */
 export class DeviceSettings {
@@ -74,6 +74,10 @@ export class DeviceSettings {
 export default DeviceSettings;
 
 /**
- * @template {import('@appium/types').StringRecord} [T=import('@appium/types').StringRecord]
+ * @typedef {import('@appium/types').StringRecord} StringRecord
+ */
+
+/**
+ * @template {StringRecord} [T=StringRecord]
  * @typedef {import('@appium/types').IDeviceSettings<T>} IDeviceSettings
  */
diff --git a/packages/base-driver/lib/basedriver/driver.ts b/packages/base-driver/lib/basedriver/driver.ts
index 156e2bc75..b03658ba6 100644
--- a/packages/base-driver/lib/basedriver/driver.ts
+++ b/packages/base-driver/lib/basedriver/driver.ts
@@ -33,11 +33,10 @@ const ON_UNEXPECTED_SHUTDOWN_EVENT = 'onUnexpectedShutdown';
 export class BaseDriver<
     C extends Constraints,
     CArgs extends StringRecord = StringRecord,
-    Settings extends StringRecord = StringRecord,
-    SessionData extends StringRecord = StringRecord
+    Settings extends StringRecord = StringRecord
   >
   extends DriverCore<C, Settings>
-  implements Driver<C, CArgs>
+  implements Driver<C, CArgs, Settings>
 {
   cliArgs: CArgs & ServerArgs;
 
@@ -380,6 +379,20 @@ export class BaseDriver<
 
     return true;
   }
+
+  async updateSettings(newSettings: Settings) {
+    if (!this.settings) {
+      this.log.errorAndThrow('Cannot update settings; settings object not found');
+    }
+    return await this.settings.update(newSettings);
+  }
+
+  async getSettings() {
+    if (!this.settings) {
+      this.log.errorAndThrow('Cannot get settings; settings object not found');
+    }
+    return this.settings.getSettings();
+  }
 }
 
 export * from './commands';