Repository: amzn/ion-js
Pull request #603 (2020-04-06 20:03:57)

Fix for large containers, instanceof
*Issue #, if available:* #601, #602 

*Description of changes:*

* `dom.Sequence` now loops over items to add to a sequence rather than calling `push(...items)`, which is subject to limitations on the number of parameters the JS VM will allow a function to accept.
* `object instanceof Value` now verifies that the `object` being tested has a `constructor` property before dereferencing it. This allows `object`s without a `constructor` property to be used to construct instances of `dom.Struct`.

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/Sequence.ts b/src/dom/Sequence.ts
index 25ddde6..7184675 100644
--- a/src/dom/Sequence.ts
+++ b/src/dom/Sequence.ts
@@ -18,7 +18,9 @@ export function Sequence(ionType: IonType) {
     return class extends Value(Array, ionType, FromJsConstructor.NONE) implements Value, Array<Value> {
         protected constructor(children: Value[], annotations: string[] = []) {
             super();
-            this.push(...children);
+            for (let child of children) {
+                this.push(child);
+            }
             this._setAnnotations(annotations);
         }
 
diff --git a/src/dom/Value.ts b/src/dom/Value.ts
index 102b845..13bde40 100644
--- a/src/dom/Value.ts
+++ b/src/dom/Value.ts
@@ -369,6 +369,7 @@ export namespace Value {
 Object.defineProperty(Value, Symbol.hasInstance, {
     get: () => (instance) => {
         return _hasValue(instance)
+            && _hasValue(instance.constructor)
             && _DOM_VALUE_SIGNET in instance.constructor
             && instance.constructor[_DOM_VALUE_SIGNET] === _DOM_VALUE_SIGNET;
     }


Test patch:
diff --git a/test/dom/Value.ts b/test/dom/Value.ts
index 19ef641..6e76a60 100644
--- a/test/dom/Value.ts
+++ b/test/dom/Value.ts
@@ -269,7 +269,8 @@ describe('Value', () => {
         describe('No plain Javascript value is an instance of dom.Value', () => {
             instanceOfValueTest(
                 false,
-                ...exampleJsValuesWhere()
+                ...exampleJsValuesWhere(),
+                Object.create(null)
             );
         });
         for (let subclass of DOM_VALUE_SUBCLASSES) {
@@ -439,4 +440,30 @@ describe('Value', () => {
             {foo: ['dog', 'cat', 'mouse']}
         );
     });
+    describe('Large containers', () => {
+        const LARGE_CONTAINER_NUM_ENTRIES = 1_000_000;
+        let largeJsArray: Value[] = new Array(LARGE_CONTAINER_NUM_ENTRIES);
+        let largeJsObject = {};
+        for (let i = 0; i < LARGE_CONTAINER_NUM_ENTRIES; i++) {
+            let ionValue = Value.from(i);
+            largeJsArray[i] = ionValue;
+            largeJsObject[i] = ionValue;
+        }
+        it('List', () => {
+            let ionList = Value.from(largeJsArray) as any;
+            assert.equal(ionList.getType(), IonTypes.LIST);
+            assert.equal(ionList.length, LARGE_CONTAINER_NUM_ENTRIES);
+        });
+        it('S-Expression', () => {
+            let ionSExpression = new dom.SExpression(largeJsArray) as any;
+            assert.equal(ionSExpression.getType(), IonTypes.SEXP);
+            assert.equal(ionSExpression.length, LARGE_CONTAINER_NUM_ENTRIES);
+        });
+        it('Struct', function () {
+            this.timeout(5_000)
+            let ionStruct = Value.from(largeJsObject) as any;
+            assert.equal(ionStruct.getType(), IonTypes.STRUCT);
+            assert.equal(ionStruct.fields().length, LARGE_CONTAINER_NUM_ENTRIES);
+        });
+    });
 });
\ No newline at end of file
diff --git a/test/mochaSupport.ts b/test/mochaSupport.ts
index 98d1414..fe8a4ed 100644
--- a/test/mochaSupport.ts
+++ b/test/mochaSupport.ts
@@ -1,10 +1,15 @@
 // Generates a mocha-friendly string representation of the provided value that can be used in test names
+import {_hasValue} from "../src/util";
+
 export function valueName(value: any): string {
     if (value === null) {
         return 'null';
     }
     if (typeof value === "object") {
-        return `${value.constructor.name}(${value})`;
+        let typeText = _hasValue(value.constructor) ? value.constructor.name : 'Object';
+        let valueText = _hasValue(value.toString) ? ''+value : JSON.stringify(value);
+
+        return `${typeText}(${valueText})`;
     }
     return `${typeof value}(${value})`;
 }