Repository: amzn/ion-js
Pull request #623 (2020-07-28 19:53:39)

add deleteField method for deleting keys of Struct type
*Issue #, if available:*   #622 

*Description of changes:*
added a deleteField method which will delete the keys for Struct type. 
added unit-tests to check if deleteField works with built-in and user defind properties.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.



Patch:
diff --git a/src/dom/Struct.ts b/src/dom/Struct.ts
index a620ac6..21afbcc 100644
--- a/src/dom/Struct.ts
+++ b/src/dom/Struct.ts
@@ -59,6 +59,14 @@ export class Struct extends Value(Object, IonTypes.STRUCT, FromJsConstructor.NON
                     return target[name];
                 }
                 return target._fields[name];
+            },
+            deleteProperty: function (target, name): boolean {
+                // Property is deleted only if it's in _field Collection
+                if (name in target._fields) {
+                    delete target._fields[name];
+                }
+                // Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete#Return_value
+                return true;
             }
         });
     }
@@ -115,6 +123,14 @@ export class Struct extends Value(Object, IonTypes.STRUCT, FromJsConstructor.NON
         writer.stepOut();
     }
 
+    deleteField(name: string): boolean {
+        if (name in this._fields) {
+            delete this._fields[name];
+            return true;
+        }
+        return false;
+    }
+
     toJSON() {
         return this._fields;
     }
@@ -127,4 +143,4 @@ export class Struct extends Value(Object, IonTypes.STRUCT, FromJsConstructor.NON
             .map(([key, value]) => [key, Value.from(value)]);
         return new this(fields, annotations);
     }
-}
\ No newline at end of file
+}
diff --git a/src/dom/Value.ts b/src/dom/Value.ts
index 13bde40..8418fda 100644
--- a/src/dom/Value.ts
+++ b/src/dom/Value.ts
@@ -138,6 +138,12 @@ export interface Value {
      * @param writer    An Ion Writer to which the value should be written.
      */
     writeTo(writer: Writer): void;
+
+    /**
+     * For the Struct type, deletes the field with provided name if present. Returns a boolean
+     * indicating whether the Struct was modified. For all other types, throws an Error.
+     */
+    deleteField(name: string): boolean;
 }
 
 /**
@@ -289,6 +295,10 @@ export function Value<Clazz extends Constructor>(BaseClass: Clazz, ionType: IonT
             this._unsupportedOperation('writeTo');
         }
 
+        deleteField(name: string): boolean {
+            this._unsupportedOperation('deleteField');
+        }
+
         // Returns the IonType associated with a particular dom.Value subclass. Useful for testing.
         static _getIonType(): IonType {
             return ionType;


Test patch:
diff --git a/test/dom/propertyShadowing.ts b/test/dom/propertyShadowing.ts
index 6592b3a..bec0577 100644
--- a/test/dom/propertyShadowing.ts
+++ b/test/dom/propertyShadowing.ts
@@ -7,13 +7,16 @@ describe('dom.Struct property shadowing', () => {
         let s = dom.Value.from({
             getType: "baz",
             getAnnotations: 56,
-            fieldNames: ['dog', 'cat', 'mouse']
+            fieldNames: ['dog', 'cat', 'mouse'],
+            toString: 10,
+            greetings: "hi",
+            age: 7
         }, ['foo', 'bar']);
 
         // Method names are still directly accessible
         assert.equal(s.getType(), IonTypes.STRUCT);
         assert.deepEqual(s.getAnnotations(), ['foo', 'bar']);
-        assert.deepEqual(s.fieldNames(), ['getType', 'getAnnotations', 'fieldNames']);
+        assert.deepEqual(s.fieldNames(), ['getType', 'getAnnotations', 'fieldNames', 'toString', 'greetings', 'age']);
 
         // Fields with names that would shadow a built-in are accessible via Value#get()
         assert.equal(s.get('getType')!.stringValue(), "baz");
@@ -22,6 +25,30 @@ describe('dom.Struct property shadowing', () => {
         assert.equal(s.get('fieldNames', 1)!.stringValue(), 'cat');
         assert.equal(s.get('fieldNames', 2)!.stringValue(), 'mouse');
 
+        // deleteProperty proxy method
+        assert.isTrue(delete s['greetings']);
+        assert.deepEqual(s.fieldNames(), ['getType', 'getAnnotations', 'fieldNames', 'toString', 'age']);
+
+        // deleteField Struct method
+        assert.isTrue(s.deleteField('age'));
+        assert.deepEqual(s.fieldNames(), ['getType', 'getAnnotations', 'fieldNames', 'toString']);
+
+        // delete for properties that match built-in
+        assert.equal(s.get('toString')!.numberValue(), 10);
+        assert.isTrue(delete s['toString']);
+        assert.equal(typeof s.toString, "function");
+        assert.deepEqual(s.fieldNames(), ['getType', 'getAnnotations', 'fieldNames']);
+
+        // delete for field that doesn't exist
+        assert.isFalse(s.deleteField('toString'));
+        assert.isFalse(s.deleteField('greetings'));
+        assert.isFalse(s.deleteField('name'));
+        assert.isUndefined(s['greetings']);
+
+        // deleteField will throw an error if it's called on a dom.Value that isn't a struct
+        let l = dom.Value.from([1, 2, 3]);
+        assert.throws(() => l.deleteField("1"), Error);
+
         // get() does not return values for properties on `Object`
         assert.isNull(s.get('toString'));
         assert.isNull(s.get('toLocaleString'));