From 6066b667961d176634bda30e3f395653dd30f30d Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Mon, 22 Jan 2024 18:48:02 +0200 Subject: [PATCH 1/3] Add polyfills to make `apache-arrow` work in Node@14 Signed-off-by: Levko Kravets --- lib/index.ts | 3 +++ lib/polyfills.ts | 48 +++++++++++++++++++++++++++++++++++++++++ tests/e2e/arrow.test.js | 4 ++-- 3 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 lib/polyfills.ts diff --git a/lib/index.ts b/lib/index.ts index bf3b3d81..710a036d 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -1,3 +1,6 @@ +// Don't move this import - it should be placed before any other +import './polyfills'; + import { Thrift } from 'thrift'; import TCLIService from '../thrift/TCLIService'; import TCLIService_types from '../thrift/TCLIService_types'; diff --git a/lib/polyfills.ts b/lib/polyfills.ts new file mode 100644 index 00000000..b3045085 --- /dev/null +++ b/lib/polyfills.ts @@ -0,0 +1,48 @@ +// `Array.at` / `TypedArray.at` is supported only since Nodejs@16.6.0 +// These methods are massively used by `apache-arrow@13`, but we have +// to use this version because older ones contain some other nasty bugs + +// https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tointegerorinfinity +function toIntegerOrInfinity(value: unknown): number { + const result = Number(value); + + // Return `0` for NaN; return `+Infinity` / `-Infinity` as is + if (!Number.isFinite(result)) { + return Number.isNaN(result) ? 0 : result; + } + + return Math.trunc(result); +} + +// https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tolength +function toLength(value: unknown): number { + const result = toIntegerOrInfinity(value); + return result > 0 ? Math.min(result, Number.MAX_SAFE_INTEGER) : 0; +} + +// https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.prototype.at +function at(this: Array, index: number): T | undefined { + const length = toLength(this.length); + const relativeIndex = toIntegerOrInfinity(index); + const absoluteIndex = relativeIndex >= 0 ? relativeIndex : length + relativeIndex; + return absoluteIndex >= 0 && absoluteIndex < length ? this[absoluteIndex] : undefined; +} + +const ArrayConstructors = [ + global.Array, + global.Int8Array, + global.Uint8Array, + global.Uint8ClampedArray, + global.Int16Array, + global.Uint16Array, + global.Int32Array, + global.Uint32Array, + global.Float32Array, + global.Float64Array, + global.BigInt64Array, + global.BigUint64Array, +]; + +ArrayConstructors.forEach((ArrayConstructor) => { + ArrayConstructor.prototype.at = ArrayConstructor.prototype.at ?? at; +}); diff --git a/tests/e2e/arrow.test.js b/tests/e2e/arrow.test.js index d23c9552..5de7e93e 100644 --- a/tests/e2e/arrow.test.js +++ b/tests/e2e/arrow.test.js @@ -48,10 +48,10 @@ async function deleteTable(session, tableName) { async function initializeTable(session, tableName) { await deleteTable(session, tableName); - const createTable = fixtures.createTableSql.replaceAll('${table_name}', tableName); + const createTable = fixtures.createTableSql.replace(/\$\{table_name\}/g, tableName); await execute(session, createTable); - const insertData = fixtures.insertDataSql.replaceAll('${table_name}', tableName); + const insertData = fixtures.insertDataSql.replace(/\$\{table_name\}/g, tableName); await execute(session, insertData); } From 50ab0846af1b6d725ffd98248dba2c03bfdb1e3a Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Mon, 22 Jan 2024 19:21:10 +0200 Subject: [PATCH 2/3] Add tests Signed-off-by: Levko Kravets --- lib/polyfills.ts | 4 +- tests/unit/polyfills.test.js | 76 ++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 tests/unit/polyfills.test.js diff --git a/lib/polyfills.ts b/lib/polyfills.ts index b3045085..4d15e1dc 100644 --- a/lib/polyfills.ts +++ b/lib/polyfills.ts @@ -1,3 +1,5 @@ +/* eslint-disable import/prefer-default-export */ + // `Array.at` / `TypedArray.at` is supported only since Nodejs@16.6.0 // These methods are massively used by `apache-arrow@13`, but we have // to use this version because older ones contain some other nasty bugs @@ -21,7 +23,7 @@ function toLength(value: unknown): number { } // https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.prototype.at -function at(this: Array, index: number): T | undefined { +export function at(this: Array, index: number): T | undefined { const length = toLength(this.length); const relativeIndex = toIntegerOrInfinity(index); const absoluteIndex = relativeIndex >= 0 ? relativeIndex : length + relativeIndex; diff --git a/tests/unit/polyfills.test.js b/tests/unit/polyfills.test.js new file mode 100644 index 00000000..a1e392d9 --- /dev/null +++ b/tests/unit/polyfills.test.js @@ -0,0 +1,76 @@ +const { expect } = require('chai'); +const { at } = require('../../dist/polyfills'); + +const defaultArrayMock = { + 0: 'a', + 1: 'b', + 2: 'c', + 3: 'd', + length: 4, + at, +}; + +describe('Array.at', () => { + it('should handle zero index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(0)).to.eq('a'); + expect(obj.at(Number('+0'))).to.eq('a'); + expect(obj.at(Number('-0'))).to.eq('a'); + }); + + it('should handle positive index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(2)).to.eq('c'); + expect(obj.at(2.2)).to.eq('c'); + expect(obj.at(2.8)).to.eq('c'); + }); + + it('should handle negative index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(-2)).to.eq('c'); + expect(obj.at(-2.2)).to.eq('c'); + expect(obj.at(-2.8)).to.eq('c'); + }); + + it('should handle positive infinity index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(Number.POSITIVE_INFINITY)).to.be.undefined; + }); + + it('should handle negative infinity index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(Number.NEGATIVE_INFINITY)).to.be.undefined; + }); + + it('should handle non-numeric index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at('2')).to.eq('c'); + }); + + it('should handle NaN index', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(Number.NaN)).to.eq('a'); + expect(obj.at('invalid')).to.eq('a'); + }); + + it('should handle index out of bounds', () => { + const obj = { ...defaultArrayMock }; + expect(obj.at(10)).to.be.undefined; + expect(obj.at(-10)).to.be.undefined; + }); + + it('should handle zero length', () => { + const obj = { ...defaultArrayMock, length: 0 }; + expect(obj.at(2)).to.be.undefined; + }); + + it('should handle negative length', () => { + const obj = { ...defaultArrayMock, length: -4 }; + expect(obj.at(2)).to.be.undefined; + }); + + it('should handle non-numeric length', () => { + const obj = { ...defaultArrayMock, length: 'invalid' }; + expect(obj.at(2)).to.be.undefined; + }); +}); From dd20df986b68fce6bea0261da5d915cabf86468c Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 24 Jan 2024 20:06:59 +0200 Subject: [PATCH 3/3] Run unit tests on Node@14 Signed-off-by: Levko Kravets --- .github/workflows/main.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 858733d4..030d063e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -35,8 +35,7 @@ jobs: strategy: matrix: # only LTS versions starting from the lowest we support - # TODO: Include Nodejs@14 - node-version: ['16', '18', '20'] + node-version: ['14', '16', '18', '20'] env: cache-name: cache-node-modules NYC_REPORT_DIR: coverage_unit_node${{ matrix.node-version }}