trxcllnt commented on a change in pull request #8216:
URL: https://github.com/apache/arrow/pull/8216#discussion_r492260498



##########
File path: .env
##########
@@ -30,7 +30,7 @@ LLVM=10
 CLANG_TOOLS=8
 RUST=nightly-2020-04-22
 GO=1.12
-NODE=11
+NODE=14

Review comment:
       No we still work on node 11, but node changed the behavior of the 
`stream.readable` property, which we're checking in 
[`streams-node-tests.ts`](https://github.com/apache/arrow/blob/63264ce2ef37e55b1561927ebb28ba9804271cfa/js/test/unit/ipc/reader/streams-node-tests.ts#L212-L213).
 We used to check for true, but now it's false.

##########
File path: js/test/unit/ipc/helpers.ts
##########
@@ -54,13 +54,13 @@ export abstract class ArrowIOTestHelper {
             await testFn(await this.writer(this.table).toUint8Array());
         };
     }
-    iterable(testFn: (iterable: Iterable<Uint8Array>) => void | Promise<void>) 
{
+    iterable(testFn: (iterable: Generator<Uint8Array>) => void | 
Promise<void>) {
         return async () => {
             expect.hasAssertions();
             await testFn(chunkedIterable(await 
this.writer(this.table).toUint8Array()));
         };
     }
-    asyncIterable(testFn: (asyncIterable: AsyncIterable<Uint8Array>) => void | 
Promise<void>) {
+    asyncIterable(testFn: (asyncIterable: AsyncGenerator<Uint8Array>) => void 
| Promise<void>) {

Review comment:
       This is a change in the TypeScript typings for generator functions. They 
didn't used to have a way to capture the error and return types of generator 
functions, but now they do. Most places we don't care about the error or return 
types, but have to use the Generator interfaces because TS 3.5+ requires it.

##########
File path: js/src/data.ts
##########
@@ -253,11 +254,11 @@ export class Data<T extends DataType = DataType> {
         return new Data(type, offset, length, nullCount, [undefined, 
toArrayBufferView(type.ArrayType, data), toUint8Array(nullBitmap)]);
     }
     /** @nocollapse */
-    public static Binary<T extends Binary>(type: T, offset: number, length: 
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets: 
ValueOffsetsBuffer, data: Uint8Array) {
+    public static Binary<T extends Binary>(type: T, offset: number, length: 
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets: 
ValueOffsetsBuffer, data: DataBuffer<T>) {
         return new Data(type, offset, length, nullCount, 
[toInt32Array(valueOffsets), toUint8Array(data), toUint8Array(nullBitmap)]);
     }
     /** @nocollapse */
-    public static Utf8<T extends Utf8>(type: T, offset: number, length: 
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets: 
ValueOffsetsBuffer, data: Uint8Array) {
+    public static Utf8<T extends Utf8>(type: T, offset: number, length: 
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets: 
ValueOffsetsBuffer, data: DataBuffer<T>) {

Review comment:
       Yes this argument doesn't have to be a Uint8Array (it could be 
null/undefined or [] too).

##########
File path: js/src/interfaces.ts
##########
@@ -356,48 +356,48 @@ type TypeToBuilder<T extends Type = any, TNull = any> = {
 
 /** @ignore */
 type DataTypeToBuilder<T extends DataType = any, TNull = any> = {
-    [key: number               ]:                                              
                                                    builders.Builder<any, 
TNull> ;
-    [Type.Null                 ]: T extends type.Null                 ? 
builders.NullBuilder<TNull>                              : 
builders.Builder<any, TNull> ;
-    [Type.Bool                 ]: T extends type.Bool                 ? 
builders.BoolBuilder<TNull>                              : 
builders.Builder<any, TNull> ;
-    [Type.Int8                 ]: T extends type.Int8                 ? 
builders.Int8Builder<TNull>                              : 
builders.Builder<any, TNull> ;
-    [Type.Int16                ]: T extends type.Int16                ? 
builders.Int16Builder<TNull>                             : 
builders.Builder<any, TNull> ;
-    [Type.Int32                ]: T extends type.Int32                ? 
builders.Int32Builder<TNull>                             : 
builders.Builder<any, TNull> ;
-    [Type.Int64                ]: T extends type.Int64                ? 
builders.Int64Builder<TNull>                             : 
builders.Builder<any, TNull> ;
-    [Type.Uint8                ]: T extends type.Uint8                ? 
builders.Uint8Builder<TNull>                             : 
builders.Builder<any, TNull> ;
-    [Type.Uint16               ]: T extends type.Uint16               ? 
builders.Uint16Builder<TNull>                            : 
builders.Builder<any, TNull> ;
-    [Type.Uint32               ]: T extends type.Uint32               ? 
builders.Uint32Builder<TNull>                            : 
builders.Builder<any, TNull> ;
-    [Type.Uint64               ]: T extends type.Uint64               ? 
builders.Uint64Builder<TNull>                            : 
builders.Builder<any, TNull> ;
-    [Type.Int                  ]: T extends type.Int                  ? 
builders.IntBuilder<T, TNull>                            : 
builders.Builder<any, TNull> ;
-    [Type.Float16              ]: T extends type.Float16              ? 
builders.Float16Builder<TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.Float32              ]: T extends type.Float32              ? 
builders.Float32Builder<TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.Float64              ]: T extends type.Float64              ? 
builders.Float64Builder<TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.Float                ]: T extends type.Float                ? 
builders.FloatBuilder<T, TNull>                          : 
builders.Builder<any, TNull> ;
-    [Type.Utf8                 ]: T extends type.Utf8                 ? 
builders.Utf8Builder<TNull>                              : 
builders.Builder<any, TNull> ;
-    [Type.Binary               ]: T extends type.Binary               ? 
builders.BinaryBuilder<TNull>                            : 
builders.Builder<any, TNull> ;
-    [Type.FixedSizeBinary      ]: T extends type.FixedSizeBinary      ? 
builders.FixedSizeBinaryBuilder<TNull>                   : 
builders.Builder<any, TNull> ;
-    [Type.Date                 ]: T extends type.Date_                ? 
builders.DateBuilder<T, TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.DateDay              ]: T extends type.DateDay              ? 
builders.DateDayBuilder<TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.DateMillisecond      ]: T extends type.DateMillisecond      ? 
builders.DateMillisecondBuilder<TNull>                   : 
builders.Builder<any, TNull> ;
-    [Type.Timestamp            ]: T extends type.Timestamp            ? 
builders.TimestampBuilder<T, TNull>                      : 
builders.Builder<any, TNull> ;
-    [Type.TimestampSecond      ]: T extends type.TimestampSecond      ? 
builders.TimestampSecondBuilder<TNull>                   : 
builders.Builder<any, TNull> ;
-    [Type.TimestampMillisecond ]: T extends type.TimestampMillisecond ? 
builders.TimestampMillisecondBuilder<TNull>              : 
builders.Builder<any, TNull> ;
-    [Type.TimestampMicrosecond ]: T extends type.TimestampMicrosecond ? 
builders.TimestampMicrosecondBuilder<TNull>              : 
builders.Builder<any, TNull> ;
-    [Type.TimestampNanosecond  ]: T extends type.TimestampNanosecond  ? 
builders.TimestampNanosecondBuilder<TNull>               : 
builders.Builder<any, TNull> ;
-    [Type.Time                 ]: T extends type.Time                 ? 
builders.TimeBuilder<T, TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.TimeSecond           ]: T extends type.TimeSecond           ? 
builders.TimeSecondBuilder<TNull>                        : 
builders.Builder<any, TNull> ;
-    [Type.TimeMillisecond      ]: T extends type.TimeMillisecond      ? 
builders.TimeMillisecondBuilder<TNull>                   : 
builders.Builder<any, TNull> ;
-    [Type.TimeMicrosecond      ]: T extends type.TimeMicrosecond      ? 
builders.TimeMicrosecondBuilder<TNull>                   : 
builders.Builder<any, TNull> ;
-    [Type.TimeNanosecond       ]: T extends type.TimeNanosecond       ? 
builders.TimeNanosecondBuilder<TNull>                    : 
builders.Builder<any, TNull> ;
-    [Type.Decimal              ]: T extends type.Decimal              ? 
builders.DecimalBuilder<TNull>                           : 
builders.Builder<any, TNull> ;
-    [Type.Union                ]: T extends type.Union                ? 
builders.UnionBuilder<T, TNull>                          : 
builders.Builder<any, TNull> ;
-    [Type.DenseUnion           ]: T extends type.DenseUnion           ? 
builders.DenseUnionBuilder<T, TNull>                     : 
builders.Builder<any, TNull> ;
-    [Type.SparseUnion          ]: T extends type.SparseUnion          ? 
builders.SparseUnionBuilder<T, TNull>                    : 
builders.Builder<any, TNull> ;
-    [Type.Interval             ]: T extends type.Interval             ? 
builders.IntervalBuilder<T, TNull>                       : 
builders.Builder<any, TNull> ;
-    [Type.IntervalDayTime      ]: T extends type.IntervalDayTime      ? 
builders.IntervalDayTimeBuilder<TNull>                   : 
builders.Builder<any, TNull> ;
-    [Type.IntervalYearMonth    ]: T extends type.IntervalYearMonth    ? 
builders.IntervalYearMonthBuilder<TNull>                 : 
builders.Builder<any, TNull> ;
-    [Type.Map                  ]: T extends type.Map_                 ? 
builders.MapBuilder<T['keyType'], T['valueType'], TNull> : 
builders.Builder<any, TNull> ;
-    [Type.List                 ]: T extends type.List                 ? 
builders.ListBuilder<T['valueType'], TNull>              : 
builders.Builder<any, TNull> ;
-    [Type.Struct               ]: T extends type.Struct               ? 
builders.StructBuilder<T['dataTypes'], TNull>            : 
builders.Builder<any, TNull> ;
-    [Type.Dictionary           ]: T extends type.Dictionary           ? 
builders.DictionaryBuilder<T, TNull>                     : 
builders.Builder<any, TNull> ;
-    [Type.FixedSizeList        ]: T extends type.FixedSizeList        ? 
builders.FixedSizeListBuilder<T['valueType'], TNull>     : 
builders.Builder<any, TNull> ;
+    [key: number               ]:                                       
builders.Builder<any, TNull>                                     ;
+    [Type.Null                 ]: T extends type.Null                 ? 
builders.NullBuilder<TNull>                              : never ;
+    [Type.Bool                 ]: T extends type.Bool                 ? 
builders.BoolBuilder<TNull>                              : never ;
+    [Type.Int8                 ]: T extends type.Int8                 ? 
builders.Int8Builder<TNull>                              : never ;
+    [Type.Int16                ]: T extends type.Int16                ? 
builders.Int16Builder<TNull>                             : never ;
+    [Type.Int32                ]: T extends type.Int32                ? 
builders.Int32Builder<TNull>                             : never ;
+    [Type.Int64                ]: T extends type.Int64                ? 
builders.Int64Builder<TNull>                             : never ;
+    [Type.Uint8                ]: T extends type.Uint8                ? 
builders.Uint8Builder<TNull>                             : never ;
+    [Type.Uint16               ]: T extends type.Uint16               ? 
builders.Uint16Builder<TNull>                            : never ;
+    [Type.Uint32               ]: T extends type.Uint32               ? 
builders.Uint32Builder<TNull>                            : never ;
+    [Type.Uint64               ]: T extends type.Uint64               ? 
builders.Uint64Builder<TNull>                            : never ;
+    [Type.Int                  ]: T extends type.Int                  ? 
builders.IntBuilder<T, TNull>                            : never ;
+    [Type.Float16              ]: T extends type.Float16              ? 
builders.Float16Builder<TNull>                           : never ;
+    [Type.Float32              ]: T extends type.Float32              ? 
builders.Float32Builder<TNull>                           : never ;
+    [Type.Float64              ]: T extends type.Float64              ? 
builders.Float64Builder<TNull>                           : never ;
+    [Type.Float                ]: T extends type.Float                ? 
builders.FloatBuilder<T, TNull>                          : never ;
+    [Type.Utf8                 ]: T extends type.Utf8                 ? 
builders.Utf8Builder<TNull>                              : never ;
+    [Type.Binary               ]: T extends type.Binary               ? 
builders.BinaryBuilder<TNull>                            : never ;
+    [Type.FixedSizeBinary      ]: T extends type.FixedSizeBinary      ? 
builders.FixedSizeBinaryBuilder<TNull>                   : never ;
+    [Type.Date                 ]: T extends type.Date_                ? 
builders.DateBuilder<T, TNull>                           : never ;
+    [Type.DateDay              ]: T extends type.DateDay              ? 
builders.DateDayBuilder<TNull>                           : never ;
+    [Type.DateMillisecond      ]: T extends type.DateMillisecond      ? 
builders.DateMillisecondBuilder<TNull>                   : never ;
+    [Type.Timestamp            ]: T extends type.Timestamp            ? 
builders.TimestampBuilder<T, TNull>                      : never ;
+    [Type.TimestampSecond      ]: T extends type.TimestampSecond      ? 
builders.TimestampSecondBuilder<TNull>                   : never ;
+    [Type.TimestampMillisecond ]: T extends type.TimestampMillisecond ? 
builders.TimestampMillisecondBuilder<TNull>              : never ;
+    [Type.TimestampMicrosecond ]: T extends type.TimestampMicrosecond ? 
builders.TimestampMicrosecondBuilder<TNull>              : never ;
+    [Type.TimestampNanosecond  ]: T extends type.TimestampNanosecond  ? 
builders.TimestampNanosecondBuilder<TNull>               : never ;
+    [Type.Time                 ]: T extends type.Time                 ? 
builders.TimeBuilder<T, TNull>                           : never ;
+    [Type.TimeSecond           ]: T extends type.TimeSecond           ? 
builders.TimeSecondBuilder<TNull>                        : never ;
+    [Type.TimeMillisecond      ]: T extends type.TimeMillisecond      ? 
builders.TimeMillisecondBuilder<TNull>                   : never ;
+    [Type.TimeMicrosecond      ]: T extends type.TimeMicrosecond      ? 
builders.TimeMicrosecondBuilder<TNull>                   : never ;
+    [Type.TimeNanosecond       ]: T extends type.TimeNanosecond       ? 
builders.TimeNanosecondBuilder<TNull>                    : never ;
+    [Type.Decimal              ]: T extends type.Decimal              ? 
builders.DecimalBuilder<TNull>                           : never ;
+    [Type.Union                ]: T extends type.Union                ? 
builders.UnionBuilder<T, TNull>                          : never ;
+    [Type.DenseUnion           ]: T extends type.DenseUnion           ? 
builders.DenseUnionBuilder<T, TNull>                     : never ;
+    [Type.SparseUnion          ]: T extends type.SparseUnion          ? 
builders.SparseUnionBuilder<T, TNull>                    : never ;
+    [Type.Interval             ]: T extends type.Interval             ? 
builders.IntervalBuilder<T, TNull>                       : never ;
+    [Type.IntervalDayTime      ]: T extends type.IntervalDayTime      ? 
builders.IntervalDayTimeBuilder<TNull>                   : never ;
+    [Type.IntervalYearMonth    ]: T extends type.IntervalYearMonth    ? 
builders.IntervalYearMonthBuilder<TNull>                 : never ;
+    [Type.Map                  ]: T extends type.Map_                 ? 
builders.MapBuilder<T['keyType'], T['valueType'], TNull> : never ;
+    [Type.List                 ]: T extends type.List                 ? 
builders.ListBuilder<T['valueType'], TNull>              : never ;
+    [Type.Struct               ]: T extends type.Struct               ? 
builders.StructBuilder<T['dataTypes'], TNull>            : never ;
+    [Type.Dictionary           ]: T extends type.Dictionary           ? 
builders.DictionaryBuilder<T, TNull>                     : never ;
+    [Type.FixedSizeList        ]: T extends type.FixedSizeList        ? 
builders.FixedSizeListBuilder<T['valueType'], TNull>     : never ;

Review comment:
       The newer versions of tsc to die in some kind of recursive loop, and the 
changes in this file seem to be the fix. Ultimately I think we can get rid of 
most of the definitions in this file with a larger refactor (to come later).

##########
File path: js/test/Arrow.ts
##########
@@ -27,6 +27,13 @@ require('web-stream-tools');
 (<any> global).window = (<any> global).window || global;
 
 // Fix for Jest in node v10.x
+Object.defineProperty(Object, Symbol.hasInstance, {
+    writable: true,
+    configurable: true,
+    value(inst: any) {
+        return inst && inst.constructor && inst.constructor.name === 'Object';
+    }
+});

Review comment:
       This is a weird one, it only seems to be important for the ES5/UMD 
closure-compiler build.

##########
File path: js/test/inference/column.ts
##########
@@ -33,33 +33,6 @@ const boolColumn = new Column(new Field('bool', boolType), [
 expect(typeof boolVector.get(0) === 'boolean').toBe(true);
 expect(typeof boolColumn.get(0) === 'boolean').toBe(true);
 
-type NamedSchema = {
-    a: Int8;
-    b: Utf8;
-    c: Dictionary<List<Bool>>;
-};
-
-const mapChildFields = [
-    { name: 'a', type: new Int8() },
-    { name: 'b', type: new Utf8() },
-    { name: 'c', type: new Dictionary<List<Bool>>(null!, null!) }
-].map(({ name, type }) => new Field(name, type));
-
-const mapType = new Map_<NamedSchema>(mapChildFields);
-
-const mapVector = Vector.new(Data.Map(mapType, 0, 0, 0, null, []));
-const mapColumn = new Column(new Field('map', mapType, false), [
-    Vector.new(Data.Map(mapType, 0, 0, 0, null, [])),
-    Vector.new(Data.Map(mapType, 0, 0, 0, null, [])),
-    Vector.new(Data.Map(mapType, 0, 0, 0, null, [])),
-]);
-
-const { a: a1, b: b1, c: c1 } = mapVector.get(0)!;
-const { a: a2, b: b2, c: c2 } = mapColumn.get(0)!;
-
-console.log(a1, b1, c1);
-console.log(a2, b2, c2);
-

Review comment:
       Yeah these aren't run, they're here to test the type 
inference/intellisense in VSCode. I manually check whether the types show up 
properly after any large set of changes to the type propagation, like in this 
PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to