From d6d540e748d314739e58b4362d7b3aebd13d928f Mon Sep 17 00:00:00 2001
From: Simon Lamon <simonlamon93@hotmail.com>
Date: Sat, 20 Nov 2021 14:58:07 +0000
Subject: [PATCH] use a proxy

---
 src/common/services/OptionsService.test.ts |  9 ++-
 src/common/services/OptionsService.ts      | 68 ++++++++++------------
 2 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts
index 8675b6b9..3bbc876d 100644
--- a/src/common/services/OptionsService.test.ts
+++ b/src/common/services/OptionsService.test.ts
@@ -10,11 +10,16 @@ describe('OptionsService', () => {
   describe('constructor', () => {
     const originalError = console.error;
     beforeEach(() => {
-      console.error = () => {};
+      console.error = () => { };
     });
     afterEach(() => {
       console.error = originalError;
     });
+    it('publicOptions and options are modifying the same object', () => {
+      const optionsService = new OptionsService({ cols: undefined, rows: undefined });
+      optionsService.options.rows = 25;
+      assert.equal(optionsService.publicOptions.rows, 25);
+    });
     it('uses default value if invalid constructor option values passed for cols/rows', () => {
       const optionsService = new OptionsService({ cols: undefined, rows: undefined });
       assert.equal(optionsService.getOption('rows'), DEFAULT_OPTIONS.rows);
@@ -26,7 +31,7 @@ describe('OptionsService', () => {
       assert.equal(optionsService.getOption('cols'), 80);
     });
     it('uses default value if invalid constructor option value passed', () => {
-      assert.equal(new OptionsService({tabStopWidth: 0}).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth);
+      assert.equal(new OptionsService({ tabStopWidth: 0 }).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth);
     });
   });
   describe('setOption', () => {
diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts
index a245c153..1ad73ea4 100644
--- a/src/common/services/OptionsService.ts
+++ b/src/common/services/OptionsService.ts
@@ -65,7 +65,6 @@ const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows'];
 export class OptionsService implements IOptionsService {
   public serviceBrand: any;
 
-  private _options: ITerminalOptions;
   public options: ITerminalOptions;
   public publicOptions: ITerminalOptions;
 
@@ -74,12 +73,12 @@ export class OptionsService implements IOptionsService {
 
   constructor(options: Partial<ITerminalOptions>) {
     // set the default value of each option
-    this._options = { ...DEFAULT_OPTIONS };
+    const defaultOptions = { ...DEFAULT_OPTIONS };
     for (const key in options) {
-      if (key in this._options) {
+      if (key in defaultOptions) {
         try {
           const newValue = options[key];
-          this._options[key] = this._sanitizeAndValidateOption(key, newValue);
+          defaultOptions[key] = this._sanitizeAndValidateOption(key, newValue);
         } catch (e) {
           console.error(e);
         }
@@ -87,42 +86,39 @@ export class OptionsService implements IOptionsService {
     }
 
     // set up getters and setters for each option
-    this.options = this._setupOptions(this._options, false);
-    this.publicOptions = this._setupOptions(this._options, true);
+    this.options = this._setupOptions(defaultOptions, false);
+    this.publicOptions = this._setupOptions(defaultOptions, true);
   }
 
   private _setupOptions(options: ITerminalOptions, isPublic: boolean): ITerminalOptions {
-    const copiedOptions = { ... options };
-    for (const propName in copiedOptions) {
-      Object.defineProperty(copiedOptions, propName, {
-        get: () => {
-          if (!(propName in DEFAULT_OPTIONS)) {
-            throw new Error(`No option with key "${propName}"`);
-          }
-          return this._options[propName];
-        },
-        set: (value: any) => {
-          if (!(propName in DEFAULT_OPTIONS)) {
-            throw new Error(`No option with key "${propName}"`);
-          }
-
-          // Throw an error if any constructor only option is modified
-          // from terminal.options
-          // Modifications from anywhere else are allowed
-          if (isPublic && CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) {
-            throw new Error(`Option "${propName}" can only be set in the constructor`);
-          }
-
-          value = this._sanitizeAndValidateOption(propName, value);
-          // Don't fire an option change event if they didn't change
-          if (this._options[propName] !== value) {
-            this._options[propName] = value;
-            this._onOptionChange.fire(propName);
-          }
+    return new Proxy(options, {
+      get: (target, propName: string): any => {
+        if (!(propName in DEFAULT_OPTIONS)) {
+          throw new Error(`No option with key "${propName}"`);
         }
-      });
-    }
-    return copiedOptions;
+        return target[propName];
+      },
+      set: (target, propName: string, value): any => {
+        if (!(propName in DEFAULT_OPTIONS)) {
+          throw new Error(`No option with key "${propName}"`);
+        }
+
+        // Throw an error if any constructor only option is modified
+        // from terminal.options
+        // Modifications from anywhere else are allowed
+        if (isPublic && CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) {
+          throw new Error(`Option "${propName}" can only be set in the constructor`);
+        }
+
+        value = this._sanitizeAndValidateOption(propName, value);
+        // Don't fire an option change event if they didn't change
+        if (target[propName] !== value) {
+          target[propName] = value;
+          this._onOptionChange.fire(propName);
+        }
+        return true;
+      }
+    });
   }
 
   public setOption(key: string, value: any): void {