Merge pull request #1342 from tediousjs/arthur/do-not-modify-input-ob…

…jects fix: do not modify objects passed to `Request.addOutputParameter` or `Request.addParameter`
tediousjs · Sep 19, 2021 · 4196d85 · 4196d85
2 parents 51754a6 + f1acd07
commit 4196d85
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 12 deletions.
diff --git a/src/request.ts b/src/request.ts
@@ -398,12 +398,8 @@ class Request extends EventEmitter {
    *   Additional type options. Optional.
    */
   // TODO: `type` must be a valid TDS value type
-  addParameter(name: string, type: DataType, value?: unknown, options?: ParameterOptions | null) {
-    if (options == null) {
-      options = {};
-    }
-
-    const { output = false, length, precision, scale } = options;
+  addParameter(name: string, type: DataType, value?: unknown, options?: Readonly<ParameterOptions> | null) {
+    const { output = false, length, precision, scale } = options ?? {};
 
     const parameter: Parameter = {
       type: type,
@@ -414,6 +410,7 @@ class Request extends EventEmitter {
       precision: precision,
       scale: scale
     };
+
     this.parameters.push(parameter);
     this.parametersByName[name] = parameter;
   }
@@ -433,12 +430,8 @@ class Request extends EventEmitter {
    * @param options
    *   Additional type options. Optional.
    */
-  addOutputParameter(name: string, type: DataType, value?: unknown, options?: ParameterOptions) {
-    if (options == null) {
-      options = {};
-    }
-    options.output = true;
-    this.addParameter(name, type, value, options);
+  addOutputParameter(name: string, type: DataType, value?: unknown, options?: Readonly<ParameterOptions> | null) {
+    this.addParameter(name, type, value, { ...options, output: true });
   }
 
   /**

diff --git a/test/unit/request-test.js b/test/unit/request-test.js
@@ -1,4 +1,5 @@
 const { assert } = require('chai');
+const { TYPES } = require('../../src/data-type');
 
 const Request = require('../../src/request');
 
@@ -36,4 +37,20 @@ describe('Request', function() {
       assert.strictEqual(eventEmitted, false);
     });
   });
+
+  describe('#addOutputParameter', function() {
+    it('does not modify the passed in options object', function() {
+      const request = new Request('', () => {});
+
+      request.addOutputParameter('foo', TYPES.NVarChar, 'test', Object.freeze({ length: 10 }));
+    });
+  });
+
+  describe('#addParameter', function() {
+    it('does not modify the passed in options object', function() {
+      const request = new Request('', () => { });
+
+      request.addParameter('foo', TYPES.NVarChar, 'test', Object.freeze({ length: 10 }));
+    });
+  });
 });